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

[Simon Glass]

Hi Tom,

On Wed, 13 Dec 2023 at 13:56, Tom Rini <trini at konsulko.com> wrote:
>
> On Wed, Dec 13, 2023 at 01:41:04PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 13 Dec 2023 at 13:17, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Wed, Dec 13, 2023 at 12:51:33PM -0700, Simon Glass wrote:
> > > > Hi Ilias,
> > > >
> > > > On Thu, 7 Dec 2023 at 02:19, 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>. Unfortunately the 'model' string found in DTs is
> > > > > usually lacking the manufacturer so we can't use it for both
> > > > > 'Manufacturer' and 'Product Name' SMBIOS entries reliably.
> > > > >
> > > > > So let's add a last resort to our current smbios parsing.  If none of
> > > > > the sysinfo properties are found, scan for those information in the
> > > > > root node of the device tree. Use the 'model' to fill the 'Product
> > > > > Name' and the first value of 'compatible' for the 'Manufacturer', since
> > > > > that always contains one.
> > > > >
> > > > > pre-patch:
> > > > > Handle 0x0001, DMI type 1, 27 bytes
> > > > > System Information
> > > > >         Manufacturer: Unknown
> > > > >         Product Name: Unknown
> > > > >         Version: Unknown
> > > > >         Serial Number: 100000000bb24ceb
> > > > >         UUID: 30303031-3030-3030-3061-613234636435
> > > > >         Wake-up Type: Reserved
> > > > >         SKU Number: Unknown
> > > > >         Family: Unknown
> > > > > [...]
> > > > >
> > > > > and post patch:
> > > > > Handle 0x0001, DMI type 1, 27 bytes
> > > > > System Information
> > > > >         Manufacturer: raspberrypi
> > > > >         Product Name: Raspberry Pi 4 Model B Rev 1.1
> > > > >         Version: Unknown
> > > > >         Serial Number: 100000000bb24ceb
> > > > >         UUID: 30303031-3030-3030-3061-613234636435
> > > > >         Wake-up Type: Reserved
> > > > >         SKU Number: Unknown
> > > > >         Family: Unknown
> > > > > [...]
> > > > >
> > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > > > ---
> > > > > Simon, I'll work with tou on the refactoring you wanted and
> > > > > remove the EFI requirement of SMBIOS in !x86 platforms.
> > > > > Since this code has no tests, I'll add some once the refactoring
> > > > > is done
> > > > >
> > > > > Changes since v2:
> > > > > - Spelling mistakes
> > > > > - rebase on top of patch #1 changes
> > > > > Changes since v1:
> > > > > - Tokenize the DT node entry and use the appropriate value instead of
> > > > >   the entire string
> > > > > - Removed Peters tested/reviewed-by tags due to the above
> > > > >  lib/smbios.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++---
> > > > >  1 file changed, 89 insertions(+), 5 deletions(-)
> > > >
> > > > Can we add a Kconfig to enable this?
> > >
> > > Why? This is the default option that we want moving forward in order to
> > > get the fields populated.
> >
> > The model name is fine. The manufacturer is in lower case (using the
> > DT vendor name), so not in the right format.
> >
> > It only populates two fields, so far as I can tell.
>
> Maybe we need to turn this discussion on its head slightly. What do you
> want to do to get SMBIOS fields to be widely populated with
> generally-correct information? What we have been doing has seen very
> little adoption so we need something else.

Well, who or what is actually using this info? Is the problem just
that it doesn't actually matter, so no one bothers? I'm just not sure.
The fact that on ARM you cannot pass it to the kernel without EFI
might be inhibiting its usefulness, too? Usefulness is the key
question for me.

Anyway, my suggestion (once we understand why we care about SMBIOS
tables) is patches like we just saw from rockpro64, perhaps even with
a few more fields filled out. This series seems like a backup for
board maintainers that don't care, which is fine, but we should at
least be able to disable it.

Regards,
Simon