[PATCH v5 6/9] efi: Show the location of the bounce buffer when debugging
Tom Rini
trini at konsulko.com
Tue Dec 3 14:57:16 CET 2024
On Tue, Dec 03, 2024 at 06:45:32AM -0700, Simon Glass wrote:
> Hi Sughosh,
>
> On Tue, 3 Dec 2024 at 06:13, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> >
> > On Tue, 3 Dec 2024 at 18:22, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > >
> > > On Tue, 3 Dec 2024 at 18:11, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > >
> > > > 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;
> > >
> > > Yes, I simply wanted to illustrate here that there would not be an
> > > issue of overlaps if we used the lmb API's to get memory addresses. In
> > > fact, this logic that you have pasted above can be added through the
> > > reserve event function that Simon is introducing in this series -- one
> > > of the functions can reserve memory for all these env variables. I
> > > don't see the need to allocate some separate memory region for EFI
> > > though. Thanks.
> >
> > Although the size of the allocation might have to be controlled
> > through a config symbol, or computed at runtime, based on the size of
> > RAM. Not all platforms might have the same amount of memory.
> >
>
> We can use the size of the image we are loading, when known. When not
Please note that the size of what we're loading alone is not enough. The
common case of "execute it right there" means needing to look at the
header to see how much space BSS needs too. Happily "Image" formatted
images have this information now and we're not quite as stuck as we were
in older times.
> known (e.g. some tftp servers) we can set an address above the others
> we have used, so it can grow, then adjust the top once we know the
> size.
>
> But all of this is in the future, since a lot of boards are not using
> bootstd and we have to respect their env vars. The latest set of
> boards to move is sunxi, which is blocked in Tom's tree.
Yes, sunxi moving to bootstd is blocked in mainline because it's not
ready yet as it doesn't support all of the use cases sunxi has and no
one has implemented what was agreed on as the path forward there.
--
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/20241203/b2b973d9/attachment.sig>
More information about the U-Boot
mailing list