[PATCH v3 3/3] efi: Show the location of the bounce buffer

[Simon Glass]

Hi Heinrich,

On Mon, 2 Sept 2024 at 05:47, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
>
>
> Am 2. September 2024 00:22:59 MESZ schrieb Simon Glass <sjg at chromium.org>:
> >The EFI_LOADER_BOUNCE_BUFFER feature was added many years ago. It is not
> >clear whether it is still needed, but 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 detect the problem.
> >
> >Note: I avoided converting this #ifdef to use IS_ENABLED() since I hope
> >that the feature may be removed.
>
> The functionality is still needed. A warning is of little use: what should the user do about it? The region should be reserved in LMB.

I don't mind dropping this patch if we can get the other two applied.
I suppose my main purpose was to highlight this sort of issue and get
something in the commit log about it. For me, the message helped me
understand why memory regions were overlapping.

Regards,
Simon



>
> Best regards
>
> Heinrich
>
>
> >
> >Signed-off-by: Simon Glass <sjg at chromium.org>
> >---
> >
> >(no changes since v2)
> >
> >Changes in v2:
> >- Drop patch 'Show more information in efi index'
> >- Drop patch 'Avoid pool allocation in efi_get_memory_map_alloc()'
> >- Add the word 'warning', use log_warning() and show the end address
> >
> > 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 5bb0fdcf75d..9779dc09b5e 100644
> >--- a/lib/efi_loader/efi_bootbin.c
> >+++ b/lib/efi_loader/efi_bootbin.c
> >@@ -211,6 +211,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_warning("Warning: 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;