Re: [PATCH v3] smbios: arm64: Ensure table is written at a good address

Hi Simon,

On Tue, 21 Nov 2023 at 04:58, Simon Glass sjg@chromium.org 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 automatically choosing an address below 4GB (but as high as possible), if needed.

Tell efi_loader about the address in the same way as we do for ACPI tables.

Signed-off-by: Simon Glass sjg@chromium.org

Please see previous discussion here:

https://patchwork.ozlabs.org/project/uboot/patch/ 20231025123117.v2.1.I054cad60e00f8cfde39256c7c7d9c960987fb9be@changeid/

Changes in v3:

• Add RISC-V to the condition, too
• Update the commit message

Changes in v2:

• Update to search for a suitable area automatically, if enabled

lib/Kconfig | 13 ++++++++ lib/efi_loader/efi_smbios.c | 63 ++++++++++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/lib/Kconfig b/lib/Kconfig index 19649517a39b..c70faeeebed5 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -998,6 +998,19 @@ 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 && SMBIOS && EFI_LOADER
  

•   depends on ARM64 || RISCV
  

•   default y
  

•   help
  

•     Use this option to place the SMBIOS table at a special 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.
  

endmenu

config LIB_RATIONAL diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c index 48446f654d9b..566f206af5b6 100644 --- a/lib/efi_loader/efi_smbios.c +++ b/lib/efi_loader/efi_smbios.c @@ -47,6 +47,60 @@ efi_status_t efi_smbios_register(void) map_sysmem(addr, 0)); }

+/**

• • find_addr_below() - Find a usable region below the given max_addr
• • Check if *addrp is suitable to define a memory region which finishes below
• • @max_addr + @req_size. If so, do nothing and return 0
• • As a backup, if CONFIG_SMBIOS_TABLE_FIXED is enabled, search for a
• • 4KB-aligned DRAM region which is large enough. Make sure it is below U-Boot's
• • stack space, assumed to be 64KB.
• • @max_addr: Maximum address that can be used (region must finish before here)
• • @req:size: Required size of region
• • @addrp: On entry: Current proposed address; on exit, holds the new address,

• • on success
    

• • Return 0 if OK (existing region was OK, or a new one was found), -ENOSPC if
• • nothing suitable was found
• */

+static int find_addr_below(ulong max_addr, ulong req_size, ulong *addrp) +{

•   struct bd_info *bd = gd->bd;
  

•   ulong max_base;
  

•   int i;
  

•   max_base = max_addr - req_size;
  

•   if (*addrp <= max_base)
  

•           return 0;
  

•   if (!IS_ENABLED(CONFIG_SMBIOS_TABLE_FIXED))
  

•           return -ENOSPC;
  

•   /* Make sure that the base is at least 64KB below the stack */
  

•   max_base = min(max_base,
  

•                  ALIGN(gd->start_addr_sp - SZ_64K - req_size, SZ_4K));
  

•   for (i = CONFIG_NR_DRAM_BANKS - 1; i >= 0; i--) {
  

•           ulong start = bd->bi_dram[i].start;
  

•           ulong size = bd->bi_dram[i].size;
  

•           ulong addr;
  

•           /* chose an address at most req_size bytes before the end */
  

•           addr = min(max_base, start - req_size + size);
  

•           /* check this is in the range */
  

•           if (addr >= start && addr + req_size < start + size) {
  

•                   *addrp = addr;
  

•                   log_warning("Forcing SMBIOS table to address %lx\n",
  

•                               addr);
  

•                   return 0;
  

•           }
  

•   }
  

•   return -ENOSPC;
  

+}

This function doesn't really belong here. We should move it to the mem subsystem. AFAICT this is a hotfix for 2024.01. So I prefer picking this [0] instead, since the smbios table generation is still controlled by EFI

[0] https://lore.kernel.org/u-boot/20231120222558.32014-1-heinrich.schuchardt@ca...

Thanks /Ilias

static int install_smbios_table(void) { ulong addr; @@ -61,7 +115,14 @@ static int install_smbios_table(void) return log_msg_ret("mem", -ENOMEM);

    addr = map_to_sysmem(buf);

•   if (!write_smbios_table(addr)) {
  

•   /*
  

•    * Deal with a fixed address if needed. For simplicity we assume that
  

•    * the SMBIOS-table size is <64KB. If a suitable address cannot be
  

•    * found, then write_smbios_table() returns an error.
  

•    */
  

•   if (find_addr_below(SZ_4G - 1, SZ_64K, &addr) ||
  

•       !write_smbios_table(addr)) {
            log_err("Failed to write SMBIOS table\n");
            return log_msg_ret("smbios", -EINVAL);
    }
  

-- 2.43.0.rc1.413.gea7ed67945-goog