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

[Simon Glass]

Hi Ilias,

On Mon, 14 Oct 2024 at 22:26, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> 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.

Indeed.

>
> >
> > 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.

Yes. At that point we know we are using an EFI feature. We can
certainly extend the intent of this patch to cover that, but I am
trying to take baby steps here.

> 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

Basically it feeds a small amount of memory to EFI initially, within
the U-Boot region. That is enough to deal with the few allocations
that are done before efi_init_obj_list() is called.

So if you don't use any EFI features, EFI's memory usage is limited to
the U-Boot region.

>
> > + *
> > + * 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

Regards,
SImon