Re: [PATCH v4 2/5] efi: Show the location of the bounce buffer when debugging

[Heinrich Schuchardt]

Am 11. Oktober 2024 23:21:23 MESZ schrieb Simon Glass <sjg at chromium.org>:
>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.
>
>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..eefdd660cee 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("Warning: EFI bounce buffer %p-%p\n", efi_bounce_buffer,
>+		  efi_bounce_buffer + EFI_LOADER_BOUNCE_BUFFER_SIZE);
>+#endif

Adding a debug message here is ok.

But writing "warning" in a debug message which is just printing an address for a configuration selected item looks weird to me.

Best regards

Heinrich

>+
> 	ret = efi_install_fdt(fdt);
> 	if (ret != EFI_SUCCESS)
> 		return ret;