[PATCH 00/10] SMBIOS improvements

[Simon Glass]

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?

Regards,
Simon