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

[Simon Glass]

Hi again Heinrich,

On Tue, 26 Nov 2024 at 08:38, Simon Glass <sjg at chromium.org> wrote:
>
> 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.

Anyway, I have this around the wrong way, so can drop this patch once
I update patch 1 to document what is actually going on...

Regards,
Simon