Hi Heinrich, On Tue, 26 Nov 2024 at 03:22, Heinrich Schuchardt <xypron.g...@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 <s...@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
