[PATCH v2 14/14] efi_memory: rename variable to highlight overlap with free memory

[Ilias Apalodimas]

Hi Sughosh,

On Tue, 8 Oct 2024 at 21:15, Sughosh Ganu <sughosh.ganu at 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 at 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
>