Re: [PATCH 00/10] SMBIOS improvements

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 4933 bytes --]

On Thu, Sep 19, 2024 at 04:13:02PM +0200, Simon Glass wrote:
> Hi,
> 
> On Sun, 15 Sept 2024 at 20:28, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sun, Sep 15, 2024 at 07:57:19PM +0200, Heinrich Schuchardt wrote:
> > > On 8/26/24 21:59, Tom Rini wrote:
> > > > On Mon, Aug 26, 2024 at 01:12:16PM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Mon, 26 Aug 2024 at 12:23, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Mon, Aug 26, 2024 at 11:58:54AM -0600, Simon Glass wrote:
> > > > > > > Hi Caleb,
> > > > > > >
> > > > > > > On Mon, 19 Aug 2024 at 17:03, Caleb Connolly <caleb.connolly@linaro.org> wrote:
> > > > > > > >
> > > > > > > > Hi Simon,
> > > > > > > >
> > > > > > > > > As a general comment, this is adding a load of code which is used by a
> > > > > > > > > lot of platforms. As more and more aarch64 platforms are created, this
> > > > > > > > > data grows. Why not use the devicetree for this hardware information?
> > > > > > > > > That is what it is for.
> > > > > > > >
> > > > > > > > This data does not belong in devicetree, the various system registers
> > > > > > > > exist to describe this information... Putting it in DT would be
> > > > > > > > duplicating it.
> > > > > > >
> > > > > > > I am not wanting to duplicate info which can be read from system registers.
> > > > > > >
> > > > > > > >
> > > > > > > > Using DT for this would additionally require having bindings accepted
> > > > > > > > upstream and for all SoCs to add them. To what end?
> > > > > > >
> > > > > > > To get the correct information in there. How are boards supposed to
> > > > > > > add SMBIOS info? Do we end up creating a whole infra in U-Boot just
> > > > > > > for the driver to read it out? It just doesn't make any sense to me...
> > > > > > >
> > > > > > > Let's put hardware info in the DT where it belongs.
> > > > > >
> > > > > > I'm a little confused here because of some older threads on this overall
> > > > > > topic. Part of the issue here is that in user space, "everyone" has
> > > > > > SMBIOS-based tooling today, and wants to have that work, rather than
> > > > > > inventing new tooling or modify existing tooling. And you were concerned
> > > > > > I thought that we had tied SMBIOS too much to EFI being present when
> > > > > > indeed it should be possible to pass the location along to the OS
> > > > > > without EFI, but at the time Linux at least only supported that notion
> > > > > > on MIPS I think?
> > > > >
> > > > > That is a whole other concern I have, that we are perpetuating this
> > > > > legacy format which is a real pain to work with, when we already have
> > > > > devicetree. Let's leave that issue aside as I have not detected any
> > > > > interest in solving that problem, or even any agreement that it is a
> > > > > problem.
> > > >
> > > > OK, yes, lets set that aside.
> > > >
> > > > > But for this particular series, I am just wanting to get the correct
> > > > > info in there. Having the CPU-detection code provide an opinion about
> > > > > what type of chassis is in use (just to take an example, the patch
> > > > > pieces I highlighted have been dropped from the email I am replying
> > > > > to) just seems a bit daft to me. Only the board vendor would know that
> > > > > info.
> > > >
> > > > Yes, I agree the detection should be reworked a good bit as some
> > > > information will be board design specific while others SoC specific. And
> > > > we should avoid adding many unused at run time strings to all platforms
> > > > that enable this too (looking at all the CPU vendor related stuff).
> > > >
> > >
> > > I doubt on productive machines there will be much use of U-Boot's smbios
> > > command use. It is more a developer tool.
> 
> Many commands fall into that category.

Yes, there is a trade-off to be made.

> > > For reading all the details we currently have
> > > lib/efi_loader/smbiosdump.efi which can dump the SMBIOS table to a file
> > > that dmidecode can read.
> > >
> > > Maybe instead of adding more and more decoding logic into the U-Boot
> > > smbios command we should add an smbios sub-command to dump to a file.
> > > This would be less of a hassle than running an EFI program for the same
> > > purpose.
> >
> > Sounds like a good idea to me.
> 
> I would like to see this series land in U-Boot as I believe it is very
> helpful for seeing what the table looks like. Dumping to a file which
> then needs to be decoded is not as convenient. We may also find it
> easier to add tests for SMBIOS.

And I don't look forward to the seemingly inevitable parsing bug means
CVE assigned. If the command is broadly enabled then "everyone" gets to
worry about it, and if it's narrowly enabled have we really gotten
better than the options of dump to file, parse elsewhere or load a
parser and run it?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]