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

[Tom Rini]

On Mon, Oct 14, 2024 at 02:20:09PM -0600, Simon Glass wrote:
> Hi Heinrich,
> 
> On Fri, 11 Oct 2024 at 16:02, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >
> >
> >
> > 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.
> 
> I added that because you said it needed to say it was a warning. I'll
> remove it and try again.

Since we talked about it on IRC, the most useful option here would be
instead to switch this to the regular CONFIG_BOUNCE_BUFFER code so that
it can go away entirely. The second most useful option is yes, we can
put a debug message for when someone enables "make a hole in memory for
me" when they do not need that hole made in memory.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20241014/e2b27e12/attachment.sig>