Change in ...coreboot[master]: Revert "src/arch: An upgrade of SMBIOS to latest version 3.2"

[Nico Huber (Code Review)]

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32059

Change subject: Revert "src/arch: An upgrade of SMBIOS to latest version 3.2"
......................................................................

Revert "src/arch: An upgrade of SMBIOS to latest version 3.2"

This reverts commit b7daf7e8fa18de7bfb3cd102791bc6af89bac4b6.

The review was spread across four different change-ids. Of course,
not all comments were addressed, now coverity complains too.

Change-Id: If5dbc1ae37120330ab192fb15eb4984afc84a7af
Signed-off-by: Nico Huber 
---
M src/arch/x86/smbios.c
M src/include/smbios.h
2 files changed, 1 insertion(+), 72 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/32059/1

diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c
index b82780d..8cb59df 100644
--- a/src/arch/x86/smbios.c
+++ b/src/arch/x86/smbios.c
@@ -328,17 +328,6 @@
    t->handle = *handle;
    *handle += 1;
    t->length = sizeof(struct smbios_type17) - 2;
-	t->memory_technology = MEMORY_TECHNOLOGY_UNKNOWN;
-	t->operating_mode_capability = MEMORY_OPERATING_MODE_CAP_UNKNOWN;
-	t->fw_version = 0xff;
-	t->manufacturer_id = dimm->mod_id;
-	t->product_id = 0x0000;
-	t->sub_ctrl_manufacturer_id = 0x0000;
-	t->sub_ctrl_product_id = 0x0000;
-	t->non_volatile_size = 0xffffffffffffffff;
-	t->volatile_size = 0xffffffffffffffff;
-	t->cache_size = 0xffffffffffffffff;
-	t->logical_size = 0xffffffffffffffff;
    return t->length + smbios_string_table_len(t->eos);
 }

@@ -558,31 +547,9 @@
    return len;
 }

-u16 __weak smbios_processor_core_thread_count(u16 level_type)
-{
-	u16 count = 0;
-	int ecx = 0;
-
-	for (ecx = 0 ; ecx < 255 ; ecx++) {
-		struct cpuid_result leaf_b;
-		leaf_b = cpuid_ext(0xb, ecx);
-		if ((cpuid_eax(0) < 0xb) ||
-		!(leaf_b.eax | leaf_b.ebx | leaf_b.ecx | leaf_b.edx))
-			return (((cpuid(1).ebx) >> 16) & 0x00ff);
-
-		if ((leaf_b.ecx & 0xff00) == level_type) {
-			count = leaf_b.ebx & 0xffff;
-			break;
-		}
-	}
-
-	return count;
-}
-
 static int smbios_write_type4(unsigned long *current, int handle)
 {
    struct cpuid_result res;
-	u16 core_count = 0, thread_count = 0;
    struct smbios_type4 *t = (struct smbios_type4 *)*current;
    int len = sizeof(struct smbios_type4);

@@ -603,15 +570,7 @@
    t->processor_version = smbios_processor_name(t->eos);
    t->processor_family = (res.eax > 0) ? 0x0c : 0x6;
    t->processor_type = 3; /* System Processor */
-
-	core_count = smbios_processor_core_thread_count(PROC_CORE_TYPE);
-	thread_count = smbios_processor_core_thread_count(PROC_THREAD_TYPE);
-	t->core_count2 = core_count;
-	t->core_count = (core_count > BYTE_LIMIT) ? 0xff : core_count;
-	t->thread_count2 = thread_count;
-	t->thread_count = (thread_count > BYTE_LIMIT) ? 0xff : core_count;
-	t->core_enabled2 = core_count;
-
+	t->core_count = (res.ebx >> 16) & 0xff;
    t->l1_cache_handle = 0xffff;
    t->l2_cache_handle = 0xffff;
    t->l3_cache_handle = 0xffff;
diff --git a/src/include/smbios.h b/src/include/smbios.h
index 2f3bc7a..af83bfe 100644
--- a/src/include/smbios.h
+++ b/src/include/smbios.h
@@ -52,10 +52,6 @@
 u8 smbios_mainboard_feature_flags(void);
 const char *smbios_mainboard_location_in_chassis(void);
 u8 smbios_mainboard_enclosure_type(void);
-u16 smbios_processor_core_thread_count(u16 level_type);
-#ifdef CONFIG_MAINBOARD_FAMILY
-const char *smbios_mainboard_family(void);
-#endif

#define BIOS_CHARACTERISTICS_PCI_SUPPORTED	(1 << 7)
 #define BIOS_CHARACTERISTICS_PC_CARD		(1 << 8)
@@ -104,11 +100,6 @@
 #define MEMORY_OPERATING_MODE_CAP_BYTE_ACCESS_PERSISTENT	(1 << 4)
 #define MEMORY_OPERATING_MODE_CAP_BLOCK_ACCESS_PERSISTENT	(1 << 5)

-#define PROC_THREAD_TYPE			0x1
-#define PROC_CORE_TYPE				0x2
-
-#define BYTE_LIMIT				255
-
 typedef enum {
    MEMORY_BUS_WIDTH_8 = 0,
    MEMORY_BUS_WIDTH_16 = 1,
@@ -308,8 +299,6 @@
    u8 location_in_chassis;
    u16 chassis_handle;
    u8 board_type;
-	u8 num_cont_obj_handles;
-	u16 cont_obj_hanles[256];
    u8 eos[2];
 } __packed;

@@ -401,9 +390,6 @@
    u8 thread_count;
    u16 processor_characteristics;
    u16 processor_family2;
-	u16 core_count2;
-	u16 core_enabled2;
-	u16 thread_count2;
    u8 eos[2];
 } __packed;

@@ -415,11 +401,6 @@
    u8 eos[2];
 } __packed;

-typedef struct {
-	u8 type;
-	u8 format_descriptor;
-} log_type_descriptor;
-
 struct smbios_type15 {
    u8 type;
    u8 length;
@@ -490,17 +471,6 @@
    u16 minimum_voltage;
    u16 maximum_voltage;
    u16 configured_voltage;
-	u8 memory_technology;
-	u16 operating_mode_capability;
-	u8 fw_version;
-	u16 manufacturer_id;
-	u16 product_id;
-	u16 sub_ctrl_manufacturer_id;
-	u16 sub_ctrl_product_id;
-	u64 non_volatile_size;
-	u64 volatile_size;
-	u64 cache_size;
-	u64 logical_size;
    u8 eos[2];
 } __packed;

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32059 )

Change subject: Revert "src/arch: An upgrade of SMBIOS to latest version 3.2"
......................................................................

Patch Set 1: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32059 )

Change subject: Revert "src/arch: An upgrade of SMBIOS to latest version 3.2"
......................................................................

Patch Set 1: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32059 )

Change subject: Revert "src/arch: An upgrade of SMBIOS to latest version 3.2"
......................................................................

Revert "src/arch: An upgrade of SMBIOS to latest version 3.2"

This reverts commit b7daf7e8fa18de7bfb3cd102791bc6af89bac4b6.

The review was spread across four different change-ids. Of course,
not all comments were addressed, now coverity complains too.

Change-Id: If5dbc1ae37120330ab192fb15eb4984afc84a7af
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32059
Tested-by: build bot (Jenkins) 
Reviewed-by: Patrick Rudolph 
Reviewed-by: Patrick Georgi 
---
M src/arch/x86/smbios.c
M src/include/smbios.h
2 files changed, 1 insertion(+), 72 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved
  Patrick Rudolph: Looks good to me, approved

diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c
index b82780d..8cb59df 100644
--- a/src/arch/x86/smbios.c
+++ b/src/arch/x86/smbios.c
@@ -328,17 +328,6 @@
    t->handle = *handle;
    *handle += 1;
    t->length = sizeof(struct smbios_type17) - 2;
-	t->memory_technology = MEMORY_TECHNOLOGY_UNKNOWN;
-	t->operating_mode_capability = MEMORY_OPERATING_MODE_CAP_UNKNOWN;
-	t->fw_version = 0xff;
-	t->manufacturer_id = dimm->mod_id;
-	t->product_id = 0x0000;
-	t->sub_ctrl_manufacturer_id = 0x0000;
-	t->sub_ctrl_product_id = 0x0000;
-	t->non_volatile_size = 0xffffffffffffffff;
-	t->volatile_size = 0xffffffffffffffff;
-	t->cache_size = 0xffffffffffffffff;
-	t->logical_size = 0xffffffffffffffff;
    return t->length + smbios_string_table_len(t->eos);
 }

@@ -558,31 +547,9 @@
    return len;
 }

-u16 __weak smbios_processor_core_thread_count(u16 level_type)
-{
-	u16 count = 0;
-	int ecx = 0;
-
-	for (ecx = 0 ; ecx < 255 ; ecx++) {
-		struct cpuid_result leaf_b;
-		leaf_b = cpuid_ext(0xb, ecx);
-		if ((cpuid_eax(0) < 0xb) ||
-		!(leaf_b.eax | leaf_b.ebx | leaf_b.ecx | leaf_b.edx))
-			return (((cpuid(1).ebx) >> 16) & 0x00ff);
-
-		if ((leaf_b.ecx & 0xff00) == level_type) {
-			count = leaf_b.ebx & 0xffff;
-			break;
-		}
-	}
-
-	return count;
-}
-
 static int smbios_write_type4(unsigned long *current, int handle)
 {
    struct cpuid_result res;
-	u16 core_count = 0, thread_count = 0;
    struct smbios_type4 *t = (struct smbios_type4 *)*current;
    int len = sizeof(struct smbios_type4);

@@ -603,15 +570,7 @@
    t->processor_version = smbios_processor_name(t->eos);
    t->processor_family = (res.eax > 0) ? 0x0c : 0x6;
    t->processor_type = 3; /* System Processor */
-
-	core_count = smbios_processor_core_thread_count(PROC_CORE_TYPE);
-	thread_count = smbios_processor_core_thread_count(PROC_THREAD_TYPE);
-	t->core_count2 = core_count;
-	t->core_count = (core_count > BYTE_LIMIT) ? 0xff : core_count;
-	t->thread_count2 = thread_count;
-	t->thread_count = (thread_count > BYTE_LIMIT) ? 0xff : core_count;
-	t->core_enabled2 = core_count;
-
+	t->core_count = (res.ebx >> 16) & 0xff;
    t->l1_cache_handle = 0xffff;
    t->l2_cache_handle = 0xffff;
    t->l3_cache_handle = 0xffff;
diff --git a/src/include/smbios.h b/src/include/smbios.h
index 2f3bc7a..af83bfe 100644
--- a/src/include/smbios.h
+++ b/src/include/smbios.h
@@ -52,10 +52,6 @@
 u8 smbios_mainboard_feature_flags(void);
 const char *smbios_mainboard_location_in_chassis(void);
 u8 smbios_mainboard_enclosure_type(void);
-u16 smbios_processor_core_thread_count(u16 level_type);
-#ifdef CONFIG_MAINBOARD_FAMILY
-const char *smbios_mainboard_family(void);
-#endif

#define BIOS_CHARACTERISTICS_PCI_SUPPORTED	(1 << 7)
 #define BIOS_CHARACTERISTICS_PC_CARD		(1 << 8)
@@ -104,11 +100,6 @@
 #define MEMORY_OPERATING_MODE_CAP_BYTE_ACCESS_PERSISTENT	(1 << 4)
 #define MEMORY_OPERATING_MODE_CAP_BLOCK_ACCESS_PERSISTENT	(1 << 5)

-#define PROC_THREAD_TYPE			0x1
-#define PROC_CORE_TYPE				0x2
-
-#define BYTE_LIMIT				255
-
 typedef enum {
    MEMORY_BUS_WIDTH_8 = 0,
    MEMORY_BUS_WIDTH_16 = 1,
@@ -308,8 +299,6 @@
    u8 location_in_chassis;
    u16 chassis_handle;
    u8 board_type;
-	u8 num_cont_obj_handles;
-	u16 cont_obj_hanles[256];
    u8 eos[2];
 } __packed;

@@ -401,9 +390,6 @@
    u8 thread_count;
    u16 processor_characteristics;
    u16 processor_family2;
-	u16 core_count2;
-	u16 core_enabled2;
-	u16 thread_count2;
    u8 eos[2];
 } __packed;

@@ -415,11 +401,6 @@
    u8 eos[2];
 } __packed;

-typedef struct {
-	u8 type;
-	u8 format_descriptor;
-} log_type_descriptor;
-
 struct smbios_type15 {
    u8 type;
    u8 length;
@@ -490,17 +471,6 @@
    u16 minimum_voltage;
    u16 maximum_voltage;
    u16 configured_voltage;
-	u8 memory_technology;
-	u16 operating_mode_capability;
-	u8 fw_version;
-	u16 manufacturer_id;
-	u16 product_id;
-	u16 sub_ctrl_manufacturer_id;
-	u16 sub_ctrl_product_id;
-	u64 non_volatile_size;
-	u64 volatile_size;
-	u64 cache_size;
-	u64 logical_size;
    u8 eos[2];
 } __packed;

[Francois Toguo Fotso (Code Review)]

Francois Toguo Fotso has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32059 )

Change subject: Revert "src/arch: An upgrade of SMBIOS to latest version 3.2"
......................................................................

Patch Set 2:

Hello Nico,

I spotted the last non addressed commentin src/include/smbios.h
A new patch has submitted with the correction.
Could you please take a look:

https://review.coreboot.org/c/coreboot/+/32090

Regards,
Francois