Hi Tom and Heinrich, On Thu, 19 Sept 2024 at 13:48, Tom Rini <tr...@konsulko.com> wrote:
> On Thu, Sep 19, 2024 at 04:13:02PM +0200, Simon Glass wrote: > > Hi, > > > > On Sun, 15 Sept 2024 at 20:28, Tom Rini <tr...@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 <tr...@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.conno...@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? > > Currently the code for parsing the SMBIOS tables really helps debugging before an external parser is ready for use. I think we can keep it until a dump tool is in the project, at that time we can just simply disable the CMD_SMBIOS config for the concern of code size. Regards, Raymond