Re: [PATCH 6/6] efi: Show the location of the bounce buffer

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 25.07.24 15:56, Simon Glass wrote:
> 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.

This is what Sughosh's LMB series in addressing.

With CONFIG_SYS_MALLOC_LEN=0x202000 for lx2160ardb_tfa_stmm we cannot
move the buffer to malloc(). I don't understand why
CONFIG_SYS_MALLOC_LEN is so small.

>
> 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.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   lib/efi_loader/efi_bootbin.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c
> index 07c8fca68cc..53e5e429d2e 100644
> --- a/lib/efi_loader/efi_bootbin.c
> +++ b/lib/efi_loader/efi_bootbin.c
> @@ -211,6 +211,14 @@ 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
> +	 */
> +	printf("EFI bounce buffer at %p\n", efi_bounce_buffer);
> +#endif

Does this sound like a warning to you?

There is nothing that a user can reasonably do when seeing this message
while the board is booting via one of the boot methods. So we should not
write it.

Best regards

Heinrich


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