[PATCH 1/1] cmd: smbios: correct output for table type 4
Raymond Mao
raymondmaoca at gmail.com
Fri Nov 28 17:48:52 CET 2025
Hi Heinrich,
Sorry for the late reply.
On Mon, Nov 17, 2025 at 12:27 PM Heinrich Schuchardt <
heinrich.schuchardt at canonical.com> wrote:
> 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?
>
>
Actually, I have a better idea to use 'table->hdr.length' for version
control instead of introducing the version explicitly.
The 'table->hdr.length' is tied to the version and it is already being used
for type 1.
I sent a patch [1] to extend the usage of 'table->hdr.length' to all types
we have and it covers all logics from 2.0 to 3.7+,
and it looks neat.
Please see if that matches your cases.
[1]
https://lore.kernel.org/u-boot/20251128163535.2301653-7-raymondmaoca@gmail.com/T/#u
Regards,
Raymond
> 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) +
>
A typo of '<< 8' ? - smbios_version is a u16.
> > > + (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