[PATCH 2/2] smbios: Fallback to the default DT if sysinfo nodes are missing

[Simon Glass]

Hi Ilias,

On Thu, 29 Sept 2022 at 04:23, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Simon,
>
> On Thu, Sep 29, 2022 at 03:59:51AM -0600, Simon Glass wrote:
> > Hi,
> >
> > On Tue, 20 Sept 2022 at 05:10, Peter Robinson <pbrobinson at gmail.com> wrote:
> > >
> > > On Tue, Sep 6, 2022 at 2:44 PM Ilias Apalodimas
> > > <ilias.apalodimas at linaro.org> wrote:
> > > >
> > > > In order to fill in the SMBIOS tables U-Boot currently relies on a
> > > > "u-boot,sysinfo-smbios" compatible node.  This is fine for the boards
> > > > that already include such nodes.  However with some recent EFI changes,
> > > > the majority of boards can boot up distros, which usually rely on
> > > > things like dmidecode etc for their reporting.  For boards that
> > > > lack this special node the SMBIOS output looks like:
> > > >
> > > > System Information
> > > >         Manufacturer: Unknown
> > > >         Product Name: Unknown
> > > >         Version: Unknown
> > > >         Serial Number: Unknown
> > > >         UUID: Not Settable
> > > >         Wake-up Type: Reserved
> > > >         SKU Number: Unknown
> > > >         Family: Unknown
> > > >
> > > > This looks problematic since most of the info are "Unknown".  The DT spec
> > > > specifies standard properties containing relevant information like
> > > > 'model' and 'compatible' for which the suggested format is
> > > > <manufacturer,model>.  So let's add a last resort to our current
> > > > smbios parsing.  If none of the sysinfo properties are found,  we can
> > > > scan the root node for 'model' and 'compatible'.
> > >
> > > I don't think the information below all needs to go in the commit,
> > > maybe in the cover letter?
> > >
> > > > pre-patch dmidecode:
> > > > <snip>
> > > > Handle 0x0001, DMI type 1, 27 bytes
> > > > System Information
> > > >         Manufacturer: Unknown
> > > >         Product Name: Unknown
> > > >         Version: Unknown
> > > >         Serial Number: Unknown
> > > >         UUID: Not Settable
> > > >         Wake-up Type: Reserved
> > > >         SKU Number: Unknown
> > > >         Family: Unknown
> > > >
> > > > Handle 0x0002, DMI type 2, 14 bytes
> > > > Base Board Information
> > > >         Manufacturer: Unknown
> > > >         Product Name: Unknown
> > > >         Version: Unknown
> > > >         Serial Number: Not Specified
> > > >         Asset Tag: Unknown
> > > >         Features:
> > > >                 Board is a hosting board
> > > >         Location In Chassis: Not Specified
> > > >         Chassis Handle: 0x0000
> > > >         Type: Motherboard
> > > >
> > > > Handle 0x0003, DMI type 3, 21 bytes
> > > > Chassis Information
> > > >         Manufacturer: Unknown
> > > >         Type: Desktop
> > > >         Lock: Not Present
> > > >         Version: Not Specified
> > > >         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: Unspecified
> > > >         Contained Elements: 0
> > > > <snip>
> > > >
> > > > post-pastch dmidecode:
> > > > <snip>
> > > > Handle 0x0001, DMI type 1, 27 bytes
> > > > System Information
> > > >         Manufacturer: socionext,developer-box
> > > >         Product Name: Socionext Developer Box
> > > >         Version: Unknown
> > > >         Serial Number: Unknown
> > > >         UUID: Not Settable
> > > >         Wake-up Type: Reserved
> > > >         SKU Number: Unknown
> > > >         Family: Unknown
> > > >
> > > > Handle 0x0002, DMI type 2, 14 bytes
> > > > Base Board Information
> > > >         Manufacturer: socionext,developer-box
> > > >         Product Name: Socionext Developer Box
> > > >         Version: Unknown
> > > >         Serial Number: Not Specified
> > > >         Asset Tag: Unknown
> > > >         Features:
> > > >                 Board is a hosting board
> > > >         Location In Chassis: Not Specified
> > > >         Chassis Handle: 0x0000
> > > >         Type: Motherboard
> > > >
> > > > Handle 0x0003, DMI type 3, 21 bytes
> > > > Chassis Information
> > > >         Manufacturer: socionext,developer-box
> > > >         Type: Desktop
> > > >         Lock: Not Present
> > > >         Version: Not Specified
> > > >         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: Unspecified
> > > >         Contained Elements: 0
> > > > <snip>
> > > >
> > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > >
> > > Reviewed-by: Peter Robinson <pbrobinson at gmail.com>
> > > Tested-by: Peter Robinson <pbrobinson at gmail.com>
> > >
> > > > ---
> > > >  lib/smbios.c | 41 +++++++++++++++++++++++++++++++++++++++--
> > > >  1 file changed, 39 insertions(+), 2 deletions(-)
> >
> > I've thought about this a lot.
> >
> > As I mentioned earlier, we should require boards to add this
> > information when they enable GENERATE_SMBIOS_TABLE
> >
> > It is a simple patch for each board vendor and it solves the problem.
> > What we have here just masks it.
>
>
> Not entirely.  I think we just see the problem differently here.  I agree
> that the code here masks a problem (but only for *some* boards) and ideally
> we should go and add smbios nodes on the boards that miss it.  However we
> conveniently keep ignoring OF_BOARD here.  Until those things are documented
> in a spec and you can *demand* a previous bootloader to include it, we'll have
> boards that display "Unknown" all over the place.   Personally I don't
> think that's acceptable,  hence the last resort solution.

I think you mean OF_HAS_PRIOR_STAGE - we have an explicit Kconfig now.

We can easily make U-Boot halt if the info is not there but it is
needed. That will cause people to fix it for their board.

>
> I'd be much happier if we popped a warning on boards that miss the smbios
> node and are not compiling with OF_BOARD, explaining smbios will be
> disabled for them in the future,  while having the flexibility to not
> display values that make no sense.

How about just failing the build and producing an error, if people
enable the SMBIOS tables without the data? We could run with a warning
for a while if you like, then change it to an error.

Regards,
Simon