| Message ID | 20231231152555.464874-13-sjg@chromium.org |
|---|---|
| State | Accepted |
| Commit | 06ef8089f876b6dabf56caba31a05c003f03c629 |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | smbios: Deal with tables beyond 4GB | expand |
On 12/31/23 16:25, Simon Glass wrote: > At present this code allocates memory when writing the tables and > then unnecessarily adds another memory map when installing it. > > Adjust the code to allocate the tables using the normal U-Boot > mechanism. This avoids doing an EFI memory allocation early in > U-Boot, which may use memory that would be overwritten by a > 'load' command, for example. > > Signed-off-by: Simon Glass <sjg@chromium.org> In patch 11/12 you changed the address fields in ACPI tables from sandbox virtual addresses to pointers. Do you plan to do the same for SMBIOS? Best regards Heinrich
Hi Heinrich, On Sun, Dec 31, 2023 at 9:27 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 12/31/23 16:25, Simon Glass wrote: > > At present this code allocates memory when writing the tables and > > then unnecessarily adds another memory map when installing it. > > > > Adjust the code to allocate the tables using the normal U-Boot > > mechanism. This avoids doing an EFI memory allocation early in > > U-Boot, which may use memory that would be overwritten by a > > 'load' command, for example. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > In patch 11/12 you changed the address fields in ACPI tables from > sandbox virtual addresses to pointers. Do you plan to do the same for > SMBIOS? I haven't looked at it, but could if it would help. Are you planning to add an EFI test app for sandbox? Regards, Simon
On 1/1/24 01:37, Simon Glass wrote: > Hi Heinrich, > > On Sun, Dec 31, 2023 at 9:27 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> >> On 12/31/23 16:25, Simon Glass wrote: >>> At present this code allocates memory when writing the tables and >>> then unnecessarily adds another memory map when installing it. >>> >>> Adjust the code to allocate the tables using the normal U-Boot >>> mechanism. This avoids doing an EFI memory allocation early in >>> U-Boot, which may use memory that would be overwritten by a >>> 'load' command, for example. >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >> >> In patch 11/12 you changed the address fields in ACPI tables from >> sandbox virtual addresses to pointers. Do you plan to do the same for >> SMBIOS? > > I haven't looked at it, but could if it would help. Are you planning > to add an EFI test app for sandbox? Yes, we need a test checking that the SMBIOS protocol is correctly installed. A tool like dtbdump.efi (lib/efi_loader/dtbdump.efi) would allow to dump the SMBIOS table to file and then use dmidecode to check the tables for consistency. Best regards Heinrich
On 1/1/24 01:37, Simon Glass wrote: > Hi Heinrich, > > On Sun, Dec 31, 2023 at 9:27 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> >> On 12/31/23 16:25, Simon Glass wrote: >>> At present this code allocates memory when writing the tables and >>> then unnecessarily adds another memory map when installing it. >>> >>> Adjust the code to allocate the tables using the normal U-Boot >>> mechanism. This avoids doing an EFI memory allocation early in >>> U-Boot, which may use memory that would be overwritten by a >>> 'load' command, for example. >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >> >> In patch 11/12 you changed the address fields in ACPI tables from >> sandbox virtual addresses to pointers. Do you plan to do the same for >> SMBIOS? > > I haven't looked at it, but could if it would help. Are you planning > to add an EFI test app for sandbox? Yes, we need a test checking that the SMBIOS protocol is correctly installed. A tool like dtbdump.efi (lib/efi_loader/dtbdump.efi) would allow to dump the SMBIOS table to file and then use dmidecode to check the tables for consistency. Best regards Heinrich Applied to u-boot-dm/next, thanks!
diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c index 5db342ee0d7..eb6d2ba43c9 100644 --- a/lib/efi_loader/efi_smbios.c +++ b/lib/efi_loader/efi_smbios.c @@ -54,27 +54,25 @@ efi_status_t efi_smbios_register(void) static int install_smbios_table(void) { - u64 addr; - efi_status_t ret; + ulong addr; + void *buf; if (!IS_ENABLED(CONFIG_GENERATE_SMBIOS_TABLE) || IS_ENABLED(CONFIG_X86)) return 0; - addr = SZ_4G; - ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, - EFI_RUNTIME_SERVICES_DATA, - efi_size_in_pages(TABLE_SIZE), &addr); - if (ret != EFI_SUCCESS) + /* Align the table to a 4KB boundary to keep EFI happy */ + buf = memalign(SZ_4K, TABLE_SIZE); + if (!buf) return log_msg_ret("mem", -ENOMEM); - addr = map_to_sysmem((void *)(uintptr_t)addr); + addr = map_to_sysmem(buf); if (!write_smbios_table(addr)) { log_err("Failed to write SMBIOS table\n"); return log_msg_ret("smbios", -EINVAL); } /* Make a note of where we put it */ - log_debug("SMBIOS tables written to %llx\n", addr); + log_debug("SMBIOS tables written to %lx\n", addr); gd->arch.smbios_start = addr; return 0;
At present this code allocates memory when writing the tables and then unnecessarily adds another memory map when installing it. Adjust the code to allocate the tables using the normal U-Boot mechanism. This avoids doing an EFI memory allocation early in U-Boot, which may use memory that would be overwritten by a 'load' command, for example. Signed-off-by: Simon Glass <sjg@chromium.org> --- I believe this is the fix we agreed on for after the release, so I am including it in this series. Changes in v5: - Add new patch to correct smbios-table installation lib/efi_loader/efi_smbios.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)