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

[Heinrich Schuchardt]

Am 25. Oktober 2023 01:31:19 MESZ schrieb Simon Glass <sjg at chromium.org>:
>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
>below 4GB (but as high as possible), if needed.

You must not overwrite memory controlled by the EFI subsystem without calling its allocator.  We should provide SMBIOS 3. SMBIOS 2 is only a fallback for outdated tools.

Best regards

Heinrich

>
>Signed-off-by: Simon Glass <sjg at chromium.org>
>---
>
>Changes in v2:
>- Update to search for a suitable area automatically, if enabled
>
> lib/Kconfig                 | 12 +++++++
> lib/efi_loader/efi_smbios.c | 63 ++++++++++++++++++++++++++++++++++++-
> 2 files changed, 74 insertions(+), 1 deletion(-)
>
>diff --git a/lib/Kconfig b/lib/Kconfig
>index f6ca559897e7..a1eec98b392f 100644
>--- a/lib/Kconfig
>+++ b/lib/Kconfig
>@@ -994,6 +994,18 @@ 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 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..bdbce4c4d785 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;
>+}
>+
> 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, SZ_64K, &addr) ||
>+	    !write_smbios_table(addr)) {
> 		log_err("Failed to write SMBIOS table\n");
> 		return log_msg_ret("smbios", -EINVAL);
> 	}