| Message ID | 20241011212126.747741-6-sjg@chromium.org |
|---|---|
| State | New |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Adjust initial EFI memory-allocation to be in the U-Boot region | expand |
Hi Simon, On Sat, 12 Oct 2024 at 00:23, Simon Glass <sjg@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@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
Hi Ilias, On Mon, 14 Oct 2024 at 22:26, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > On Sat, 12 Oct 2024 at 00:23, Simon Glass <sjg@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@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
[...] > > > + > > > +/** > > > + * 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
Hi Ilias, On Wed, 16 Oct 2024 at 08:02, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > [...] > > > > > + > > > > +/** > > > > + * 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? Just that. U-Boot should not use memory below its own region, that's all. > > > > > > > > > > > + * > > > > + * 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 Yes that sounds like a good idea...but there is a catch. There are few allocations that happen when U-Boot is running - e.g. partitions. I think we need to work towards that bit by bit. Regards, Simon > > Thanks > /Ilias > > > > > /* Set up console modes */ > > > > efi_setup_console_size(); > > > > > > > > -- > > > > 2.34.1 > > > > > > > > > > Thanks > > > /Ilias > > > > Regards, > > SImon
Hi Simon On Sat, 19 Oct 2024 at 21:07, Simon Glass <sjg@chromium.org> wrote: > > Hi Ilias, > > On Wed, 16 Oct 2024 at 08:02, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > [...] > > > > > > > + > > > > > +/** > > > > > + * 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? > > Just that. U-Boot should not use memory below its own region, that's all. That's not a problem though. Before the subsystem bringup the only thing that gets initialized is the memory map > > > > > > > > > > > > > > > > > + * > > > > > + * 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 > > Yes that sounds like a good idea...but there is a catch. There are few > allocations that happen when U-Boot is running - e.g. partitions. I > think we need to work towards that bit by bit. There's also the early capsule updates that might need this -- hence the memory init really early. I'll have a look at this instead of splitting the memory inits Thanks /Ilias > > Regards, > Simon > > > > > Thanks > > /Ilias > > > > > > > /* Set up console modes */ > > > > > efi_setup_console_size(); > > > > > > > > > > -- > > > > > 2.34.1 > > > > > > > > > > > > > Thanks > > > > /Ilias > > > > > > Regards, > > > SImon
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 + * + * 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();
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. Signed-off-by: Simon Glass <sjg@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(-)