[PATCH] x86: efi_loader: Ensure the SMBIOS tables are sent via EFI

[Heinrich Schuchardt]

On 11.04.25 12:04, Ilias Apalodimas wrote:
> Hi Simon
>
> On Sat, 5 Apr 2025 at 23:46, Simon Glass <sjg at chromium.org> wrote:
>>
>> The EFI-loader code has not been fully converted to use bloblist, so
>> relies on the SMBIOS-table address being set in global_data.
>
> Can you explain a bit more what this means? The SMBIOS tables for EFI
> are supposed to be on EFI allocated memory
>
>>
>> Set this up in write_tables() so that the SMBIOS tables are actually
>> available.
>>
>> Enable the command for x86 QEMU so that the SMBIOS tests actually run.
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> Fixes: 83ce35d6ebb ("emulation: Use bloblist to hold tables")
>> Reported-by: Niklas Sombert <niklas.sombert at uni-duesseldorf.de>
>> ---
>>
>>   arch/x86/lib/tables.c            | 2 ++
>>   board/emulation/qemu-x86/Kconfig | 1 +
>>   lib/efi_loader/efi_smbios.c      | 1 +
>>   3 files changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
>> index ec52992209f..d7f183f95dc 100644
>> --- a/arch/x86/lib/tables.c
>> +++ b/arch/x86/lib/tables.c
>> @@ -126,6 +126,8 @@ int write_tables(void)
>>                          use_high = true;
>>                          if (!gd->arch.table_start_high)
>>                                  gd->arch.table_start_high = rom_addr;
>> +                       if (table->tag == BLOBLISTT_SMBIOS_TABLES)
>> +                               gd_set_smbios_start(rom_addr);
>>                  }
>>                  rom_table_end = table->write(rom_addr);
>>                  if (!rom_table_end) {
>> diff --git a/board/emulation/qemu-x86/Kconfig b/board/emulation/qemu-x86/Kconfig
>> index b2a4e0891a4..c1564fba7cd 100644
>> --- a/board/emulation/qemu-x86/Kconfig
>> +++ b/board/emulation/qemu-x86/Kconfig
>> @@ -23,5 +23,6 @@ config BOARD_SPECIFIC_OPTIONS # dummy
>>          imply VIRTIO_PCI
>>          imply VIRTIO_NET
>>          imply VIRTIO_BLK
>> +       imply CMD_SMBIOS
>>
>>   endif
>> diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
>> index 8d2ef6deb51..5275e966dfe 100644
>> --- a/lib/efi_loader/efi_smbios.c
>> +++ b/lib/efi_loader/efi_smbios.c
>> @@ -35,6 +35,7 @@ efi_status_t efi_smbios_register(void)
>>          void *buf;
>>
>>          addr = gd_smbios_start();
>> +       printf("register %lx\n", addr);

User will not know what "register" relates to.

log_debug() with a meaningful message in the else branch of the if
statement below would be acceptable.

Best regards

Heinrich

>
> I don't think this print is useful at all, why isn't the log_err()
> right below enough?
>
>>          if (!addr) {
>>                  log_err("No SMBIOS tables to install\n");
>>                  return EFI_NOT_FOUND;
>> --
>> 2.43.0
>>
>
> Thanks
> /Ilias