On 5/23/26 11:38, Simon Glass wrote:
Hi Heinrich,
On 2026-05-21T18:50:22, Heinrich Schuchardt
<[email protected]> wrote:
efi_loader: use pointers in efi_acpi_register()
Both efi_add_memory_map() and efi_install_configuration_table() expect
pointers and not virtual sandbox addresses.
We need to convert the virtual sandbox addresses in efi_acpi_register() to
pointers in memory before using them.
Signed-off-by: Heinrich Schuchardt <[email protected]>
Reviewed-by: Ilias Apalodimas <[email protected]>
lib/efi_loader/efi_acpi.c | 42 ++++++++++++++++++++++++++----------------
1 file changed, 26 insertions(+), 16 deletions(-)
diff --git a/lib/efi_loader/efi_acpi.c b/lib/efi_loader/efi_acpi.c
@@ -25,7 +25,7 @@ static const efi_guid_t acpi_guid = EFI_ACPI_TABLE_GUID;
*/
efi_status_t efi_acpi_register(void)
{
- ulong addr, start, end;
+ void *addr, *start, *end;
Making start/end pointers doesn't buy anything - they only feed a
length calculation and efi_add_memory_map(), which takes a u64 - every
later use then needs a (uintptr_t) cast, plus (void *) casts on the
ALIGN_DOWN()/ALIGN() results. Please leave start/end as ulong and only
convert addr, which is the one that genuinely wants pointer type for
efi_install_configuration_table()
Yes, using uintptr_t for start and end would eliminate conversions.
diff --git a/lib/efi_loader/efi_acpi.c b/lib/efi_loader/efi_acpi.c
@@ -37,39 +37,49 @@ efi_status_t efi_acpi_register(void)
+ start = (void *)ALIGN_DOWN(
+ (uintptr_t)map_sysmem(gd->arch.table_start, 0),
+ (uintptr_t)EFI_PAGE_MASK);
+ end = (void *)ALIGN((uintptr_t)map_sysmem(gd->arch.table_end,
+ 0),
+ (uintptr_t)EFI_PAGE_MASK);
Do you need the (uintptr_t) cast on EFI_PAGE_MASK ?
EFI_PAGE_MASK and EFI_PAGE_SIZE are 64bit wide. uintptr_t and void * are
only 32bit on 32bit systems. Removing the conversion leads to an error.
BTW the use of EFI_PAGE_MASK as the alignment argument is
pre-existing, but looks wrong: ALIGN(x, a) treats a as the alignment
and internally computes a-1 as the mask, so this becomes ALIGN(x,
EFI_PAGE_SIZE-1). Worth a separate fix?
You are right, we must use EFI_PAGE_SIZE here.
Best regards
Heinrich
Regards,
Simon
