[PATCH 08/13] efi_loader: Tidy up use of addresses

[Simon Glass]

Hi Heinrich,

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