[PATCH 02/13] efi_loader: Convert efi_get_memory_map() to return pointers

[Simon Glass]

Hi Heinrich,

On Tue, 26 Nov 2024 at 02:27, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 25.11.24 21:44, Simon Glass wrote:
> > This function should return pointers, not addresses. Add the conversion.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> >   lib/efi_loader/efi_memory.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index e493934c713..8b01821a993 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -748,6 +748,11 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size,
> >       memory_map = &memory_map[map_entries - 1];
> >       list_for_each_entry(lmem, &efi_mem, link) {
> >               *memory_map = lmem->desc;
> > +             /* Convert to pointers, which is what the caller expects */
> > +             memory_map->physical_start =
> > +                     (ulong)map_sysmem(lmem->desc.physical_start, 0);
> > +             memory_map->virtual_start =
> > +                     (ulong)map_sysmem(lmem->desc.virtual_start, 0);
> >               memory_map--;
> >       }
> >
>
> NAK.
>
> physical_start and virtual_addr are not sandbox virtual addresses.
>
> The sandbox virtual address space is irrelevant for UEFI. They should be
> restricted to the CLI.

Well, you can't have it both ways. Either this is a pointer expressed
as a u64, or it is an address.

My understanding is that it is a pointer expressed as a u64. The use
of u64 is a confusing part of the spec, but it is because they want to
specify the size as 64-bit, whereas a void * would not have a fixed
size. But you can't run a 32-bit image on a 64-bit machine anyway, so
this decision is a mystery to me.

Regards,
Simon