[PATCH v2 12/14] efi_memory: do not add RAM memory to the memory map

[Ilias Apalodimas]

Hi Sughosh,

On Tue, 8 Oct 2024 at 21:15, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> The EFI_CONVENTIONAL_MEMORY type, which is the usable RAM memory is
> now being managed by the LMB module. Remove the addition of this
> memory type to the EFI memory map. This memory now gets added to the
> EFI memory map as part of the LMB memory map update event handler.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> Reviewed-by: Simon Glass <sjg at chromium.org>
> ---
> Changes since V1: None
>
>  include/efi_loader.h        | 13 ++++---
>  lib/efi_loader/efi_memory.c | 75 +++----------------------------------
>  2 files changed, 12 insertions(+), 76 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 081cb65d3f7..0d5555c7597 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -798,10 +798,6 @@ efi_status_t efi_add_memory_map_pg(u64 start, u64 pages,
>                                           int memory_type,
>                                           bool overlap_only_ram);
>
> -/* Adds a conventional range into the EFI memory map */
> -efi_status_t efi_add_conventional_memory_map(u64 ram_start, u64 ram_end,
> -                                            u64 ram_top);
> -
>  /* Called by board init to initialize the EFI drivers */
>  efi_status_t efi_driver_init(void);
>  /* Called when a block device is added */
> @@ -1186,9 +1182,14 @@ efi_status_t efi_console_get_u16_string
>  efi_status_t efi_disk_get_device_name(const efi_handle_t handle, char *buf, int size);
>
>  /**
> - * efi_add_known_memory() - add memory banks to EFI memory map
> + * efi_add_known_memory() - add memory types to the EFI memory map
> + *
> + * This function is to be used to adding different memory types other

/adding/add

> + * than EFI_CONVENTIONAL_MEMORY to the EFI memory map. The conventional
> + * memory is handled by the LMB module, and gets added to the memory

no comma after module

> + * map through the LMB module.
>   *
> - * This weak function may be overridden for specific architectures.
> + * This function may be overridden for specific architectures.

Since you are rewriting this I think for "architecture specific
purposes" is better

>   */
>  void efi_add_known_memory(void);
>
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index 974ecc51113..0f25094dbf8 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -780,82 +780,17 @@ efi_status_t efi_get_memory_map_alloc(efi_uintn_t *map_size,
>  }
>
>  /**
> - * efi_add_conventional_memory_map() - add a RAM memory area to the map
> + * efi_add_known_memory() - add memory types to the EFI memory map
>   *
> - * @ram_start:         start address of a RAM memory area
> - * @ram_end:           end address of a RAM memory area
> - * @ram_top:           max address to be used as conventional memory
> - * Return:             status code
> - */
> -efi_status_t efi_add_conventional_memory_map(u64 ram_start, u64 ram_end,
> -                                            u64 ram_top)
> -{
> -       u64 pages;
> -
> -       /* Remove partial pages */
> -       ram_end &= ~EFI_PAGE_MASK;
> -       ram_start = (ram_start + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
> -
> -       if (ram_end <= ram_start) {
> -               /* Invalid mapping */
> -               return EFI_INVALID_PARAMETER;
> -       }
> -
> -       pages = (ram_end - ram_start) >> EFI_PAGE_SHIFT;
> -
> -       efi_add_memory_map_pg(ram_start, pages,
> -                             EFI_CONVENTIONAL_MEMORY, false);
> -
> -       /*
> -        * Boards may indicate to the U-Boot memory core that they
> -        * can not support memory above ram_top. Let's honor this
> -        * in the efi_loader subsystem too by declaring any memory
> -        * above ram_top as "already occupied by firmware".
> -        */
> -       if (ram_top < ram_start) {
> -               /* ram_top is before this region, reserve all */
> -               efi_add_memory_map_pg(ram_start, pages,
> -                                     EFI_BOOT_SERVICES_DATA, true);
> -       } else if (ram_top < ram_end) {
> -               /* ram_top is inside this region, reserve parts */
> -               pages = (ram_end - ram_top) >> EFI_PAGE_SHIFT;
> -
> -               efi_add_memory_map_pg(ram_top, pages,
> -                                     EFI_BOOT_SERVICES_DATA, true);
> -       }
> -
> -       return EFI_SUCCESS;
> -}
> -
> -/**
> - * efi_add_known_memory() - add memory banks to map
> + * This function is to be used to adding different memory types other
> + * than EFI_CONVENTIONAL_MEMORY to the EFI memory map. The conventional
> + * memory is handled by the LMB module, and gets added to the memory
> + * map through the LMB module.
>   *
>   * This function may be overridden for specific architectures.
>   */
>  __weak void efi_add_known_memory(void)

 We can also get rid of this in the future. Any idea if we have
platforms using it after the last 2 you removed?


>  {
> -       u64 ram_top = gd->ram_top & ~EFI_PAGE_MASK;
> -       int i;
> -
> -       /*
> -        * ram_top is just outside mapped memory. So use an offset of one for
> -        * mapping the sandbox address.
> -        */
> -       ram_top = (uintptr_t)map_sysmem(ram_top - 1, 0) + 1;
> -
> -       /* Fix for 32bit targets with ram_top at 4G */
> -       if (!ram_top)
> -               ram_top = 0x100000000ULL;
> -
> -       /* Add RAM */
> -       for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> -               u64 ram_end, ram_start;
> -
> -               ram_start = (uintptr_t)map_sysmem(gd->bd->bi_dram[i].start, 0);
> -               ram_end = ram_start + gd->bd->bi_dram[i].size;
> -
> -               efi_add_conventional_memory_map(ram_start, ram_end, ram_top);
> -       }
>  }
>
>  /**
> --
> 2.34.1
>

With the spelling nits fixed
Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>