[PATCH v8 4/8] efi_loader: add a function to remove memory from the EFI map
Sughosh Ganu
sughosh.ganu at linaro.org
Wed Mar 12 11:25:19 CET 2025
On Wed, 12 Mar 2025 at 15:43, 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()
This function does not get the size in terms of number of pages, but
the size in bytes, which it then converts to pages. Additionally,
there is this conversion from address to pointers.
> * efi_update_memory_map()
> * efi_remove_memory_map()
The efi_remove_memory_map() can indeed be removed, and we can directly
call efi_update_memory_map() directly.
> * efi_add_memory_map_pg()
This function will no longer exist once this series gets applied.
-sughosh
>
> I start failing to understand when to use which. This is getting as
> gruesome as LMB.
>
> Can't we have a single exported function with a properly defined
> interface to cover all use cases?
>
> 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