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