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.g...@gmx.de> wrote:
> >
> >
> >
> > Am 11. Oktober 2024 23:21:23 MESZ schrieb Simon Glass <s...@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 <s...@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
signature.asc
Description: PGP signature
