[PATCH 00/10] SMBIOS improvements

[Raymond Mao]

Hi Simon,

On Tue, 10 Sept 2024 at 14:44, Simon Glass <sjg at chromium.org> wrote:

> Hi Raymond,
>
> On Tue, 3 Sept 2024 at 10:07, Raymond Mao <raymond.mao at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Sat, 17 Aug 2024 at 11:58, Simon Glass <sjg at chromium.org> wrote:
> >>
> >> Hi Raymond,
> >>
> >> On Fri, 16 Aug 2024 at 09:47, Raymond Mao <raymond.mao at linaro.org>
> wrote:
> >> >
> >> > Motivations for changes:
> >> > Current SMBIOS library and command-line tool is not fully matching
> with
> >> > the requirements:
> >> > 1. Missing support for other mandatory types (#7, #9, #16, #17, #19).
> >> > 2. Only a few platforms support SMBIOS node from the device tree.
> >> > 3. Values of some fields are hardcoded in the library other than
> fetching
> >> >    from the device hardware.
> >> > 4. Embedded data with dynamic length is not supported (E.g. Contained
> >> >    Object Handles in Type #2 and Contained Elements in Type #3)
> >> >
> >> > Changes:
> >> > 1. Refactor the SMBIOS library and command-line tool to better align
> with
> >> >    the SMBIOS spec.
> >> > 2. Create an arch-specific driver for all aarch64-based platforms to
> fetch
> >> >    SMBIOS private data from the device hardware.
> >> > 3. Create a sysinfo driver to poppulate platform SMBIOS private data.
> >> > 4. Put device tree SMBIOS node as a fallback solution when SMBIOS
> data is
> >> >    missing from sysinfo driver.
> >> > 5. Add support for Type #7 (Cache Information) and link its handles to
> >> >    Type #4.
> >> >
> >> > Once this patch is acceptted, subsequent patch sets will add other
> missing
> >> > types (#9, #16, #17, #19).
> >> >
> >> > Raymond Mao (10):
> >> >   sysinfo: Add sysinfo API for accessing data area
> >> >   sysinfo: Add sysinfo driver and data structure for SMBIOS
> >> >   smbios: Refactor SMBIOS library
> >> >   smbios: ignore the non-existence of platform sysinfo detect
> >> >   armv8: Add arch-specific sysinfo driver
> >> >   sysinfo: Add sysinfo driver for SMBIOS type 7
> >> >   smbios: Add support to SMBIOS type 7
> >> >   armv8: Add sysinfo driver for cache information
> >> >   configs: Enable sysinfo for QEMU Arm64
> >> >   tests: update smbios pytest
> >> >
> >> >  arch/arm/cpu/armv8/Makefile      |   5 +
> >> >  arch/arm/cpu/armv8/sysinfo.c     | 391 ++++++++++++++++++++++++++
> >> >  cmd/smbios.c                     | 350 ++++++++++++++++++++++-
> >> >  configs/qemu_arm64_defconfig     |   2 +
> >> >  drivers/misc/Kconfig             |   2 +-
> >> >  drivers/sysinfo/Makefile         |   1 +
> >> >  drivers/sysinfo/smbios_plat.c    | 442 +++++++++++++++++++++++++++++
> >> >  drivers/sysinfo/smbios_plat.h    | 131 +++++++++
> >> >  drivers/sysinfo/sysinfo-uclass.c |  20 ++
> >> >  include/smbios.h                 | 240 ++++++++++++++--
> >> >  include/sysinfo.h                | 124 ++++++++-
> >> >  lib/Makefile                     |   2 +
> >> >  lib/smbios.c                     | 461
> ++++++++++++++++++++++++++-----
> >> >  test/py/tests/test_smbios.py     |   2 +-
> >> >  14 files changed, 2058 insertions(+), 115 deletions(-)
> >> >  create mode 100644 arch/arm/cpu/armv8/sysinfo.c
> >> >  create mode 100644 drivers/sysinfo/smbios_plat.c
> >> >  create mode 100644 drivers/sysinfo/smbios_plat.h
> >> >
> >> > --
> >> > 2.25.1
> >> >
> >>
> >> 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.
> >>
> >> Some of the information detected makes sense, such as cache setup, but
> >> some of it seems like an approximation, or is missing, but suggests it
> >> is authoritative.
> >>
> > The idea is that precise information can still come from dt (if the node
> exists,
> > but as a fact, not many platforms have it up to now).
> > When it does not exist, system drivers provides the information as much
> as
> > it can (some comes from registers, some comes from generic
> strings/enums).
> > So it is not assumed that each vendor to add their code but just uses the
> > arch-specific code in this series - if vendors want precise information
> they can
> > still add into the dt.
>
> I fully understand what you are doing. I just don't think it is a
> great idea. We should have a clear boundary between:
>
> 1. things which are part of the hardware (although not explicitly in
> the devicetree) and can be probed
> 2. things which we can only guess at
> 3. grey area
>
> I am very happy with 1). It is just 2) that I want to avoid.
>
> The grey area is where your series adds lots of strings for different
> hardware...that just seems like code-size bloat to me.
>
> How about working on a devicetree binding for this stuff? Or perhaps
> add the info to the boards you care about?
>
> What I planned to do is to have a general one for arm64, then all the
arm64 boards missing the dt
node can leverage it.
Regarding to 2) you pointed out, maybe we can leave them to the dt later,
my patch can go first with
those architectural common stuffs, for example, cache information.
I will think about it before posting the next version.

Regards,
Raymond