[PATCH] smbios: arm64: Allow table to be written at a fixed addr

[Simon Glass]

Hi Caleb,

On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>
> Hi Simon,
>
> On 21/10/2023 01:45, Simon Glass wrote:
> > U-Boot typically sets up its malloc() pool near the top of memory. On
> > ARM64 systems this can result in an SMBIOS table above 4GB which is
> > not supported by SMBIOSv2.
> >
> > Work around this problem by providing a new option to choose an address
> > just below 4GB, if needed.
> I may well be missing something here, but I don't quite understand the
> justification behind the original patch [1] which caused this regression.
>
> I'm currently preparing support for a few different Qualcomm boards
> which have different memory layouts, so far it's been possible to avoid
> having any fixed addresses, so the same u-boot image can be used on all
> of them (with just a different DTB).

What sort of memory layout do you have? I could enhance the patch to
find the top of a memory bank below 4GB, perhaps? E.g. see the
'bdinfo' command.

>
> As I mentioned before, efi_allocate_pages() seems to work fine from
> install_smbios_table(), I haven't delved into the code too much but I
> wonder if this could be an acceptable solution?

Unfortunately that function is EFI-specific. The function can only be
called once EFI is inited. We only want to do that if booting with
EFI.

The tables are written at the start of U-Boot, partly because that is
how it is done on x86 and partly so the 'acpi' command actually works.

The EFI code ended up writing a separate set of tables, which is of
course not correct.

I did look at creating an API in U-Boot to alloc memory below 4GB but
then decided that for just this one use case it might not be worth it.

>
> Barring that, could we use an environment variable to pass this address
> in? Although I think that might not be a very desirable solution.
>
> I appreciate you taking the time to prepare this patch and CC me. If
> nobody else objects to this patch then I'd prefer to avoid blocking it.
> In either case, I'd greatly appreciate any input on the above
> suggestions so that I can implement a solution for my boards.

I would like it to be automatic. I assumed on ARM64 that there is
memory available at 3.99GB if U-Boot has relocated to above 4GB, but
as above I could make this more clever.

Regards,
Simon


>
> Thanks and regards,
>
> [1]:
> https://lore.kernel.org/u-boot/20230920030027.1385833-16-sjg@chromium.org/
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> >   lib/Kconfig                 | 17 +++++++++++++++++
> >   lib/efi_loader/efi_smbios.c | 12 ++++++++++++
> >   2 files changed, 29 insertions(+)
> >
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index f6ca559897e7..a77f2f3e9089 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -994,6 +994,23 @@ config GENERATE_SMBIOS_TABLE
> >         See also SMBIOS_SYSINFO which allows SMBIOS values to be provided in
> >         the devicetree.
> >
> > +config SMBIOS_TABLE_FIXED
> > +     bool "Place the SMBIOS table at a special address"
> > +     depends on GENERATE_SMBIOS_TABLE && ARM64 && SMBIOS && EFI_LOADER
> > +     default y
> > +     help
> > +       Use this option to place the SMBIOS table at a fixed address.
> > +
> > +       U-Boot typically sets up its malloc() pool near the top of memory. On
> > +       ARM64 systems this can result in an SMBIOS table above 4GB which is
> > +       not supported by SMBIOSv2. This option works around this problem by
> > +       chosing an address just below 4GB, if needed.
> > +
> > +config SMBIOS_TABLE_FIXED_ADDR
> > +     hex "Fixed address for use for SMBIOS table"
> > +     depends on SMBIOS_TABLE_FIXED
> > +     default 0xffff0000
> > +
> >   endmenu
> >
> >   config LIB_RATIONAL
> > diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
> > index 48446f654d9b..84ea027ea48c 100644
> > --- a/lib/efi_loader/efi_smbios.c
> > +++ b/lib/efi_loader/efi_smbios.c
> > @@ -61,6 +61,18 @@ static int install_smbios_table(void)
> >               return log_msg_ret("mem", -ENOMEM);
> >
> >       addr = map_to_sysmem(buf);
> > +
> > +     /*
> > +      * Deal with a fixed address if needed. For simplicity we assume that
> > +      * the SMBIOS-table size is <64KB and that the malloc region does not
> > +      * straddle the 4GB boundary.
> > +      */
> > +     if (IS_ENABLED(CONFIG_SMBIOS_TABLE_FIXED) && addr >= SZ_4G - SZ_64K) {
> > +             addr = IF_ENABLED_INT(CONFIG_SMBIOS_TABLE_FIXED,
> > +                                   CONFIG_SMBIOS_TABLE_FIXED_ADDR);
> > +             log_warning("Forcing SMBIOS table to address %lx\n", addr);
> > +     }
> > +
> >       if (!write_smbios_table(addr)) {
> >               log_err("Failed to write SMBIOS table\n");
> >               return log_msg_ret("smbios", -EINVAL);
>
> --
> // Caleb (they/them)