[PATCH v4 0/9] Adjust initial EFI memory-allocation to be in the U-Boot region
U-Boot relocates itself to the top of memory and is supposed to avoid using any memory below that region. This allows the maximum amount of memory to be available for loading images.
The lmb subsystem automatically protects U-Boot's code and data from being overwritten. It relies on this region being a contiguous area of memory. This setup has worked well for at least 15 years and has been documented in the README for as long.
The EFI_LOADER subsystem currently uses memory outside the U-Boot region when starting up.
Some of the downstream problems this causes are:
1. It can overwrite images that have been loaded 2. Requires lmb reservations to avoid (1) even if EFI_LOADER is not being used 3. Uses regions which scripts expect to be available, e.g. the existing kernel_addr_r variables 4. Introduces confusion as to what memory U-Boot can use for itself
This series sets up a very small region of memory which EFI can used at start-up, thus avoiding touching memory outside the U-Boot region.
There are still a few tables which are not in the bloblist, but that will be resolved separately.
Other than tables, the amount of API-visible memory allocated by EFI before booting is actually very small: 3 allocations, of a total of about 250 bytes. However, there are some areas where EFI allocations are done unnecessarily. For example efi_disk_add_dev() uses page-allocation to allocate an efi_device_path, then immediately frees it again. In this case it would be better to use malloc(). Since each allocation consumes a page (4KB), 256KB has been chosen for the size here, enough for 64 allocations.
As soon as efi_init_obj_list() is called, we know that the EFI subsystem is to be used, so we can relax the restrictions and allow EFI to use any memory it likes. This is handled using a simple boolean flag in BSS, since it cannot happen before relocation.
Changes in v5: - Drop the word 'warning'
Changes in v4: - Expand the commit message - Reword the commit message since this feature is needed - 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 v3: - Add new patch to drop the memset() from efi_alloc() - Drop patch to convert device_path allocation to use malloc()
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
Simon Glass (9): kernel: Document ALIGN macros board_f: Correct stack reservation board_f: Rename reserve_stack_aligned() efi: Drop the memset() from efi_alloc() efi: Show the location of the bounce buffer when debugging event: Add a generic way to reserve memory efi: Reserve some memory for initial use efi: Keep early allocations to the U-Boot region wip
cmd/meminfo.c | 7 +++ common/board_f.c | 15 ++--- common/event.c | 1 + include/asm-generic/global_data.h | 28 +++++++++ include/efi_loader.h | 33 ++++++++++- include/event.h | 11 ++++ include/linux/kernel.h | 31 ++++++++++ include/lmb.h | 12 ++++ lib/efi_loader/efi_bootbin.c | 9 +++ lib/efi_loader/efi_memory.c | 98 +++++++++++++++++++++++++++++-- lib/efi_loader/efi_setup.c | 5 ++ lib/lmb.c | 12 ++++ test/py/tests/test_event_dump.py | 1 + 13 files changed, 249 insertions(+), 14 deletions(-)
Add comments for these macros, so it is clear what they do.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
include/linux/kernel.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 9467edd65ab..e5557257e6f 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -35,9 +35,40 @@
#define REPEAT_BYTE(x) ((~0ul / 0xff) * (x))
+/** + * Rounds a value up to a given alignment + * + * Note that @a must be a power of two + * + * Returns the lowest value >= @x which is a multiple of @a + * + * For example ALIGN(0x123, 8) == 0x128 + */ #define ALIGN(x,a) __ALIGN_MASK((x),(typeof(x))(a)-1) + +/** + * Rounds a down to a given alignment + * + * Note that @a must be a power of two + * + * Returns the highest value <= @x which is a multiple of @a + * + * For example ALIGN_DOWN(0x123, 8) == 0x120 + * ALIGN_DOWN(0x120, 16) == 0x120 + */ #define ALIGN_DOWN(x, a) ALIGN((x) - ((a) - 1), (a)) + +/** + * Rounds a value up to a given alignment-mask +* + * Note that @mask must be one less than a power of two + * + * Returns the lowest value >= @x where the bits set in @mask are 0 + * + * For example __ALIGN_MASK(0x123, 0xf) == 0x130 + */ #define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask)) + #define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a))) #define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
On 11/3/24 01:28, Simon Glass wrote:
Add comments for these macros, so it is clear what they do.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
include/linux/kernel.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 9467edd65ab..e5557257e6f 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -35,9 +35,40 @@
#define REPEAT_BYTE(x) ((~0ul / 0xff) * (x))
+/**
- Rounds a value up to a given alignment
- Note that @a must be a power of two
- Returns the lowest value >= @x which is a multiple of @a
- For example ALIGN(0x123, 8) == 0x128
- */
Thanks for describing the macros.
We follow https://docs.kernel.org/doc-guide/kernel-doc.html#function-documentation for function-like macros:
/** * ALIGN() - Rounds a value up to a given alignment * * @x: value to be rounded up * @a: alignment factor. @a must be a power of two. * Return: lowest value >= @x which is a multiple of @a * * For example ALIGN(0x123, 8) == 0x128. */
Please, apply the same to all macros.
On the webpage there is also a rule for object like macros.
Best regards
Heinrich
#define ALIGN(x,a) __ALIGN_MASK((x),(typeof(x))(a)-1)
+/**
- Rounds a down to a given alignment
- Note that @a must be a power of two
- Returns the highest value <= @x which is a multiple of @a
- For example ALIGN_DOWN(0x123, 8) == 0x120
- ALIGN_DOWN(0x120, 16) == 0x120
- */ #define ALIGN_DOWN(x, a) ALIGN((x) - ((a) - 1), (a))
+/**
- Rounds a value up to a given alignment-mask
+*
- Note that @mask must be one less than a power of two
- Returns the lowest value >= @x where the bits set in @mask are 0
- For example __ALIGN_MASK(0x123, 0xf) == 0x130
- */ #define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask))
- #define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a))) #define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
The reserve_stack_aligned() function already ensures that the resulting address is aligned to a 16-byte boundary. The comment seems to suggest that 16 is passed reserve_stack_aligned() to make it aligned.
Change the value to 0, since the stack can start at the current address, if it is suitably aligned already.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
common/board_f.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 98dc2591e1d..677e37d93c0 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -601,7 +601,7 @@ __weak int arch_reserve_stacks(void) static int reserve_stacks(void) { /* make stack pointer 16-byte aligned */ - gd->start_addr_sp = reserve_stack_aligned(16); + gd->start_addr_sp = reserve_stack_aligned(0);
/* * let the architecture-specific code tailor gd->start_addr_sp and
On 11/3/24 01:28, Simon Glass wrote:
The reserve_stack_aligned() function already ensures that the resulting address is aligned to a 16-byte boundary. The comment seems to suggest that 16 is passed reserve_stack_aligned() to make it aligned.
Change the value to 0, since the stack can start at the current address, if it is suitably aligned already.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
common/board_f.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 98dc2591e1d..677e37d93c0 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -601,7 +601,7 @@ __weak int arch_reserve_stacks(void) static int reserve_stacks(void) { /* make stack pointer 16-byte aligned */
- gd->start_addr_sp = reserve_stack_aligned(16);
- gd->start_addr_sp = reserve_stack_aligned(0);
In 1938f4a5b62f ("Introduce generic pre-relocation board_f.c") you wrote:
/* setup stack pointer for exceptions */ gd->dest_addr_sp -= 16; /* leave 3 words for abort-stack, plus 1 for alignment */ gd->dest_addr_sp -= 16;
Is that not relevant anymore?
Does all and every U-Boot architecture decrement the stack pointer before writing values to the stack?
E.g. Microblaze, MIPS, RISC-V, and Super-H do not have a dedicated PUSH instruction.
Best regards
Heinrich
/* * let the architecture-specific code tailor gd->start_addr_sp and
Hi Heinrich,
On Sun, 3 Nov 2024 at 10:23, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/3/24 01:28, Simon Glass wrote:
The reserve_stack_aligned() function already ensures that the resulting address is aligned to a 16-byte boundary. The comment seems to suggest that 16 is passed reserve_stack_aligned() to make it aligned.
Change the value to 0, since the stack can start at the current address, if it is suitably aligned already.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
common/board_f.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 98dc2591e1d..677e37d93c0 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -601,7 +601,7 @@ __weak int arch_reserve_stacks(void) static int reserve_stacks(void) { /* make stack pointer 16-byte aligned */
gd->start_addr_sp = reserve_stack_aligned(16);
gd->start_addr_sp = reserve_stack_aligned(0);In 1938f4a5b62f ("Introduce generic pre-relocation board_f.c") you wrote:
/* setup stack pointer for exceptions */ gd->dest_addr_sp -= 16; /* leave 3 words for abort-stack, plus 1 for alignment */ gd->dest_addr_sp -= 16;
Is that not relevant anymore?
Well that looks like ARM code that has moved to arch_reserve_stacks() some time ago.
Does all and every U-Boot architecture decrement the stack pointer before writing values to the stack?
E.g. Microblaze, MIPS, RISC-V, and Super-H do not have a dedicated PUSH instruction.
I'm not sure about that, but they certainly need to. I see that powerpc seems to write to values above the stack top! I don't think we have enough people on this patch to look into powerpc, at least.
For RISC-V, does it use a full-descending stack, or empty? Looking at relocate_code() I don't see it subtracting anything from the stack-pointer that is passed in.
Regards, SImon
Most of the calls to this function have nothing to do with the stack. Rename the function to avoid confusion.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
common/board_f.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index 677e37d93c0..3532ffec46f 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -492,7 +492,7 @@ static int reserve_uboot(void) * ref = x86_64 ABI: https://reviews.llvm.org/D30049: 16 bytes * = ARMv8 Instruction Set Overview: quad word, 16 bytes */ -static unsigned long reserve_stack_aligned(size_t size) +static unsigned long reserve_aligned(size_t size) { return ALIGN_DOWN(gd->start_addr_sp - size, 16); } @@ -522,7 +522,7 @@ static int reserve_noncached(void) /* reserve memory for malloc() area */ static int reserve_malloc(void) { - gd->start_addr_sp = reserve_stack_aligned(TOTAL_MALLOC_LEN); + gd->start_addr_sp = reserve_aligned(TOTAL_MALLOC_LEN); debug("Reserving %dk for malloc() at: %08lx\n", TOTAL_MALLOC_LEN >> 10, gd->start_addr_sp); #ifdef CONFIG_SYS_NONCACHED_MEMORY @@ -536,7 +536,7 @@ static int reserve_malloc(void) static int reserve_board(void) { if (!gd->bd) { - gd->start_addr_sp = reserve_stack_aligned(sizeof(struct bd_info)); + gd->start_addr_sp = reserve_aligned(sizeof(struct bd_info)); gd->bd = (struct bd_info *)map_sysmem(gd->start_addr_sp, sizeof(struct bd_info)); memset(gd->bd, '\0', sizeof(struct bd_info)); @@ -548,7 +548,7 @@ static int reserve_board(void)
static int reserve_global_data(void) { - gd->start_addr_sp = reserve_stack_aligned(sizeof(gd_t)); + gd->start_addr_sp = reserve_aligned(sizeof(gd_t)); gd->new_gd = (gd_t *)map_sysmem(gd->start_addr_sp, sizeof(gd_t)); debug("Reserving %zu Bytes for Global Data at: %08lx\n", sizeof(gd_t), gd->start_addr_sp); @@ -567,7 +567,7 @@ static int reserve_fdt(void) gd->boardf->fdt_size = ALIGN(fdt_totalsize(gd->fdt_blob), 32);
- gd->start_addr_sp = reserve_stack_aligned( + gd->start_addr_sp = reserve_aligned( gd->boardf->fdt_size); gd->boardf->new_fdt = map_sysmem(gd->start_addr_sp, gd->boardf->fdt_size); @@ -584,7 +584,7 @@ static int reserve_bootstage(void) #ifdef CONFIG_BOOTSTAGE int size = bootstage_get_size(true);
- gd->start_addr_sp = reserve_stack_aligned(size); + gd->start_addr_sp = reserve_aligned(size); gd->boardf->new_bootstage = map_sysmem(gd->start_addr_sp, size); debug("Reserving %#x Bytes for bootstage at: %08lx\n", size, gd->start_addr_sp); @@ -601,7 +601,7 @@ __weak int arch_reserve_stacks(void) static int reserve_stacks(void) { /* make stack pointer 16-byte aligned */ - gd->start_addr_sp = reserve_stack_aligned(0); + gd->start_addr_sp = reserve_aligned(0);
/* * let the architecture-specific code tailor gd->start_addr_sp and
From my inspection none of the users need the memory to be zeroed. It
is somewhat unexpected that it does so, since the name gives no clue to this.
Drop the memset() so that it effectively becomes a wrapper around the normal EFI-pool allocator.
Another option would be to drop this function and call efi_allocate_pool() directly, but that increase code size a little.
Move the function comment to the header file like most other exported functions in U-Boot.
Comments were made in v3 that another project uses memset() when allocating memory, but that is not required by the spec. In any case, as above, from inspection, none of the users need the memory to be zeroed, as they fill the entire region with their own data.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v4)
Changes in v4: - Expand the commit message
Changes in v3: - Add new patch to drop the memset() from efi_alloc() - Drop patch to convert device_path allocation to use malloc()
include/efi_loader.h | 11 ++++++++++- lib/efi_loader/efi_memory.c | 1 - 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 291eca5c077..f940dc81345 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -758,8 +758,17 @@ efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf, * Return: size in pages */ #define efi_size_in_pages(size) (((size) + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT) -/* Allocate boot service data pool memory */ + +/** + * efi_alloc() - allocate boot services data pool memory + * + * Allocate memory from pool with type EFI_BOOT_SERVICES_DATA + * + * @size: number of bytes to allocate + * Return: pointer to allocated memory, or NULL if out of memory + */ void *efi_alloc(size_t len); + /* Allocate pages on the specified alignment */ void *efi_alloc_aligned_pages(u64 len, int memory_type, size_t align); /* More specific EFI memory allocator, called by EFI payloads */ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 2982c45dc38..8b086b165a7 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -696,7 +696,6 @@ void *efi_alloc(size_t size) log_err("out of memory\n"); return NULL; } - memset(buf, 0, size);
return buf; }
On 11/3/24 01:28, Simon Glass wrote:
From my inspection none of the users need the memory to be zeroed. It is somewhat unexpected that it does so, since the name gives no clue to this.
Drop the memset() so that it effectively becomes a wrapper around the normal EFI-pool allocator.
Another option would be to drop this function and call efi_allocate_pool() directly, but that increase code size a little.
Move the function comment to the header file like most other exported functions in U-Boot.
Comments were made in v3 that another project uses memset() when allocating memory, but that is not required by the spec. In any case, as above, from inspection, none of the users need the memory to be zeroed, as they fill the entire region with their own data.
As already written in response to v3 of the series I want to be sure that code that runs on EDK II does not fail on U-Boot.
EFI_DEVICE_PATH_UTILITIES_PROTOCOL.CreateDeviceNode() in EDK II zeros out the returned buffer. We implement this function via efi_alloc().
Here is a code example relying on a zeroed out node:
https://github.com/acidanthera/gfxutil/blob/5284a662181ff4b81dd19d1279aedb68...
Please, drop the patch.
I have created issue USWG 0002487 https://mantis.uefi.org/mantis/view.php?id=2487) asking to indicate in the specification if the returned device path node should be zeroed out by the firmware.
Best regards
Heinrich
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v4)
Changes in v4:
- Expand the commit message
Changes in v3:
Add new patch to drop the memset() from efi_alloc()
Drop patch to convert device_path allocation to use malloc()
include/efi_loader.h | 11 ++++++++++- lib/efi_loader/efi_memory.c | 1 - 2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 291eca5c077..f940dc81345 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -758,8 +758,17 @@ efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf,
- Return: size in pages
*/ #define efi_size_in_pages(size) (((size) + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT) -/* Allocate boot service data pool memory */
+/**
- efi_alloc() - allocate boot services data pool memory
- Allocate memory from pool with type EFI_BOOT_SERVICES_DATA
- @size: number of bytes to allocate
- Return: pointer to allocated memory, or NULL if out of memory
- */ void *efi_alloc(size_t len);
- /* Allocate pages on the specified alignment */ void *efi_alloc_aligned_pages(u64 len, int memory_type, size_t align); /* More specific EFI memory allocator, called by EFI payloads */
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 2982c45dc38..8b086b165a7 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -696,7 +696,6 @@ void *efi_alloc(size_t size) log_err("out of memory\n"); return NULL; }
memset(buf, 0, size);
return buf; }
Hi Heinrich,
On Sun, 3 Nov 2024 at 01:54, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/3/24 01:28, Simon Glass wrote:
From my inspection none of the users need the memory to be zeroed. It is somewhat unexpected that it does so, since the name gives no clue to this.
Drop the memset() so that it effectively becomes a wrapper around the normal EFI-pool allocator.
Another option would be to drop this function and call efi_allocate_pool() directly, but that increase code size a little.
Move the function comment to the header file like most other exported functions in U-Boot.
Comments were made in v3 that another project uses memset() when allocating memory, but that is not required by the spec. In any case, as above, from inspection, none of the users need the memory to be zeroed, as they fill the entire region with their own data.
As already written in response to v3 of the series I want to be sure that code that runs on EDK II does not fail on U-Boot.
EFI_DEVICE_PATH_UTILITIES_PROTOCOL.CreateDeviceNode() in EDK II zeros out the returned buffer. We implement this function via efi_alloc().
Here is a code example relying on a zeroed out node:
https://github.com/acidanthera/gfxutil/blob/5284a662181ff4b81dd19d1279aedb68...
Please, drop the patch.
I have created issue USWG 0002487 https://mantis.uefi.org/mantis/view.php?id=2487) asking to indicate in the specification if the returned device path node should be zeroed out by the firmware.
OK thanks.
But please ignore this patch. The whole series was sent by mistake.
Regards, Simon
The EFI_LOADER_BOUNCE_BUFFER feature was added many years ago. It is still needed and 24 boards (lx2160ardb_tfa_stmm, lx2162aqds_tfa_SECURE_BOOT and the like) use it.
This feature uses EFI page allocation to create a 64MB buffer 'in space' without any knowledge of where boards intend to load their images. This may result in image corruption or other problems.
For example, if the feature is enabled on qemu_arm64 it puts the EFI bounce buffer at 1045MB, with the kernel at 1028MB and the ramdisk at 1088MB. The kernel is probably smaller than 27MB but the buffer does overlap the ramdisk.
The solution is probably to use BOUNCE_BUFFER instead, with the EFI version being dropped. For now, show the address of the EFI bounce buffer so people have a better chance to debug any problem.
Signed-off-by: Simon Glass sjg@chromium.org --- In response to questions on how to show this problem:
First, add CONFIG_EFI_LOADER_BOUNCE_BUFFER=y to configs/qemu_arm64_defconfig and build
Then, to run: qemu-system-aarch64 -machine virt -nographic -cpu cortex-a57 \ -bios u-boot.bin
U-Boot 2024.10-00809-gae29e5f76e4a-dirty (Oct 11 2024 - 11:48:46 -0600)
DRAM: 128 MiB Core: 51 devices, 14 uclasses, devicetree: board Flash: 64 MiB Loading Environment from Flash... *** Warning - bad CRC, using default environment
In: serial,usbkbd Out: serial,vidconsole Err: serial,vidconsole No USB controllers found Net: eth0: virtio-net#32
starting USB... No USB controllers found Hit any key to stop autoboot: 0 => bootefi hello Enabling coop memory No EFI system partition No EFI system partition Failed to persist EFI variables Missing TPMv2 device for EFI_TCG_PROTOCOL Missing RNG device for EFI_RNG_PROTOCOL Warning: EFI bounce buffer 000000004157f000-000000004557f000 Not loaded from disk Booting /MemoryMapped(0x0,0x47763730,0x31b0) EFI: Call: efi_load_image(false, efi_root, file_path, source_buffer, source_size, &handle) EFI: 0 returned by efi_load_image(false, efi_root, file_path, source_buffer, source_size, &handle) Hello, world! Running on UEFI 2.10 Firmware vendor: Das U-Boot Firmware revision: 20241000 Have device tree Load options: <none> File path: <none> Boot device: /MemoryMapped(0x0,0x47763730,0x31b0) => print kernel_addr_r kernel_addr_r=0x40400000 => print ramdisk_addr_r ramdisk_addr_r=0x44000000 =>
The EFI bounce buffer overlaps with the ramdisk.
Changes in v5: - Drop the word 'warning'
Changes in v4: - Reword the commit message since this feature is needed
lib/efi_loader/efi_bootbin.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index a87006b3c0e..6f02c5ed5dd 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -209,6 +209,15 @@ efi_status_t efi_binary_run(void *image, size_t size, void *fdt) return -1; }
+#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER + /* + * Add a warning about this buffer, since it may conflict with other + * things + */ + log_debug("EFI bounce buffer %p-%p\n", efi_bounce_buffer, + efi_bounce_buffer + EFI_LOADER_BOUNCE_BUFFER_SIZE); +#endif + ret = efi_install_fdt(fdt); if (ret != EFI_SUCCESS) return ret;
Hi,
On Sat, 2 Nov 2024 at 18:29, Simon Glass sjg@chromium.org wrote:
U-Boot relocates itself to the top of memory and is supposed to avoid using any memory below that region. This allows the maximum amount of memory to be available for loading images.
The lmb subsystem automatically protects U-Boot's code and data from being overwritten. It relies on this region being a contiguous area of memory. This setup has worked well for at least 15 years and has been documented in the README for as long.
The EFI_LOADER subsystem currently uses memory outside the U-Boot region when starting up.
Some of the downstream problems this causes are:
- It can overwrite images that have been loaded
- Requires lmb reservations to avoid (1) even if EFI_LOADER is not being used
- Uses regions which scripts expect to be available, e.g. the existing kernel_addr_r variables
- Introduces confusion as to what memory U-Boot can use for itself
This series sets up a very small region of memory which EFI can used at start-up, thus avoiding touching memory outside the U-Boot region.
There are still a few tables which are not in the bloblist, but that will be resolved separately.
Other than tables, the amount of API-visible memory allocated by EFI before booting is actually very small: 3 allocations, of a total of about 250 bytes. However, there are some areas where EFI allocations are done unnecessarily. For example efi_disk_add_dev() uses page-allocation to allocate an efi_device_path, then immediately frees it again. In this case it would be better to use malloc(). Since each allocation consumes a page (4KB), 256KB has been chosen for the size here, enough for 64 allocations.
As soon as efi_init_obj_list() is called, we know that the EFI subsystem is to be used, so we can relax the restrictions and allow EFI to use any memory it likes. This is handled using a simple boolean flag in BSS, since it cannot happen before relocation.
Changes in v5:
- Drop the word 'warning'
Changes in v4:
- Expand the commit message
- Reword the commit message since this feature is needed
- 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 v3:
- Add new patch to drop the memset() from efi_alloc()
- Drop patch to convert device_path allocation to use malloc()
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
Simon Glass (9): kernel: Document ALIGN macros board_f: Correct stack reservation board_f: Rename reserve_stack_aligned() efi: Drop the memset() from efi_alloc() efi: Show the location of the bounce buffer when debugging event: Add a generic way to reserve memory efi: Reserve some memory for initial use efi: Keep early allocations to the U-Boot region wip
cmd/meminfo.c | 7 +++ common/board_f.c | 15 ++--- common/event.c | 1 + include/asm-generic/global_data.h | 28 +++++++++ include/efi_loader.h | 33 ++++++++++- include/event.h | 11 ++++ include/linux/kernel.h | 31 ++++++++++ include/lmb.h | 12 ++++ lib/efi_loader/efi_bootbin.c | 9 +++ lib/efi_loader/efi_memory.c | 98 +++++++++++++++++++++++++++++-- lib/efi_loader/efi_setup.c | 5 ++ lib/lmb.c | 12 ++++ test/py/tests/test_event_dump.py | 1 + 13 files changed, 249 insertions(+), 14 deletions(-)
Ignore this, sent by mistake due to my getting confused by a flaky internet connection.
Regards, Simon
participants (2)
-
Heinrich Schuchardt -
Simon Glass