Re: [PATCH 1/2] smbios: Simplify reporting of unknown values

[Sean Anderson <seanga2@gmail.com>]

On 9/16/22 16:30, Ilias Apalodimas wrote:
> Hi Simon,
> 
> [...]
> 
>>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>>> ---
>>>   lib/smbios.c | 17 +++--------------
>>>   1 file changed, 3 insertions(+), 14 deletions(-)
>>
>> Perhaps a better fix is to drop the smbios info?
> 
> Unfortunately there's a ton of userspace tools still using it.  So I think
> we still need it
> 
>>
>> What upstream projects use this information to show things to the
>> user? You showed a screenshot of some sort of system-info app. We
>> could teach it about falling back to the device tree. That way we are
>> not adding fake information to SMBIOS.
>>
> 
> What's fake here?  The model and compatible are taken directly from the DT
> and that should be accurate.  I'd rather fix the DT if that's problematic.
> What would make sense for me to change is take the first token of the
> compatible node instead of the entire string as it's format is expected to
> be <manufacturer, model> anyway.

>         Manufacturer: socionext,developer-box
>         Product Name: Socionext Developer Box

Well, firstly, the manufacturer is "Socionext", not
"socionext,developer-box". Compatibles are not suitable for
user-visible identifiers. The product name should also be something like
"Socionext Developerbox" or maybe "SynQuacer E-series", but this more of
a "bug" in the devicetree model property.

Second, these identifiers are not suitable for all structures you want
to use it for. For example, the chassis is really a "INWIN industrial PC
case: MicroATX mini-tower case IW-BK623/300-H E USB 3.0 Black with 300W
SFX power supply" [1]. I would describe this as something like

Handle 0x0003, DMI type 3, 21 bytes
Chassis Information
         Manufacturer: INWIN
         Type: Mini Tower
         Lock: Not Present
         Version: Unknown
         Serial Number: Not Specified
         Asset Tag: Not Specified
         Boot-up State: Safe
         Power Supply State: Safe
         Thermal State: Safe
         Security Status: None
         OEM Information: 0x00000000
         Height: Unspecified
         Number Of Power Cords: 1
         Contained Elements: 0

The exact values are not particularly important, but I would certainly
classify a manufacturer of "socionext,developer-box" as fake. We might
not even know what the chassis is; what's to stop a user from using a
different case?

[1] https://www.96boards.org/documentation/enterprise/developerbox/hardware-docs/MN04-00002-3E.pdf

>> Also, SMBIOS is a legacy thing and a PITA to work with. How about we
>> use the device tree binding for the same info:
>>
>>      smbios {
>>          compatible = "u-boot,sysinfo-smbios";
>>
>>          smbios {
>>              system {
>>                  manufacturer = "pine64";
>>                  product = "rock64_rk3328";
>>              };
>>
>>              baseboard {
>>                  manufacturer = "pine64";
>>                  product = "rock64_rk3328";
>>              };
>>
>>              chassis {
>>                  manufacturer = "pine64";
>>                  product = "rock64_rk3328";
>>              };
>>          };
>>      };
>>
>> This is easy to parse and gets us away from all this legacy junk that
>> we don't need.
> 
> That's the exact opposite of the patch description.  Most of these info are
> already included in the DT in it's standard properties.  So if U-Boot ends
> up with a DT without these we get a usable smbios table.  For example a DT
> handed over by the previous stage bootloader would not include these nodes.

I agree. I think a better example would fill in these fields with
descriptive values.

> As far as sysinfo-smbios node is concerned,  it's only present in 13
> boards, so it's not like  it's used by the majority of boards.  Yes we
> could fix them, but imho we are better off re-using what's already there
> and defined on the DT spec at least for the simplistic values.

IMO SYS_VENDOR and SYS_BOARD are more descriptive than the devicetree
values, but neither is good...

How many boards do we have which actually use the SMBIOS tables? There
are a lot of boards with EFI_LOADER enabled by default, but I suspect
most never boot anything EFI.

--Sean