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

[Tom Rini]

On Wed, Dec 13, 2023 at 03:22:25PM -0700, Simon Glass wrote:
> 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.

Users are using this information. Peter has explained that Fedora has
been carrying Ilias' original version of this patch since it was posted
so that bug reports say (something close enough and that can be tracked
down) what device they're on, rather than "Unknown" / "Unknown Product".

> 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.

I'm setting aside the discussion of how one will tell the kernel where
the SMBIOS is, outside of EFI as it's not a U-Boot problem and belongs
on other lists.

> 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.

The issue is that this series adds, for free, useful and sufficiently
correct information in the common cases. The "for free" is a huge
feature too. Drawing on the rockpro64 patch, I would suggest seeing if
/version (or /hardware-rev or whatever) is something Rob is interested
in adding so it can be something filled out or not and then pulled from
DT instead of a new set of DT nodes that look a whole lot like the
existing DT nodes and properties that already exist.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20231213/dbd40014/attachment.sig>