On 11/17/25 18:13, Raymond Mao wrote:
Hi Heinrich,
On Mon, Nov 17, 2025 at 11:43 AM Heinrich Schuchardt
<[email protected]
<mailto:[email protected]>> wrote:
On 11/17/25 16:48, Raymond Mao wrote:
> Hi Heinrich,
>
> On Sun, Nov 16, 2025 at 5:39 AM Heinrich Schuchardt
> <[email protected]
<mailto:[email protected]>
> <mailto:[email protected]
<mailto:[email protected]>>> 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
> <[email protected]
<mailto:[email protected]>
> <mailto:[email protected]
<mailto:[email protected]>>>
> ---
> 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 <http://www.dmtf.org/sites/default/files/
standards/documents/DSP0134_3.8.0.pdf>
> <https://www.dmtf.org/sites/default/files/standards/documents/
<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.
But it also says: " 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."
So the "old" fields might be with the same values as the "new" ones, or
they might be "FFh".
That is the reason I believe it is still valuable to print and show
whether it is aligned to the spec.
For values below 256 the values will be equal. Why do you want to print
redundant information?
Best regards
Heinrich
But anyway, it is not really a big deal to print them or not in cmd - it
is just for demo/debug purposes, what I wanted to highlight is the logic
to exclude fields for old versions < 0x0300 should be needed in lib/smbios.
Regards,
Raymond
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
>
