[PATCH v5 09/23] efi_loader: Drop virtual_start from priv_mem_desc

[Simon Glass]

Hi Ilias,

On Wed, 11 Dec 2024 at 08:09, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Simon,
>
> On Wed, 11 Dec 2024 at 15:54, Simon Glass <sjg at chromium.org> wrote:
> >
> > This field is always the same as physical_start, so keeping track of it
> > separately is unnecessary and confusing. Drop it.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >  lib/efi_loader/efi_memory.c | 10 +++-------
> >  1 file changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index db40aee8353..8211600f7f6 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -37,7 +37,6 @@ efi_uintn_t efi_memory_map_key;
> >   * @type (enum efi_memory_type): EFI memory-type
> >   * @reserved: unused
> >   * @physical_start: Start address of region in physical memory
> > - * @virtual_start: Start address of region in physical memory
> >   * @num_pages: Number of EFI pages this record covers (each is EFI_PAGE_SIZE
> >   *     bytes)
> >   * @attribute: Memory attributes (see EFI_MEMORY...)
> > @@ -46,7 +45,6 @@ struct priv_mem_desc {
> >         u32 type;
> >         u32 reserved;
> >         efi_physical_addr_t physical_start;
> > -       efi_virtual_addr_t virtual_start;
> >         u64 num_pages;
> >         u64 attribute;
> >  };
> > @@ -187,7 +185,6 @@ static void efi_mem_sort(void)
> >                                 pages = cur->num_pages;
> >                                 prev->num_pages += pages;
> >                                 prev->physical_start -= pages << EFI_PAGE_SHIFT;
> > -                               prev->virtual_start -= pages << EFI_PAGE_SHIFT;
> >                                 list_del(&lmem->link);
> >                                 free(lmem);
> >
> > @@ -255,7 +252,6 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map,
> >                         free(map);
> >                 } else {
> >                         map->desc.physical_start = carve_end;
> > -                       map->desc.virtual_start = carve_end;
> >                         map->desc.num_pages = (map_end - carve_end)
> >                                               >> EFI_PAGE_SHIFT;
> >                 }
> > @@ -276,7 +272,6 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map,
> >                 return EFI_CARVE_OUT_OF_RESOURCES;
> >         newmap->desc = map->desc;
> >         newmap->desc.physical_start = carve_start;
> > -       newmap->desc.virtual_start = carve_start;
> >         newmap->desc.num_pages = (map_end - carve_start) >> EFI_PAGE_SHIFT;
> >         /* Insert before current entry (descending address order) */
> >         list_add_tail(&newmap->link, &map->link);
> > @@ -313,7 +308,6 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages,
> >                 return EFI_OUT_OF_RESOURCES;
> >         newlist->desc.type = memory_type;
> >         newlist->desc.physical_start = start;
> > -       newlist->desc.virtual_start = start;
> >         newlist->desc.num_pages = pages;
> >
> >         switch (memory_type) {
> > @@ -711,7 +705,9 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size,
> >                 memory_map->type = lmem->desc.type;
> >                 memory_map->reserved = lmem->desc.reserved;
> >                 memory_map->physical_start = lmem->desc.physical_start;
> > -               memory_map->virtual_start = lmem->desc.virtual_start;
> > +
> > +               /* virtual and physical are always the same */
> > +               memory_map->virtual_start = lmem->desc.physical_start;
> >                 memory_map->num_pages = lmem->desc.num_pages;
> >                 memory_map->attribute = lmem->desc.attribute;
> >                 memory_map--;
> > --
> > 2.34.1
> >
> I don't like duplicating efi_mem_desc and have a priv without the
> virtual address. I don't understand what you find confusing, but we
> have follows the spec (minus that reserved member) and I prefer it

We are still stuck on this address/pointer thing. As you will see at
the end of this series, I am changing the internal table to use
addresses, like lmb, so it is easier to understand what is going on...

Regards.

Simon