Re: [PATCH v2 6/8] armv8: Add generic smbios information into the device tree

[Heinrich Schuchardt]

Am 29. Oktober 2024 16:45:44 MEZ schrieb Simon Glass <sjg at chromium.org>:
>On Mon, 28 Oct 2024 at 21:00, Raymond Mao <raymond.mao at linaro.org> wrote:
>>
>> Hi Simon,
>>
>> On Mon, 28 Oct 2024 at 13:05, Simon Glass <sjg at chromium.org> wrote:
>>>
>>> Hi Raymond,
>>>
>>> On Tue, 22 Oct 2024 at 22:07, Raymond Mao <raymond.mao at linaro.org> wrote:
>>> >
>>> > Add common smbios information that can be used by all armv8
>>> > platforms and set it as default for qemu-arm64.
>>> > From now smbios library can load values from here for those fields
>>> > doesn't exist in the sysinfo driver.
>>> >
>>> > To run this with QEMU arm64, please dump the generated DTB
>>> > from QEMU first, merge it with the one we build and then re-run
>>> > QEMU with the merged DTB.
>>> > ```
>>> > qemu-system-arm -machine virt -machine dumpdtb=qemu.dtb
>>> > cat  <(dtc -I dtb qemu.dtb) <(dtc -I dtb ./dts/dt.dtb | \
>>> >   grep -v /dts-v1/) | dtc - -o merged.dtb
>>> > qemu-system-arm -machine virt -nographic -bios u-boot.bin \
>>> >   -dtb merged.dtb
>>>
>>> and please talk to Peter about accepting my patch[1]
>>>
>>>
>>> > ```
>>> > For details please take reference on dt_qemu.rst
>>> >
>>> > Signed-off-by: Raymond Mao <raymond.mao at linaro.org>
>>> > ---
>>> > Changes in v2
>>> > - Initial patch.
>>> >
>>> >  arch/arm/dts/qemu-arm64.dts      |  4 ++
>>> >  arch/arm/dts/smbios_generic.dtsi | 82 ++++++++++++++++++++++++++++++++
>>> >  2 files changed, 86 insertions(+)
>>> >  create mode 100644 arch/arm/dts/smbios_generic.dtsi
>>> >
>>> > diff --git a/arch/arm/dts/qemu-arm64.dts b/arch/arm/dts/qemu-arm64.dts
>>> > index 096b3910728..95fcf53ed74 100644
>>> > --- a/arch/arm/dts/qemu-arm64.dts
>>> > +++ b/arch/arm/dts/qemu-arm64.dts
>>> > @@ -7,5 +7,9 @@
>>> >
>>> >  /dts-v1/;
>>> >
>>> > +#if defined(CONFIG_SYSINFO_SMBIOS) && !defined(QFW_SMBIOS)
>>> > +#include "smbios_generic.dtsi"
>>> > +#endif
>>> > +
>>> >  / {
>>> >  };
>>> > diff --git a/arch/arm/dts/smbios_generic.dtsi b/arch/arm/dts/smbios_generic.dtsi
>>> > new file mode 100644
>>> > index 00000000000..c9f07283403
>>> > --- /dev/null
>>> > +++ b/arch/arm/dts/smbios_generic.dtsi
>>> > @@ -0,0 +1,82 @@
>>> > +// SPDX-License-Identifier: GPL-2.0+
>>> > +/*
>>> > + * Default SMBIOS information for Arm64 platforms
>>> > + *
>>> > + * Copyright (c) 2024 Linaro Limited
>>> > + * Author: Raymond Mao <raymond.mao at linaro.org>
>>> > + */
>>> > +#include <config.h>
>>> > +#include <smbios_def.h>
>>> > +
>>> > +/ {
>>> > +       smbios {
>>> > +               compatible = "u-boot,sysinfo-smbios";
>>> > +
>>> > +               smbios {
>>> > +                       system {
>>> > +                               manufacturer = CONFIG_SYS_VENDOR;
>>> > +                               product = CONFIG_SYS_BOARD;
>>> > +                               version = "Not Specified";
>>> > +                               serial = "Not Specified";
>>>
>>> Does it make sense to add these strings? Perhaps if the property is missing in the DT, it should be missing in the SMBIOS table? Is 'Not Specified' a special string in the SMBIO spec?


The correct way to handle unknown values is not to provide any string at all. In the SMBIOS table a string ID number of zero indicates that the value is unknown and dmidecode will show this correctly.

This is described in the SMBIOS spec.

Please, adjust the patch accordingly.

Best regards

Heinrich

>>
>>
>> This is not defined in the spec and actually those strings can be anything.
>> As currently this is for qemu_arm64 only and we don't have real vendor-defined values,
>> "Not Specified" is used as a placeholder to show an example for other vendors if they
>> want to add similar properties.
>
>OK I see.
>
>Reviewed-by: Simon Glass <sjg at chromium.org>