
Hi Heinrich,
On Wed, 1 Dec 2021 at 11:13, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/1/21 17:02, Simon Glass wrote:
The current EFI implementation confuses pointers and addresses. Normally we can get away with this but in the case of sandbox it causes failures.
Despite the fact that efi_allocate_pages() returns a u64, it is actually a pointer, not an address. Add special handling to avoid a crash when running 'bootefi hello'.
Signed-off-by: Simon Glass sjg@chromium.org
lib/efi_loader/efi_acpi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_acpi.c b/lib/efi_loader/efi_acpi.c index 016bbf6db33..9d101aa843e 100644 --- a/lib/efi_loader/efi_acpi.c +++ b/lib/efi_loader/efi_acpi.c @@ -8,6 +8,7 @@ #include <common.h> #include <efi_loader.h> #include <log.h> +#include <mapmem.h> #include <acpi/acpi_table.h>
static const efi_guid_t acpi_guid = EFI_ACPI_TABLE_GUID; @@ -22,6 +23,7 @@ efi_status_t efi_acpi_register(void) /* Map within the low 32 bits, to allow for 32bit ACPI tables */ u64 acpi = U32_MAX; efi_status_t ret;
ulong addr; /* Reserve 64kiB page for ACPI */ ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
@@ -34,7 +36,8 @@ efi_status_t efi_acpi_register(void) * a 4k-aligned address, so it is safe to assume that * write_acpi_tables() will write the table at that address. */
write_acpi_tables((ulong)acpi);
addr = map_to_sysmem((void *)(ulong)acpi);
Please, don't pollute general code with sandbox specific stuff where this can be avoided.
write_acpi_tables() anyway converts to a pointer. We should not convert twice. Correct the parameter of write_acpi_tables() instead to expect a pointer.
I don't want to use pointers to pass addresses. It is not the common approach in U-Boot. We use ulong to pass addresses.
Re the need for this change, I beg to differ, and the crash shows that I am right :-)
Don't you think it is odd that the EFI allocation routine returns a u64 instead of a void *, like malloc()? If that were fixed, then this code would make more sense. I think we talked about it before.
Regards, Simon