[PATCH 1/3] efi_loader: use pointers in efi_acpi_register()

[Simon Glass]

Hi Heinrich,

On 2026-05-21T18:50:22, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> 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 <heinrich.schuchardt at canonical.com>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>
> 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()

> 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 ?

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?

Regards,
Simon