[PATCH 1/1] cmd: smbios: correct output for table type 4

[Heinrich Schuchardt]

On 11/17/25 16:48, Raymond Mao wrote:
> Hi Heinrich,
> 
> On Sun, Nov 16, 2025 at 5:39 AM Heinrich Schuchardt 
> <heinrich.schuchardt at canonical.com 
> <mailto:heinrich.schuchardt at canonical.com>> wrote:
> 
>     In the QFW case the SMBIOS specification version is controlled by QEMU.
>     Field 'Thread Enable' is not available before SMBIOS v3.6.
> 
>     We should not print the core, core enabled, and thread counts twice.
>     Use the appropriate based on the SMBIOS version.
>     Use decimal output which is easier to read for humans.
> 
>     Signed-off-by: Heinrich Schuchardt
>     <heinrich.schuchardt at canonical.com
>     <mailto:heinrich.schuchardt at canonical.com>>
>     ---
>       cmd/smbios.c | 21 +++++++++++++++------
>       1 file changed, 15 insertions(+), 6 deletions(-)
> 
>     diff --git a/cmd/smbios.c b/cmd/smbios.c
>     index 671c14e05b5..f9a352c107d 100644
>     --- a/cmd/smbios.c
>     +++ b/cmd/smbios.c
>     @@ -262,6 +262,8 @@ static const struct str_lookup_table
>     md_tech_strings[] = {
>              { SMBIOS_MD_TECH_OPTANE,        "Intel Optane persistent
>     memory" },
>       };
> 
>     +static u16 smbios_version;
>     +
>       /**
>        * smbios_get_string() - get SMBIOS string from table
>        *
>     @@ -504,18 +506,23 @@ static void smbios_print_type4(struct
>     smbios_type4 *table)
>              smbios_print_str("Serial Number", table, table->serial_number);
>              smbios_print_str("Asset Tag", table, table->asset_tag);
>              smbios_print_str("Part Number", table, table->part_number);
>     -       printf("\tCore Count: 0x%02x\n", table->core_count);
>     -       printf("\tCore Enabled: 0x%02x\n", table->core_enabled);
>     -       printf("\tThread Count: 0x%02x\n", table->thread_count);
>     +       if (smbios_version < 0x0300) {
>     +               printf("\tCore Count: %d\n", table->core_count);
>     +               printf("\tCore Enabled: %d\n", table->core_enabled);
>     +               printf("\tThread Count: %d\n", table->thread_count);
>     +       } else {
>     +               printf("\tCore Count: %d\n", table->core_count2);
>     +               printf("\tCore Enabled: %d\n", table->core_enabled2);
>     +               printf("\tThread Count: %d\n", table->thread_count2);
>     +       }
> 
> 
> I think for ">= 0x0300", both parts are needed.
> 
> Below is the description of " Core Count 2" in spec: https:// 
> www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.8.0.pdf 
> <https://www.dmtf.org/sites/default/files/standards/documents/ 
> DSP0134_3.8.0.pdf>
> Number of Cores per processor socket. Supports core counts >255. If this 
> field is present, it holds the core count for the processor socket. Core 
> Count will also hold the core count, except for core counts that are 256 
> or greater. In that case, Core Count shall be set to FFh and Core Count 
> 2 will hold the count. See 7.5.6. Legal values: 0000h = unknown 
> 0001h-00FFh = core counts 1 to 255. Matches Core Count value. 0100h- 
> FFFEh = Core counts 256 to 65534, respectively. FFFFh = reserved.
> 
> Similar for core_enabled2 and thread_count2, they are all extensions 
> when values above 255.

The descriptions of fields "Core Count 2", "Core Enabled 2", "Thread 
Count 2" all say: "If this field is present, it holds the ... for the 
processor socket."

So there is no need to look into the old fields "Core Count", "Core 
Enabled", "Thread Count" for SMBIOS version >= 3.0 and test for 0xff.

We should avoid printing duplicate information.

Best regards

Heinrich

> 
> So, if for the consideration of the old versions of spec, the change 
> should be:
> ```
> -       printf("\tCore Count 2: 0x%04x\n", table->core_count2);
> -       printf("\tCore Enabled 2: 0x%04x\n", table->core_enabled2);
> -       printf("\tThread Count 2: 0x%04x\n", table->thread_count2);
> +      if (smbios_version >= 0x0300) {
> +               printf("\tCore Count 2: %d\n", table->core_count2);
> +               printf("\tCore Enabled 2: %d\n", table->core_enabled2);
> +               printf("\tThread Count 2: %d\n", table->thread_count2);
> +       }
> ```
> Moreover, I think the same logic should be done when the table is being 
> constructed in lib/smbios.c, where the real content of tables is located.
> 
> Regards,
> Raymond
> 
>              printf("\tProcessor Characteristics: 0x%04x\n",
>                     table->processor_characteristics);
>              smbios_print_lookup_str(processor_family_strings,
>                                      table->processor_family2,
>                                      ARRAY_SIZE(processor_family_strings),
>                                      "Processor Family 2");
>     -       printf("\tCore Count 2: 0x%04x\n", table->core_count2);
>     -       printf("\tCore Enabled 2: 0x%04x\n", table->core_enabled2);
>     -       printf("\tThread Count 2: 0x%04x\n", table->thread_count2);
>     +       if (smbios_version < 0x0306)
>     +               return;
>              printf("\tThread Enabled: 0x%04x\n", table->thread_enabled);
>       }
> 
>     @@ -719,6 +726,8 @@ static int do_smbios(struct cmd_tbl *cmdtp, int
>     flag, int argc,
>                      struct smbios3_entry *entry3 = entry;
> 
>                      table = (void *)(uintptr_t)entry3-
>      >struct_table_address;
>     +               smbios_version = ((u16)entry3->major_ver << 16) +
>     +                                (u16)entry3->minor_ver;
>                      snprintf(version, sizeof(version), "%d.%d.%d",
>                               entry3->major_ver, entry3->minor_ver,
>     entry3->doc_rev);
>                      table = (void *)(uintptr_t)entry3-
>      >struct_table_address;
>     -- 
>     2.51.0
>