| Message ID | 20241008181435.1753814-15-sughosh.ganu@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Make EFI memory allocations synchronous with LMB | expand |
Hi Sughosh, On Tue, 8 Oct 2024 at 21:15, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > The functions to add a memory region to the EFI memory map have a > variable, overlap_only_ram, which is used to indicate if a new memory > region needs to be carved out from the free, or conventional > memory. This is a bit confusing. Conventional memory is free memory. Isn't it this from conventional or non-conventional memory? > The name overlap_only_ram is a bit confusing, as other > allocated memory regions, like boot-services code and data are also > part of the RAM memory. Rename the variable to highlight the fact that > it is the free/conventional memory that is being referred to in this > context. overlap_conventional should be even better? Thanks /Ilias > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > Changes since V1: New patch > > include/efi_loader.h | 16 +++++++----- > lib/efi_loader/efi_memory.c | 51 ++++++++++++++++++++----------------- > 2 files changed, 37 insertions(+), 30 deletions(-) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 0d5555c7597..c3512c63a50 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -788,15 +788,17 @@ efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type); > /** > * efi_add_memory_map_pg() - add pages to the memory map > * > - * @start: start address, must be a multiple of EFI_PAGE_SIZE > - * @pages: number of pages to add > - * @memory_type: type of memory added > - * @overlap_only_ram: region may only overlap RAM > - * Return: status code > + * @start: start address, must be a multiple of > + * EFI_PAGE_SIZE > + * @pages: number of pages to add > + * @memory_type: type of memory added > + * @overlap_only_freemem: region may only overlap free(conventional) > + * memory > + * Return: status code > */ > efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, > - int memory_type, > - bool overlap_only_ram); > + int memory_type, > + bool overlap_only_freemem); > > /* Called by board init to initialize the EFI drivers */ > efi_status_t efi_driver_init(void); > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > index 0f25094dbf8..97b89a97a2e 100644 > --- a/lib/efi_loader/efi_memory.c > +++ b/lib/efi_loader/efi_memory.c > @@ -173,17 +173,19 @@ static void efi_mem_sort(void) > /** > * efi_mem_carve_out() - unmap memory region > * > - * @map: memory map > - * @carve_desc: memory region to unmap > - * @overlap_only_ram: the carved out region may only overlap RAM > - * Return: the number of overlapping pages which have been > - * removed from the map, > - * EFI_CARVE_NO_OVERLAP, if the regions don't overlap, > - * EFI_CARVE_OVERLAPS_NONRAM, if the carve and map overlap, > - * and the map contains anything but free ram > - * (only when overlap_only_ram is true), > - * EFI_CARVE_LOOP_AGAIN, if the mapping list should be > - * traversed again, as it has been altered. > + * @map: memory map > + * @carve_desc: memory region to unmap > + * @overlap_only_freemem: the carved out region may only overlap free, > + * or conventional memory > + * Return: the number of overlapping pages which have been > + * removed from the map, > + * EFI_CARVE_NO_OVERLAP, if the regions don't > + * overlap, EFI_CARVE_OVERLAPS_NONRAM, if the carve > + * and map overlap, and the map contains anything > + * but free ram(only when overlap_only_freemem is > + * true), > + * EFI_CARVE_LOOP_AGAIN, if the mapping list should > + * be traversed again, as it has been altered. > * > * Unmaps all memory occupied by the carve_desc region from the list entry > * pointed to by map. > @@ -193,7 +195,7 @@ static void efi_mem_sort(void) > */ > static s64 efi_mem_carve_out(struct efi_mem_list *map, > struct efi_mem_desc *carve_desc, > - bool overlap_only_ram) > + bool overlap_only_freemem) > { > struct efi_mem_list *newmap; > struct efi_mem_desc *map_desc = &map->desc; > @@ -208,7 +210,7 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, > return EFI_CARVE_NO_OVERLAP; > > /* We're overlapping with non-RAM, warn the caller if desired */ > - if (overlap_only_ram && (map_desc->type != EFI_CONVENTIONAL_MEMORY)) > + if (overlap_only_freemem && (map_desc->type != EFI_CONVENTIONAL_MEMORY)) > return EFI_CARVE_OVERLAPS_NONRAM; > > /* Sanitize carve_start and carve_end to lie within our bounds */ > @@ -258,15 +260,17 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, > /** > * efi_add_memory_map_pg() - add pages to the memory map > * > - * @start: start address, must be a multiple of EFI_PAGE_SIZE > - * @pages: number of pages to add > - * @memory_type: type of memory added > - * @overlap_only_ram: region may only overlap RAM > - * Return: status code > + * @start: start address, must be a multiple of > + * EFI_PAGE_SIZE > + * @pages: number of pages to add > + * @memory_type: type of memory added > + * @overlap_only_freemem: region may only overlap free(conventional) > + * memory > + * Return: status code > */ > efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, > - int memory_type, > - bool overlap_only_ram) > + int memory_type, > + bool overlap_only_freemem) > { > struct efi_mem_list *lmem; > struct efi_mem_list *newlist; > @@ -275,7 +279,8 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, > struct efi_event *evt; > > EFI_PRINT("%s: 0x%llx 0x%llx %d %s\n", __func__, > - start, pages, memory_type, overlap_only_ram ? "yes" : "no"); > + start, pages, memory_type, overlap_only_freemem ? > + "yes" : "no"); > > if (memory_type >= EFI_MAX_MEMORY_TYPE) > return EFI_INVALID_PARAMETER; > @@ -312,7 +317,7 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, > s64 r; > > r = efi_mem_carve_out(lmem, &newlist->desc, > - overlap_only_ram); > + overlap_only_freemem); > switch (r) { > case EFI_CARVE_OUT_OF_RESOURCES: > free(newlist); > @@ -348,7 +353,7 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, > } > } while (carve_again); > > - if (overlap_only_ram && (carved_pages != pages)) { > + if (overlap_only_freemem && (carved_pages != pages)) { > /* > * The payload wanted to have RAM overlaps, but we overlapped > * with an unallocated region. Error out. > -- > 2.34.1 >
On Fri, 11 Oct 2024 at 16:29, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Sughosh, > > On Tue, 8 Oct 2024 at 21:15, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > The functions to add a memory region to the EFI memory map have a > > variable, overlap_only_ram, which is used to indicate if a new memory > > region needs to be carved out from the free, or conventional > > memory. > > This is a bit confusing. Conventional memory is free memory. > Isn't it this from conventional or non-conventional memory? Okay, I will try to frame this sentence better. > > > The name overlap_only_ram is a bit confusing, as other > > allocated memory regions, like boot-services code and data are also > > part of the RAM memory. Rename the variable to highlight the fact that > > it is the free/conventional memory that is being referred to in this > > context. > > overlap_conventional should be even better? Indeed, it is much better. Will change. Thanks. -sughosh > > Thanks > /Ilias > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > Changes since V1: New patch > > > > include/efi_loader.h | 16 +++++++----- > > lib/efi_loader/efi_memory.c | 51 ++++++++++++++++++++----------------- > > 2 files changed, 37 insertions(+), 30 deletions(-) > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > index 0d5555c7597..c3512c63a50 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -788,15 +788,17 @@ efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type); > > /** > > * efi_add_memory_map_pg() - add pages to the memory map > > * > > - * @start: start address, must be a multiple of EFI_PAGE_SIZE > > - * @pages: number of pages to add > > - * @memory_type: type of memory added > > - * @overlap_only_ram: region may only overlap RAM > > - * Return: status code > > + * @start: start address, must be a multiple of > > + * EFI_PAGE_SIZE > > + * @pages: number of pages to add > > + * @memory_type: type of memory added > > + * @overlap_only_freemem: region may only overlap free(conventional) > > + * memory > > + * Return: status code > > */ > > efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, > > - int memory_type, > > - bool overlap_only_ram); > > + int memory_type, > > + bool overlap_only_freemem); > > > > /* Called by board init to initialize the EFI drivers */ > > efi_status_t efi_driver_init(void); > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > > index 0f25094dbf8..97b89a97a2e 100644 > > --- a/lib/efi_loader/efi_memory.c > > +++ b/lib/efi_loader/efi_memory.c > > @@ -173,17 +173,19 @@ static void efi_mem_sort(void) > > /** > > * efi_mem_carve_out() - unmap memory region > > * > > - * @map: memory map > > - * @carve_desc: memory region to unmap > > - * @overlap_only_ram: the carved out region may only overlap RAM > > - * Return: the number of overlapping pages which have been > > - * removed from the map, > > - * EFI_CARVE_NO_OVERLAP, if the regions don't overlap, > > - * EFI_CARVE_OVERLAPS_NONRAM, if the carve and map overlap, > > - * and the map contains anything but free ram > > - * (only when overlap_only_ram is true), > > - * EFI_CARVE_LOOP_AGAIN, if the mapping list should be > > - * traversed again, as it has been altered. > > + * @map: memory map > > + * @carve_desc: memory region to unmap > > + * @overlap_only_freemem: the carved out region may only overlap free, > > + * or conventional memory > > + * Return: the number of overlapping pages which have been > > + * removed from the map, > > + * EFI_CARVE_NO_OVERLAP, if the regions don't > > + * overlap, EFI_CARVE_OVERLAPS_NONRAM, if the carve > > + * and map overlap, and the map contains anything > > + * but free ram(only when overlap_only_freemem is > > + * true), > > + * EFI_CARVE_LOOP_AGAIN, if the mapping list should > > + * be traversed again, as it has been altered. > > * > > * Unmaps all memory occupied by the carve_desc region from the list entry > > * pointed to by map. > > @@ -193,7 +195,7 @@ static void efi_mem_sort(void) > > */ > > static s64 efi_mem_carve_out(struct efi_mem_list *map, > > struct efi_mem_desc *carve_desc, > > - bool overlap_only_ram) > > + bool overlap_only_freemem) > > { > > struct efi_mem_list *newmap; > > struct efi_mem_desc *map_desc = &map->desc; > > @@ -208,7 +210,7 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, > > return EFI_CARVE_NO_OVERLAP; > > > > /* We're overlapping with non-RAM, warn the caller if desired */ > > - if (overlap_only_ram && (map_desc->type != EFI_CONVENTIONAL_MEMORY)) > > + if (overlap_only_freemem && (map_desc->type != EFI_CONVENTIONAL_MEMORY)) > > return EFI_CARVE_OVERLAPS_NONRAM; > > > > /* Sanitize carve_start and carve_end to lie within our bounds */ > > @@ -258,15 +260,17 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, > > /** > > * efi_add_memory_map_pg() - add pages to the memory map > > * > > - * @start: start address, must be a multiple of EFI_PAGE_SIZE > > - * @pages: number of pages to add > > - * @memory_type: type of memory added > > - * @overlap_only_ram: region may only overlap RAM > > - * Return: status code > > + * @start: start address, must be a multiple of > > + * EFI_PAGE_SIZE > > + * @pages: number of pages to add > > + * @memory_type: type of memory added > > + * @overlap_only_freemem: region may only overlap free(conventional) > > + * memory > > + * Return: status code > > */ > > efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, > > - int memory_type, > > - bool overlap_only_ram) > > + int memory_type, > > + bool overlap_only_freemem) > > { > > struct efi_mem_list *lmem; > > struct efi_mem_list *newlist; > > @@ -275,7 +279,8 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, > > struct efi_event *evt; > > > > EFI_PRINT("%s: 0x%llx 0x%llx %d %s\n", __func__, > > - start, pages, memory_type, overlap_only_ram ? "yes" : "no"); > > + start, pages, memory_type, overlap_only_freemem ? > > + "yes" : "no"); > > > > if (memory_type >= EFI_MAX_MEMORY_TYPE) > > return EFI_INVALID_PARAMETER; > > @@ -312,7 +317,7 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, > > s64 r; > > > > r = efi_mem_carve_out(lmem, &newlist->desc, > > - overlap_only_ram); > > + overlap_only_freemem); > > switch (r) { > > case EFI_CARVE_OUT_OF_RESOURCES: > > free(newlist); > > @@ -348,7 +353,7 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, > > } > > } while (carve_again); > > > > - if (overlap_only_ram && (carved_pages != pages)) { > > + if (overlap_only_freemem && (carved_pages != pages)) { > > /* > > * The payload wanted to have RAM overlaps, but we overlapped > > * with an unallocated region. Error out. > > -- > > 2.34.1 > >
diff --git a/include/efi_loader.h b/include/efi_loader.h index 0d5555c7597..c3512c63a50 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -788,15 +788,17 @@ efi_status_t efi_add_memory_map(u64 start, u64 size, int memory_type); /** * efi_add_memory_map_pg() - add pages to the memory map * - * @start: start address, must be a multiple of EFI_PAGE_SIZE - * @pages: number of pages to add - * @memory_type: type of memory added - * @overlap_only_ram: region may only overlap RAM - * Return: status code + * @start: start address, must be a multiple of + * EFI_PAGE_SIZE + * @pages: number of pages to add + * @memory_type: type of memory added + * @overlap_only_freemem: region may only overlap free(conventional) + * memory + * Return: status code */ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, - int memory_type, - bool overlap_only_ram); + int memory_type, + bool overlap_only_freemem); /* Called by board init to initialize the EFI drivers */ efi_status_t efi_driver_init(void); diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 0f25094dbf8..97b89a97a2e 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -173,17 +173,19 @@ static void efi_mem_sort(void) /** * efi_mem_carve_out() - unmap memory region * - * @map: memory map - * @carve_desc: memory region to unmap - * @overlap_only_ram: the carved out region may only overlap RAM - * Return: the number of overlapping pages which have been - * removed from the map, - * EFI_CARVE_NO_OVERLAP, if the regions don't overlap, - * EFI_CARVE_OVERLAPS_NONRAM, if the carve and map overlap, - * and the map contains anything but free ram - * (only when overlap_only_ram is true), - * EFI_CARVE_LOOP_AGAIN, if the mapping list should be - * traversed again, as it has been altered. + * @map: memory map + * @carve_desc: memory region to unmap + * @overlap_only_freemem: the carved out region may only overlap free, + * or conventional memory + * Return: the number of overlapping pages which have been + * removed from the map, + * EFI_CARVE_NO_OVERLAP, if the regions don't + * overlap, EFI_CARVE_OVERLAPS_NONRAM, if the carve + * and map overlap, and the map contains anything + * but free ram(only when overlap_only_freemem is + * true), + * EFI_CARVE_LOOP_AGAIN, if the mapping list should + * be traversed again, as it has been altered. * * Unmaps all memory occupied by the carve_desc region from the list entry * pointed to by map. @@ -193,7 +195,7 @@ static void efi_mem_sort(void) */ static s64 efi_mem_carve_out(struct efi_mem_list *map, struct efi_mem_desc *carve_desc, - bool overlap_only_ram) + bool overlap_only_freemem) { struct efi_mem_list *newmap; struct efi_mem_desc *map_desc = &map->desc; @@ -208,7 +210,7 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, return EFI_CARVE_NO_OVERLAP; /* We're overlapping with non-RAM, warn the caller if desired */ - if (overlap_only_ram && (map_desc->type != EFI_CONVENTIONAL_MEMORY)) + if (overlap_only_freemem && (map_desc->type != EFI_CONVENTIONAL_MEMORY)) return EFI_CARVE_OVERLAPS_NONRAM; /* Sanitize carve_start and carve_end to lie within our bounds */ @@ -258,15 +260,17 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map, /** * efi_add_memory_map_pg() - add pages to the memory map * - * @start: start address, must be a multiple of EFI_PAGE_SIZE - * @pages: number of pages to add - * @memory_type: type of memory added - * @overlap_only_ram: region may only overlap RAM - * Return: status code + * @start: start address, must be a multiple of + * EFI_PAGE_SIZE + * @pages: number of pages to add + * @memory_type: type of memory added + * @overlap_only_freemem: region may only overlap free(conventional) + * memory + * Return: status code */ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, - int memory_type, - bool overlap_only_ram) + int memory_type, + bool overlap_only_freemem) { struct efi_mem_list *lmem; struct efi_mem_list *newlist; @@ -275,7 +279,8 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, struct efi_event *evt; EFI_PRINT("%s: 0x%llx 0x%llx %d %s\n", __func__, - start, pages, memory_type, overlap_only_ram ? "yes" : "no"); + start, pages, memory_type, overlap_only_freemem ? + "yes" : "no"); if (memory_type >= EFI_MAX_MEMORY_TYPE) return EFI_INVALID_PARAMETER; @@ -312,7 +317,7 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, s64 r; r = efi_mem_carve_out(lmem, &newlist->desc, - overlap_only_ram); + overlap_only_freemem); switch (r) { case EFI_CARVE_OUT_OF_RESOURCES: free(newlist); @@ -348,7 +353,7 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages, } } while (carve_again); - if (overlap_only_ram && (carved_pages != pages)) { + if (overlap_only_freemem && (carved_pages != pages)) { /* * The payload wanted to have RAM overlaps, but we overlapped * with an unallocated region. Error out.
The functions to add a memory region to the EFI memory map have a variable, overlap_only_ram, which is used to indicate if a new memory region needs to be carved out from the free, or conventional memory. The name overlap_only_ram is a bit confusing, as other allocated memory regions, like boot-services code and data are also part of the RAM memory. Rename the variable to highlight the fact that it is the free/conventional memory that is being referred to in this context. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- Changes since V1: New patch include/efi_loader.h | 16 +++++++----- lib/efi_loader/efi_memory.c | 51 ++++++++++++++++++++----------------- 2 files changed, 37 insertions(+), 30 deletions(-)