[PATCH v5 6/9] efi: Show the location of the bounce buffer when debugging
Heinrich Schuchardt
xypron.glpk at gmx.de
Tue Dec 3 13:41:06 CET 2024
On 03.12.24 13:06, Sughosh Ganu wrote:
> On Sun, 1 Dec 2024 at 20:58, Simon Glass <sjg at chromium.org> wrote:
>>
>> 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 at 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.
>
> Why can't we "allocate" sane values for the env variables in question,
> similar to what is being done for the apple and qcom boards -- you can
> refer to arch/arm/mach-apple/board.c:board_late_init(). We won't be
> facing these issues then. Eventually, there can be a common, platform
> agnostic function to initialise these values. So, just as a PoC, I
> added this code to the qemu-arm.c, and no longer observe any overlaps.
>
> diff --git a/board/emulation/qemu-arm/qemu-arm.c
> b/board/emulation/qemu-arm/qemu-arm.c
> index 6095cb02b23..bad787f7a65 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -109,6 +109,9 @@ int board_init(void)
>
> int board_late_init(void)
> {
> +
> + u32 status = 0;
> +
> /*
> * Make sure virtio bus is enumerated so that peripherals
> * on the virtio bus can be discovered by their drivers
> @@ -119,6 +122,14 @@ int board_late_init(void)
> if (CONFIG_IS_ENABLED(USB_KEYBOARD))
> usb_init();
>
> + status |= env_set_hex("loadaddr", lmb_alloc(SZ_2M, SZ_2M));
> + status |= env_set_hex("fdt_addr_r", lmb_alloc(SZ_2M, SZ_2M));
> + status |= env_set_hex("kernel_addr_r", lmb_alloc(SZ_2M, SZ_2M));
> + status |= env_set_hex("ramdisk_addr_r", lmb_alloc(SZ_8M, SZ_2M));
Hello Sughosh,
avoiding fixed addresses would make sense for most boards.
Your suggested memory reservations for the kernel and initrd are way too
small. On my system I find
decompressed kernel - 67 MiB
initrd - 77 MiB
scriptaddr is missing.
ARM have address restrictions (see function efi_get_max_initrd_addr()):
armv7:
max_init_rd_addr =
round_down(image_addr, SZ_4M) + SZ_512M
armv8:
#define VA_BITS_MIN (48)
max_init_rd_addr =
round_down(image_addr, SZ_1G) + (1UL << (VA_BITS_MIN - 1))
These are not modeled in your suggestion.
I would suggest to make a single <= 512 MiB allocation for both initrd
and kernel to keep both together and then assign the addresses in the
allocated area, e.g.
kerneladdr = lmb_alloc(SZ_512M, SZ_4M);
if (!kerneladdr)
goto err;
if (env_set_hex("kernel_addr_r", kerneladdr))
goto err;
if (env_set_hex("initrd_addr_r", kerneladdr + SZ_256M))
goto err;
Best regards
Heinrich
> +
> + if (status)
> + log_warning("late_init: Failed to set run time variables\n");
> +
> return 0;
> }
>
>
> U-Boot 2025.01-rc3-00042-gacab6e78aca7-dirty (Dec 03 2024 - 17:17:42 +0530)
>
> DRAM: 512 MiB
> EFI bounce buffer 000000005957f000-000000005d57f000
> 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#31
>
> starting USB...
> No USB controllers found
> Hit any key to stop autoboot: 0
>
> => bdinfo
>
> ...
>
> lmb_dump_all:
> memory.count = 0x1
> memory[0] [0x40000000-0x5fffffff], 0x20000000 bytes, flags: none
> reserved.count = 0x3
> reserved[0] [0x58600000-0x593fffff], 0xe00000 bytes, flags: none
> reserved[1] [0x5957c000-0x5d57efff], 0x4003000 bytes, flags:
> no-notify, no-overwrite
> reserved[2] [0x5d57fde0-0x5fffffff], 0x2a80220 bytes, flags: no-overwrite
>
> ...
>
> => printenv kernel_addr_r
> kernel_addr_r=58e00000
> => printenv ramdisk_addr_r
> ramdisk_addr_r=58600000
> => printenv fdt_addr_r
> fdt_addr_r=59000000
> => printenv loadaddr
> loadaddr=59200000
>
> -sughosh
>
>>
>> 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 7e7a6bf31aa..9a880251b47 100644
>> --- a/lib/efi_loader/efi_bootbin.c
>> +++ b/lib/efi_loader/efi_bootbin.c
>> @@ -208,6 +208,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;
>> --
>> 2.43.0
>>
More information about the U-Boot
mailing list