Hi Heinrich,
On Tue, 26 Nov 2024 at 03:22, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 25.11.24 21:44, Simon Glass wrote:
The EFI loader's memory implementation is quite confusing. The EFI spec requires that addresses are used in the calls to allocate_pages(), etc. This is unavoidable.
This usage then filters down to various functions within the implementation. This is unfortunate, as there is no clear separation between addresses and pointers. In some parts of the code things become hopelessly confusing, and several bugs have crept in.
Adjust the internal functions to always use a ulong for an address or a void * for a pointer.
Signed-off-by: Simon Glass sjg@chromium.org
include/efi_loader.h | 13 +++-- lib/efi_loader/efi_bootmgr.c | 9 ++-- lib/efi_loader/efi_boottime.c | 37 ++++++++------ lib/efi_loader/efi_helper.c | 7 +-- lib/efi_loader/efi_image_loader.c | 2 +- lib/efi_loader/efi_memory.c | 84 +++++++++++++------------------ lib/efi_loader/efi_var_mem.c | 6 +-- 7 files changed, 76 insertions(+), 82 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index cae94fc4661..c4afcf2b60b 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -791,7 +791,7 @@ void *efi_alloc_aligned_pages(u64 len, enum efi_memory_type mem_type, */ efi_status_t efi_allocate_pages(enum efi_allocate_type type, enum efi_memory_type mem_type,
efi_uintn_t pages, uint64_t *memoryp);
efi_uintn_t pages, void **memoryp);/**
- efi_free_pages() - free memory pages
@@ -800,7 +800,7 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type,
- @pages: number of pages to be freed
- Return: status code
*/ -efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages); +efi_status_t efi_free_pages(void *memory, efi_uintn_t pages);
Read the UEFI spec, please.
Again?
FreePages() expects EFI_PHYSICAL_ADDRESS which is defined as
typedef UINT64 EFI_PHYSICAL_ADDRESS;
How does this comment relate to my patch? We should be using pointers in most cases, as is done with allocate_pool.
[..]
Regards, Simon