Hi Caleb, On Sat, 21 Oct 2023 at 01:43, Caleb Connolly <caleb.conno...@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-...@chromium.org/ > > > > Signed-off-by: Simon Glass <s...@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)
