[PATCH v4 5/5] efi: Keep early allocations to the U-Boot region

[Ilias Apalodimas]

Hi Simon,

On Sat, 12 Oct 2024 at 00:23, Simon Glass <sjg at chromium.org> wrote:
>
> Adjust EFI_LOADER to use the provided region for early memory
> allocations, to avoid going outside the U-Boot region.
>
> Expand the available memory when efi_init_obj_list() is called.

efi_init_obj_list() is not called when an EFI app is called. It's
called when the EFI subsystem comes up.

>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v4:
> - Use a different technique to keep the memory-usage in place
> - Drop changes to pool allocation
> - Reorder the patches
> - Rewrite the cover letter
> - Make it a debug message for now
>
> Changes in v2:
> - Drop patch 'Show more information in efi index'
> - Drop patch 'Avoid pool allocation in efi_get_memory_map_alloc()'
> - Add the word 'warning', use log_warning() and show the end address
>
>  include/efi_loader.h        | 13 +++++++++++++
>  lib/efi_loader/efi_memory.c | 24 ++++++++++++++++++++++--
>  lib/efi_loader/efi_setup.c  |  5 +++++
>  3 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 08e27e61b06..aa464e9d754 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -805,6 +805,19 @@ int efi_disk_probe(void *ctx, struct event *event);
>  int efi_disk_remove(void *ctx, struct event *event);
>  /* Called by board init to initialize the EFI memory map */
>  int efi_memory_init(void);
> +
> +/**
> + * efi_memory_coop() - Allow EFI to use all available memory
> + *
> + * Up until this function is called, only a small portion of memory is available
> + * for use by the EFI memory-allocator. This function is called at the
> + * 'point of cooperation', before jumping into an EFI app, which needs to be
> + * able to make use of all the memory in the machine

As above, this is called when the EFI subsystem comes up. Not when an
application is called. For example doing a 'printenv -e' to print EFI
variables would trigger this.
The funtion call you are removing just adds all memory all memory as
EFI_CONVENTIONAL_MEMORY, which basically means free memory. I can't
understand what this patch is supposed to fix

> + *
> + * Return: efi_status_t (EFI_SUCCESS on success)
> + */
> +int efi_memory_coop(void);
> +
>  /* Adds new or overrides configuration table entry to the system table */
>  efi_status_t efi_install_configuration_table(const efi_guid_t *guid, void *table);
>  /* Sets up a loaded image */
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index 9cc33397371..2cd0b00e9eb 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -30,6 +30,9 @@ DECLARE_GLOBAL_DATA_PTR;
>   */
>  #define EFI_EARLY_REGION_SIZE  SZ_256K
>
> +/* Set when all memory has been added for use by EFI */
> +static bool efi_full_memory;
> +
>  efi_uintn_t efi_memory_map_key;
>
>  struct efi_mem_list {
> @@ -932,8 +935,25 @@ static void add_u_boot_and_runtime(void)
>
>  int efi_memory_init(void)
>  {
> -       efi_add_known_memory();
> +       efi_status_t ret;
> +
> +       /* restrict EFI to its own region until efi_memory_coop() is called */
> +       ret = efi_add_memory_map_pg((ulong)map_sysmem(gd->efi_region, 0),
> +                                   EFI_EARLY_REGION_SIZE >> EFI_PAGE_SHIFT,
> +                                   EFI_CONVENTIONAL_MEMORY, false);
> +
> +       return ret;
> +}
>
> +int efi_memory_coop(void)
> +{
> +       if (efi_full_memory)
> +               return 0;
> +       log_debug("Enabling coop memory\n");
> +
> +       efi_full_memory = true;
> +
> +       efi_add_known_memory();
>         add_u_boot_and_runtime();
>
>  #ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> @@ -943,7 +963,7 @@ int efi_memory_init(void)
>         if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_BOOT_SERVICES_DATA,
>                                (64 * 1024 * 1024) >> EFI_PAGE_SHIFT,
>                                &efi_bounce_buffer_addr) != EFI_SUCCESS)
> -               return -1;
> +               return -ENOMEM;
>
>         efi_bounce_buffer = (void*)(uintptr_t)efi_bounce_buffer_addr;
>  #endif
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index a610e032d2f..40e23ca104c 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -228,6 +228,11 @@ efi_status_t efi_init_obj_list(void)
>         if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
>                 return efi_obj_list_initialized;
>
> +       /* Allow EFI to use all memory */
> +       ret = efi_memory_coop();
> +       if (ret != EFI_SUCCESS)
> +               goto out;
> +
>         /* Set up console modes */
>         efi_setup_console_size();
>
> --
> 2.34.1
>

Thanks
/Ilias