Hi Heinrich, On Mon, Nov 17, 2025 at 11:43 AM Heinrich Schuchardt < [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]>> 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]>> > > --- > > 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 > > > >
