[...]
> > > +
> > > +/**
> > > + * 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.
Yes, but what does it 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;
> > > +
I am not sure that will work. I'll have to check if not adding the
runtime regions from the linker scripts during bringup is safe.
But, instead of adding all that, including a small gd region for
efi_region and having to explicitly make that region bigger every time
something new gets added on efi allocations, why don't we simply move
efi_memory_init() out of init_sequence_r()?
We can protect the runtime services etc via LMB now and initialize the
memory map in efi_init_obj_list().
I haven't gone through all the code, but it sounds doable
Thanks
/Ilias
> > > /* Set up console modes */
> > > efi_setup_console_size();
> > >
> > > --
> > > 2.34.1
> > >
> >
> > Thanks
> > /Ilias
>
> Regards,
> SImon
