Francois Toguo Fotso has posted comments on this change. (
Change subject: smbios: Add and adapt fields for version 3.2
Patch Set 5:
Patch Set 5:
the revert. Please, split out the functional
changes (e.g. the cpuid stuff around core/thread count)
into their own patch (it's not related to the SMBIOS
update, is it?). This change does a lot of things, and
I haven't spotted anything yet that is *required* for
From the 3.2 spec, core counts and thread counts don't seem to be optional.
The newly added procedure is populating them to comply to one of your previous comment:
" please don't add fields without giving them a reasonable value"
Well, core/thread count are not added by in this change,
so my comment doesn't apply to them. I was mostly refer-
ring to the added fields that were left untouched (at 0)
where 0 isn't a valid value. Sorry for the confusion.
Still, you could fix the cpuid code first, then add SMBIOS
3.2 stuff. I don't see why it would have to be part of the
I can remove it from this patch if you require it
and set the values to 0. Because the count generated by the code before this upgrade was
not accurate, at least not on the GKL Sparky platform.
I know the current code doesn't provide accurate values in
all cases. But in some cases it might. So why would you
want to set it to 0 in all cases?
Also, we are talking about an extension of existing fields.
With less than 256 cores, core_count2 must have the same
value as core_count. Whether or not the value of the latter
is accurate is an orthogonal issue.
Since the comment don't apply to the procedure then I am still confused.
|> Still, you could fix the cpuid code first, then add SMBIOS 3.2 stuff. I don't
see why it would have to be part of the same commit.
Removing the new core/tread procedure here and keeping everything else, along with the old
core count in this commit would achieve the same goal, wouldn't it? If that is your
request please confirm and I will do just that.
|> I was mostly referring to the added fields that were left untouched (at 0) where 0
isn't a valid value. Sorry for the confusion.
There are 3 added field which are set to 0, and 0 indeed is a valid value for each of
them. Please see these lines from the SMBIOS spec:
" A value of 0000h indicates the Memory Subsystem Controller Product ID is
" A value of 0000h indicates the Memory Subsystem Controller Manufacturer ID is
" A value of 0000h indicates the Module Product ID is unknown"
The FSP code have not yet been upgraded to 3.2 to popolate those memory fields
appropriately. Hence their "unknown" 0 values.
To view, visit https://review.coreboot.org/c/coreboot/+/32090
To unsubscribe, or for help writing mail filters, visit
Gerrit-Owner: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com>
Gerrit-Reviewer: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Francois Toguo Fotso
Gerrit-CC: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Comment-Date: Fri, 29 Mar 2019 17:33:04 +0000