[PATCH v8 4/8] efi_loader: add a function to remove memory from the EFI map

Ilias Apalodimas ilias.apalodimas at linaro.org
Wed Mar 12 11:18:27 CET 2025


Hi Heinrich

On Wed, 12 Mar 2025 at 12:13, Heinrich Schuchardt <xypron.glpk at gmx.de>
wrote:

> On 12.03.25 09:54, Sughosh Ganu wrote:
> > From: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> >
> > With upcoming changes supporting pmem nodes, we need to remove the
> > pmem area from the EFI memory map. Add a function to do that.
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > ---
> > Changes since V7: None
> >
> >   include/efi_loader.h        | 18 ++++++++++++
> >   lib/efi_loader/efi_memory.c | 56 ++++++++++++++++++++++++++-----------
> >   2 files changed, 58 insertions(+), 16 deletions(-)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index e9c10819ba2..c20c98c6476 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -878,6 +878,24 @@ efi_status_t efi_get_memory_map(efi_uintn_t
> *memory_map_size,
> >   /* Adds a range into the EFI memory map */
> >   efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type);
> >
> > +/**
> > + * efi_update_memory_map() - update the memory map by adding/removing
> pages
> > + *
> > + * @start:                   start address, must be a multiple of
> > + *                           EFI_PAGE_SIZE
> > + * @pages:                   number of pages to add
> > + * @memory_type:             type of memory added
> > + * @overlap_conventional:    region may only overlap free(conventional)
> > + *                           memory
> > + * @remove:                  remove memory map
> > + * Return:                   status code
> > + */
> > +efi_status_t efi_update_memory_map(u64 start, u64 pages, int
> memory_type,
> > +                                bool overlap_conventional, bool remove);
>
> Why do we have so many functions that are all doing nearly the same thing:
>
> * efi_map_update_notify()
> * efi_update_memory_map()
> * efi_remove_memory_map()
> * efi_add_memory_map_pg()
>
> I start failing to understand when to use which. This is getting as
> gruesome as LMB.
>

We are not there yet!


> Can't we have a single exported function with a properly defined
> interface to cover all use cases
>

We actually do,
efi_update_memory_map() is what all the functions call with the correct
arguments.
The reset were added to make the thing easier not harder. You want to
remove the wrappers?

Thanks
/Ilias

Best regards
>
> Heinrich
>
> > +
> > +/* Remove memory from the EFI memory map */
> > +efi_status_t efi_remove_memory_map(u64 start, u64 size, int
> memory_type);
> > +
> >   /* Called by board init to initialize the EFI drivers */
> >   efi_status_t efi_driver_init(void);
> >   /* Called when a block device is added */
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index 6d00b186250..3fec40b6de5 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -258,7 +258,7 @@ static s64 efi_mem_carve_out(struct efi_mem_list
> *map,
> >   }
> >
> >   /**
> > - * efi_add_memory_map_pg() - add pages to the memory map
> > + * efi_update_memory_map() - update the memory map by adding/removing
> pages
> >    *
> >    * @start:                  start address, must be a multiple of
> >    *                          EFI_PAGE_SIZE
> > @@ -266,12 +266,11 @@ static s64 efi_mem_carve_out(struct efi_mem_list
> *map,
> >    * @memory_type:            type of memory added
> >    * @overlap_conventional:   region may only overlap free(conventional)
> >    *                          memory
> > + * @remove:                  remove memory map
> >    * Return:                  status code
> >    */
> > -static
> > -efi_status_t efi_add_memory_map_pg(u64 start, u64 pages,
> > -                                int memory_type,
> > -                                bool overlap_conventional)
> > +efi_status_t efi_update_memory_map(u64 start, u64 pages, int
> memory_type,
> > +                                bool overlap_conventional, bool remove)
> >   {
> >       struct efi_mem_list *lmem;
> >       struct efi_mem_list *newlist;
> > @@ -279,9 +278,9 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64
> pages,
> >       uint64_t carved_pages = 0;
> >       struct efi_event *evt;
> >
> > -     EFI_PRINT("%s: 0x%llx 0x%llx %d %s\n", __func__,
> > +     EFI_PRINT("%s: 0x%llx 0x%llx %d %s %s\n", __func__,
> >                 start, pages, memory_type, overlap_conventional ?
> > -               "yes" : "no");
> > +               "yes" : "no", remove ? "remove" : "add");
> >
> >       if (memory_type >= EFI_MAX_MEMORY_TYPE)
> >               return EFI_INVALID_PARAMETER;
> > @@ -364,7 +363,10 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64
> pages,
> >       }
> >
> >       /* Add our new map */
> > -        list_add_tail(&newlist->link, &efi_mem);
> > +     if (!remove)
> > +             list_add_tail(&newlist->link, &efi_mem);
> > +     else
> > +             free(newlist);
> >
> >       /* And make sure memory is listed in descending order */
> >       efi_mem_sort();
> > @@ -401,7 +403,29 @@ efi_status_t efi_add_memory_map(u64 start, u64
> size, int memory_type)
> >       pages = efi_size_in_pages(size + (start & EFI_PAGE_MASK));
> >       start &= ~EFI_PAGE_MASK;
> >
> > -     return efi_add_memory_map_pg(start, pages, memory_type, false);
> > +     return efi_update_memory_map(start, pages, memory_type, false,
> false);
> > +}
> > +
> > +/**
> > + * efi_remove_memory_map() - remove memory area to the memory map
> > + *
> > + * @start:           start address of the memory area
> > + * @size:            length in bytes of the memory area
> > + * @memory_type:     type of memory removed
> > + *
> > + * Return:           status code
> > + *
> > + * This function automatically aligns the start and size of the memory
> area
> > + * to EFI_PAGE_SIZE.
> > + */
> > +efi_status_t efi_remove_memory_map(u64 start, u64 size, int memory_type)
> > +{
> > +     u64 pages;
> > +
> > +     pages = efi_size_in_pages(size + (start & EFI_PAGE_MASK));
> > +     start &= ~EFI_PAGE_MASK;
> > +
> > +     return efi_update_memory_map(start, pages, memory_type, false,
> true);
> >   }
> >
> >   /**
> > @@ -502,7 +526,7 @@ efi_status_t efi_allocate_pages(enum
> efi_allocate_type type,
> >
> >       efi_addr = (u64)(uintptr_t)map_sysmem(addr, 0);
> >       /* Reserve that map in our memory maps */
> > -     ret = efi_add_memory_map_pg(efi_addr, pages, memory_type, true);
> > +     ret = efi_update_memory_map(efi_addr, pages, memory_type, true,
> false);
> >       if (ret != EFI_SUCCESS) {
> >               /* Map would overlap, bail out */
> >               lmb_free_flags(addr, (u64)pages << EFI_PAGE_SHIFT, flags);
> > @@ -823,8 +847,8 @@ static void add_u_boot_and_runtime(void)
> >                      uboot_stack_size) & ~EFI_PAGE_MASK;
> >       uboot_pages = ((uintptr_t)map_sysmem(gd->ram_top - 1, 0) -
> >                      uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
> > -     efi_add_memory_map_pg(uboot_start, uboot_pages,
> EFI_BOOT_SERVICES_CODE,
> > -                           false);
> > +     efi_update_memory_map(uboot_start, uboot_pages,
> EFI_BOOT_SERVICES_CODE,
> > +                           false, false);
> >   #if defined(__aarch64__)
> >       /*
> >        * Runtime Services must be 64KiB aligned according to the
> > @@ -842,8 +866,8 @@ static void add_u_boot_and_runtime(void)
> >       runtime_end = (uintptr_t)__efi_runtime_stop;
> >       runtime_end = (runtime_end + runtime_mask) & ~runtime_mask;
> >       runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
> > -     efi_add_memory_map_pg(runtime_start, runtime_pages,
> > -                           EFI_RUNTIME_SERVICES_CODE, false);
> > +     efi_update_memory_map(runtime_start, runtime_pages,
> > +                           EFI_RUNTIME_SERVICES_CODE, false, false);
> >   }
> >
> >   int efi_memory_init(void)
> > @@ -878,11 +902,11 @@ int efi_map_update_notify(phys_addr_t addr,
> phys_size_t size,
> >       pages = efi_size_in_pages(size + (efi_addr & EFI_PAGE_MASK));
> >       efi_addr &= ~EFI_PAGE_MASK;
> >
> > -     status = efi_add_memory_map_pg(efi_addr, pages,
> > +     status = efi_update_memory_map(efi_addr, pages,
> >                                      op == LMB_MAP_OP_RESERVE ?
> >                                      EFI_BOOT_SERVICES_DATA :
> >                                      EFI_CONVENTIONAL_MEMORY,
> > -                                    false);
> > +                                    false, false);
> >       if (status != EFI_SUCCESS) {
> >               log_err("LMB Map notify failure %lu\n",
> >                       status & ~EFI_ERROR_MASK);
>
>


More information about the U-Boot mailing list