[PATCH 00/10] SMBIOS improvements

[Simon Glass]

Hi Raymond,

On Tue, 10 Sept 2024 at 13:20, Raymond Mao <raymond.mao at linaro.org> wrote:
>
> 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.

OK. Let's try to avoid tables of strings.

Regards,
Simon