On 9/16/22 16:30, Ilias Apalodimas wrote:
Hi Simon,

[...]

Signed-off-by: Ilias Apalodimas <ilias.apalodi...@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

Reply via email to