[PATCH 1/1] cmd: smbios: correct output for table type 4
Raymond Mao
raymondmaoca at gmail.com
Mon Nov 17 18:13:53 CET 2025
Hi Heinrich,
On Mon, Nov 17, 2025 at 11:43 AM Heinrich Schuchardt <
heinrich.schuchardt at canonical.com> wrote:
> 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.
>
>
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.
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
> >
>
>
More information about the U-Boot
mailing list