From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 11DB5C433FE for ; Fri, 30 Sep 2022 14:25:28 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id A89B484D47; Fri, 30 Sep 2022 16:25:26 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="SeB/Yv2M"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 29FE384D68; Fri, 30 Sep 2022 16:25:24 +0200 (CEST) Received: from mail-qv1-xf29.google.com (mail-qv1-xf29.google.com [IPv6:2607:f8b0:4864:20::f29]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 5B74584C30 for ; Fri, 30 Sep 2022 16:25:20 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qv1-xf29.google.com with SMTP id l14so2876509qvq.8 for ; Fri, 30 Sep 2022 07:25:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date; bh=RY7o31m4tLbpmsznr4thcGgopFH+tAYtTjOIgGESE7s=; b=SeB/Yv2M7+DosJ2v0KTQqPEhuMiwCXH4cgKCYSF92SPVXcFJIn2jodQmi22B5x2Qgr GOlfUxwLc6dlZutGL9ZvmdQhRXlVWiB6Ir3vABMzHSVuZIjq3Wl6AcitfMzdPYS5Napu bINW/4IyhsQuDqRGbDswy2hRB3z692Y6Cm4FA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date; bh=RY7o31m4tLbpmsznr4thcGgopFH+tAYtTjOIgGESE7s=; b=ivbydCS9DauWPV52BJCEH4zKzCbNpfC8H3fJjvKZG9Ui9RSmMaanODo1LQvvsLObMO qE59ZUhJZIEpzmBRX0bnfCaaPU/bjzYpaS89w6tWdINmqxXNrwkwLw3MgtywCHYr/06Z T7xrOpnBElZNB1g7DIdWKhRrmm/cxm0nEmHxmkdLvORQHOvKrJJn9Ges9kyjYlTvZknu qwiNoyhFyo99cShH8ooAqNe6IhvlcPaIf8+ULZvyEyNlNKqvriTDckw20qL1rDL6H8l3 2em8ul4cIL+qyqVIbSaJiEb9jvlD3BTYjfrnOY+qmnNR0bCqxqgmGE9E87Xgu3iify12 mbTg== X-Gm-Message-State: ACrzQf0DjcwZ+qrdEHyzWguEyLFOX7QTAhERQxUv4dJTlGXkGISaroQ9 JUI4uHcPzoHL5RGn7aWgL5HVJw== X-Google-Smtp-Source: AMsMyM7kC0D8Dkp/mEKozPSk3MGH/o73YRaYupQqV3jZwIAYi0er9sAUnhDukLXNtXPiwUzzqHdwrQ== X-Received: by 2002:a05:6214:27ee:b0:4af:95d7:c899 with SMTP id jt14-20020a05621427ee00b004af95d7c899mr6876679qvb.75.1664547919078; Fri, 30 Sep 2022 07:25:19 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b00-6400-0186-ea3a-a82b-3964.res6.spectrum.com. [2603:6081:7b00:6400:186:ea3a:a82b:3964]) by smtp.gmail.com with ESMTPSA id l18-20020a05620a28d200b006ce813bb306sm2685163qkp.125.2022.09.30.07.25.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 30 Sep 2022 07:25:18 -0700 (PDT) Date: Fri, 30 Sep 2022 10:25:16 -0400 From: Tom Rini To: Sean Anderson Cc: Simon Glass , Ilias Apalodimas , U-Boot Mailing List , Heinrich Schuchardt , Peter Robinson Subject: Re: [PATCH 1/2] smbios: Simplify reporting of unknown values Message-ID: <20220930142516.GT3044094@bill-the-cat> References: <20220906134426.53748-1-ilias.apalodimas@linaro.org> <0e250d37-de96-330b-4666-af07fd65e2e8@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="SqceQ39T/BkJMppc" Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean --SqceQ39T/BkJMppc Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Sep 29, 2022 at 10:02:48AM -0400, Sean Anderson wrote: > On 9/29/22 05:59, Simon Glass wrote: > > Hi, > >=20 > > On Wed, 28 Sept 2022 at 22:34, Sean Anderson wrote: > > >=20 > > > On 9/26/22 06:56, Ilias Apalodimas wrote: > > > > Hi Sean > > > >=20 > > > > On Sat, 17 Sept 2022 at 19:55, Sean Anderson wr= ote: > > > > >=20 > > > > > On 9/16/22 16:30, Ilias Apalodimas wrote: > > > > > > Hi Simon, > > > > > >=20 > > > > > > [...] > > > > > >=20 > > > > > > > > Signed-off-by: Ilias Apalodimas > > > > > > > > --- > > > > > > > > lib/smbios.c | 17 +++-------------- > > > > > > > > 1 file changed, 3 insertions(+), 14 deletions(-) > > > > > > >=20 > > > > > > > Perhaps a better fix is to drop the smbios info? > > > > > >=20 > > > > > > Unfortunately there's a ton of userspace tools still using it. = So I think > > > > > > we still need it > > > > > >=20 > > > > > > >=20 > > > > > > > What upstream projects use this information to show things to= the > > > > > > > user? You showed a screenshot of some sort of system-info app= =2E We > > > > > > > could teach it about falling back to the device tree. That wa= y we are > > > > > > > not adding fake information to SMBIOS. > > > > > > >=20 > > > > > >=20 > > > > > > What's fake here? The model and compatible are taken directly = =66rom the DT > > > > > > and that should be accurate. I'd rather fix the DT if that's p= roblematic. > > > > > > 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 anyway. > > > > >=20 > > > > > > Manufacturer: socionext,developer-box > > > > > > Product Name: Socionext Developer Box > > > > >=20 > > > > > Well, firstly, the manufacturer is "Socionext", not > > > > > "socionext,developer-box". Compatibles are not suitable for > > > > > user-visible identifiers. The product name should also be somethi= ng like > > > > > "Socionext Developerbox" or maybe "SynQuacer E-series", but this = more of > > > > > a "bug" in the devicetree model property. > > > >=20 > > > > Yea as I said we can get rid of the everything after the ',' on the > > > > compatible node. Ideally if vendors followed the DT spec, we could > > > > also just use manufacturer node, the reality is that we can't thou= gh. > > >=20 > > > This is another one of the problems with this approach. There's no > > > consistency in existing device trees, because at most this info is > > > printed in the boot log. > > >=20 > > > > The whole point of the patchset is provide something reasonable > > > > without having to add a .dtsi smbios node for all our devices. We = can > > > > then go back to fixing the DT with proper values if it's a DT "bug". > > > > >=20 > > > > > Second, these identifiers are not suitable for all structures you= want > > > > > to use it for. For example, the chassis is really a "INWIN indust= rial PC > > > > > case: MicroATX mini-tower case IW-BK623/300-H E USB 3.0 Black wit= h 300W > > > > > SFX power supply" [1]. I would describe this as something like > > > >=20 > > > > The chassis isn't even addressed in the series. IIRC it's currently > > > > hardcoded in smbios.c. > > >=20 > > > You showed it as different in the commit message. > > >=20 > > > > >=20 > > > > > 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 > > > > >=20 > > > > > The exact values are not particularly important, but I would cert= ainly > > > > > 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 usi= ng a > > > > > different case? > > > >=20 > > > > But the chassis isn't even addressed in the series? Again I am mos= tly > > > > interested in a sane fallback for device and manufacturer. > > >=20 > > > ditto > > >=20 > > > > >=20 > > > > > [1] https://www.96boards.org/documentation/enterprise/developerbo= x/hardware-docs/MN04-00002-3E.pdf > > > > >=20 > > > > > > > Also, SMBIOS is a legacy thing and a PITA to work with. How a= bout we > > > > > > > use the device tree binding for the same info: > > > > > > >=20 > > > > > > > smbios { > > > > > > > compatible =3D "u-boot,sysinfo-smbios"; > > > > > > >=20 > > > > > > > smbios { > > > > > > > system { > > > > > > > manufacturer =3D "pine64"; > > > > > > > product =3D "rock64_rk3328"; > > > > > > > }; > > > > > > >=20 > > > > > > > baseboard { > > > > > > > manufacturer =3D "pine64"; > > > > > > > product =3D "rock64_rk3328"; > > > > > > > }; > > > > > > >=20 > > > > > > > chassis { > > > > > > > manufacturer =3D "pine64"; > > > > > > > product =3D "rock64_rk3328"; > > > > > > > }; > > > > > > > }; > > > > > > > }; > > > > > > >=20 > > > > > > > This is easy to parse and gets us away from all this legacy j= unk that > > > > > > > we don't need. > > > > > >=20 > > > > > > That's the exact opposite of the patch description. Most of th= ese 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 e= xample a DT > > > > > > handed over by the previous stage bootloader would not include = these nodes. > > > > >=20 > > > > > I agree. I think a better example would fill in these fields with > > > > > descriptive values. > > > >=20 > > > > We are off to a chicken and egg problem now. Can you provide U-Boot > > > > with a 'configuration' DT, which would be disjoint from the DT that > > > > describes hardware? > > >=20 > > > Sorry, I misread the context there. > > >=20 > > > I still don't think this is the right approach for this... better to = fix > > > the prior stage's devicetree. > > >=20 > > > > >=20 > > > > > > 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 alre= ady there > > > > > > and defined on the DT spec at least for the simplistic values. > > > > >=20 > > > > > IMO SYS_VENDOR and SYS_BOARD are more descriptive than the device= tree > > > > > values, but neither is good... > > > >=20 > > > > Didn't we use to do that? IOW fill in smbios nodes based on Kconfig > > > > values. But then we moved away from that in favor of the > > > > sysinfo-smbios node, but a very small amount of boards got converte= d. > > >=20 > > > I mean that SYS_VENDOR and SYS_BOARD have content which more closely > > > matches the content of the SMBios tables, not that we should use them > > > ("neither is good..."). > > >=20 > > > > >=20 > > > > > 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 sus= pect > > > > > most never boot anything EFI. > > > >=20 > > > > I don't see how that's relevant? If someone for any reason enables > > > > smbios it shouldn't report always "Unknown". > > >=20 > > > I'm mostly trying to figure out how much effort it would be to just a= dd > > > nodes for all devices which boot with SMBios. I know that most boards > > > which have it enabled don't actually use it, since it's enabled by > > > default. > >=20 > > It is a patch like this: > >=20 > > https://patchwork.ozlabs.org/project/uboot/patch/20220929001520.9095-1-= christian@kohlschutter.com/ > >=20 > > I just found out that this option is enabled for hundreds of boards. >=20 > I first noticed it when doing the K210 and wondering why I had EFI > enabled. >=20 > > Perhaps the solution is to turn it off unless the board enables it? >=20 > But how do we determine if the board enables it? Since it was on by > default, it's not so easy. One way would be to look at the boards which > use bootefi, but from what I can tell, that's enabled by distroboot. > Which has a similar problem where include/configs/mycpu_common.h might > enable it, but (most of) the boards for that cpu might not care. I think the point that's trying to be made in the thread is that this bit of code is common and widely used / visible as it's part of the regular commodity Linux distro "tell the user useful things" tools. So it should default to be as correct as can be. --=20 Tom --SqceQ39T/BkJMppc Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmM2/EkACgkQFHw5/5Y0 tywj4wv8D7hsmz1YhUCDYcZwFG05avt199ffo/nyiEV++SJa9yYk4bMwPPwvLNNc FTMLKHgSfGkdD5U7a3o7V4hhenFrBtFsXLbg0siGuOKwCDpv2wFe6C9OGQr9sR7q Lj2hh8upWI7Pwd0CebTPeScsE0dhui1InaGqeLomTCc4I/q9RUuCOvHy1mg3jubu johBYh5citQfxZtFqihJOS2E4dugY/Q6iM7tGNS8CPAYZQZacbJn/FAqG+Fm1wq3 rVFWhroe7MzecZ+L4YjQp+sqrFxgkYi75EqsOgsmvUQK41Jz5G6DtznPcvjnkPKm iMJQkcnXNUIhoUIaMNvmVg6dfeHDuNWVwA3OZfxq8bxZayi195f/r4ZBrElYRSJ/ 2eYEBg+4EsmBmWE5pZpdsvwdKAIESOVd+uQkSciZU/aWUVeIkeFllYRiEtZmdCGt W7P1bGCGtDxTtWijycKZwyEDoivhr5bxMd4LKjiE/racPbHUvRkqz2j7bLBzqApu B1dAIBmp =VwLt -----END PGP SIGNATURE----- --SqceQ39T/BkJMppc--