Re: [PATCH v2 0/8] SMBIOS improvements

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 4740 bytes --]

On Thu, Oct 24, 2024 at 09:35:36AM -0400, Raymond Mao wrote:
> Hi Tom,
> 
> On Wed, 23 Oct 2024 at 20:23, Tom Rini <trini@konsulko.com> wrote:
> 
> > On Tue, Oct 22, 2024 at 01:05:21PM -0700, Raymond Mao 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 (processor and cache).
> > > 3. Create a sysinfo driver to poppulate platform SMBIOS private data.
> > > 4. Add generic SMBIOS DTS file for arm64 platforms for those common
> > strings
> > >    and values which cannot be retrieved from the system registers.
> > >    Vendors can create their own SMBIOS node using this as an example.
> > >    For those boards without SMBIOS nodes, this DTS file can be included
> > to
> > >    have a generic SMBIOS information of the system.
> > > 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).
> > >
> > > Tests:
> > > To test this with QEMU arm64, please follow the guide on dt_qemu.rst to
> > > get a merged DT to run with.
> > > ```
> > > 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
> > > ```
> > >
> > > Known issues:
> > > It hits the image size limitation on R-CAR board(rcar3_salvator-x).
> > > ```
> > > u-boot.img exceeds file size limit:
> > >   limit:  0x100000 bytes
> > >   actual: 0x10049d bytes
> > >   excess: 0x49d bytes
> > > ```
> > > This board needs a clean-up to reserve spaces for the changes as SMBIOS
> > > is a fundamental feature.
> > >
> > > Below is the breakdown of the size-growth of the related functions:
> > >   function                                   old     new   delta
> > >     static.smbios_write_type4                  252    1052    +800
> > >     static.smbios_write_type7                    -     764    +764
> > >     static.smbios_write_type3                  188     488    +300
> > >     smbios_get_val_si                            -     128    +128
> > >     static.smbios_write_type2                  316     376     +60
> > >     sysinfo_get_data                             -      56     +56
> > >     static.smbios_write_type1                  380     396     +16
> > >     smbios_write_funcs                         112     128     +16
> > >     ofnode_read_u32                              -      12     +12
> > >     sysinfo_rcar_ops                            40      48      +8
> > >     install_smbios_table                       468     472      +4
> >
> > Right, so here's the problem I see right now. About 70% of all U-Boot
> > platforms enable GENERATE_SMBIOS_TABLE and so "smbios: Refactor smbios
> > library" causes a growth of around 1.5 kilobytes. That's a problem.
> > There is a place where we're going to generate as full and complete a
> > table as we can, and a place where we just want maybe the basics. We
> > need to re-factor things first so that the platforms which aren't doing
> > detailed tables do not grow and perhaps even shrink because we can pull
> > existing code out.
> >
> Do you have a list of those platforms which don't need detailed tables?
> I can add a new kconfig for this in the next version.

No, you need to determine this, but it should be something that can be
determined by existing data. The platforms which grow by only 1.5KiB
don't. The ones that grow by ~4KiB, maybe? You need to see what they're
actually doing today to determine that. Growing by ~8KiB? Yes.

You can see my log at:
https://gist.github.com/trini/8d3d4ab5b53402059a9d178786033c18

And all of that is aside from my original worry about including a large
number of static strings. I did not look in to if v2 deals with that.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]