efi_loader: remove EFI_BOUNCE_BUFFER

The EFI subsystem defines its own bounce buffer for devices that
can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
which can be reused instead of defining another symbol.
The only limitation for EFI is that we don't know how big the file a user
chooses to transfer is and as a result we can't depend on allocating the
memory from the malloc area, which can prove too small.

So allocate an EFI buffer of the correct size and pass it to the DM,
which already supports bounce buffering via bounce_buffer_start_extalign()

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 arch/arm/Kconfig                              |  8 ++
 configs/ls1028aqds_tfa_SECURE_BOOT_defconfig  |  1 -
 configs/ls1028aqds_tfa_defconfig              |  1 -
 configs/ls1028aqds_tfa_lpuart_defconfig       |  1 -
 configs/ls1028ardb_tfa_SECURE_BOOT_defconfig  |  1 -
 configs/ls1028ardb_tfa_defconfig              |  1 -
 configs/ls1043ardb_tfa_SECURE_BOOT_defconfig  |  1 -
 configs/ls1043ardb_tfa_defconfig              |  1 -
 configs/ls1046ardb_tfa_SECURE_BOOT_defconfig  |  1 -
 configs/ls1046ardb_tfa_defconfig              |  1 -
 configs/ls1088aqds_tfa_defconfig              |  1 -
 configs/ls1088ardb_tfa_SECURE_BOOT_defconfig  |  1 -
 configs/ls1088ardb_tfa_defconfig              |  1 -
 configs/ls2088aqds_tfa_defconfig              |  1 -
 configs/ls2088ardb_tfa_SECURE_BOOT_defconfig  |  1 -
 configs/ls2088ardb_tfa_defconfig              |  1 -
 configs/lx2160aqds_tfa_SECURE_BOOT_defconfig  |  1 -
 configs/lx2160aqds_tfa_defconfig              |  1 -
 configs/lx2160ardb_tfa_SECURE_BOOT_defconfig  |  1 -
 configs/lx2160ardb_tfa_defconfig              |  1 -
 configs/lx2160ardb_tfa_stmm_defconfig         |  1 -
 configs/lx2162aqds_tfa_SECURE_BOOT_defconfig  |  1 -
 configs/lx2162aqds_tfa_defconfig              |  1 -
 .../lx2162aqds_tfa_verified_boot_defconfig    |  1 -
 configs/ten64_tfa_defconfig                   |  1 -
 include/efi_loader.h                          |  4 -
 lib/efi_loader/Kconfig                        |  7 --
 lib/efi_loader/efi_disk.c                     | 78 +++++++------------
 lib/efi_loader/efi_memory.c                   | 16 ----
 29 files changed, 36 insertions(+), 101 deletions(-)

--
2.47.2

> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Date: Mon, 17 Mar 2025 15:38:36 +0200
> 
> The EFI subsystem defines its own bounce buffer for devices that
> can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> which can be reused instead of defining another symbol.
> The only limitation for EFI is that we don't know how big the file a user
> chooses to transfer is and as a result we can't depend on allocating the
> memory from the malloc area, which can prove too small.
> 
> So allocate an EFI buffer of the correct size and pass it to the DM,
> which already supports bounce buffering via bounce_buffer_start_extalign()

The existing bounce buffer code servers a completely different purpose
though.  It exists to make sure that hardware with cache-incoherent
DMA can safely do the required cache flushes.

This means that:

* SoCs with cache-coherent DMA don't necessarily set BOUNCE_BUFFER.
  Looks like you added that option to all the SoCs where you remove
  EFI_LOADER_BOUNCE_BUFFER.

* SoCs that (now) set BOUNCE_BUFFER may double bounce if the buffers
  aren't properly aligned.  I suppose that this won't happen since
  efi_disk_add_dev() sets medio.io_align to the device block size
  which is typically larger than the cache line size.

Still the commit message is somewhat misleading; this code doesn't
really make us use the DM bounce buffering code.

I also spotted a few bugs in the implementation.  See below.

> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  arch/arm/Kconfig                              |  8 ++
>  configs/ls1028aqds_tfa_SECURE_BOOT_defconfig  |  1 -
>  configs/ls1028aqds_tfa_defconfig              |  1 -
>  configs/ls1028aqds_tfa_lpuart_defconfig       |  1 -
>  configs/ls1028ardb_tfa_SECURE_BOOT_defconfig  |  1 -
>  configs/ls1028ardb_tfa_defconfig              |  1 -
>  configs/ls1043ardb_tfa_SECURE_BOOT_defconfig  |  1 -
>  configs/ls1043ardb_tfa_defconfig              |  1 -
>  configs/ls1046ardb_tfa_SECURE_BOOT_defconfig  |  1 -
>  configs/ls1046ardb_tfa_defconfig              |  1 -
>  configs/ls1088aqds_tfa_defconfig              |  1 -
>  configs/ls1088ardb_tfa_SECURE_BOOT_defconfig  |  1 -
>  configs/ls1088ardb_tfa_defconfig              |  1 -
>  configs/ls2088aqds_tfa_defconfig              |  1 -
>  configs/ls2088ardb_tfa_SECURE_BOOT_defconfig  |  1 -
>  configs/ls2088ardb_tfa_defconfig              |  1 -
>  configs/lx2160aqds_tfa_SECURE_BOOT_defconfig  |  1 -
>  configs/lx2160aqds_tfa_defconfig              |  1 -
>  configs/lx2160ardb_tfa_SECURE_BOOT_defconfig  |  1 -
>  configs/lx2160ardb_tfa_defconfig              |  1 -
>  configs/lx2160ardb_tfa_stmm_defconfig         |  1 -
>  configs/lx2162aqds_tfa_SECURE_BOOT_defconfig  |  1 -
>  configs/lx2162aqds_tfa_defconfig              |  1 -
>  .../lx2162aqds_tfa_verified_boot_defconfig    |  1 -
>  configs/ten64_tfa_defconfig                   |  1 -
>  include/efi_loader.h                          |  4 -
>  lib/efi_loader/Kconfig                        |  7 --
>  lib/efi_loader/efi_disk.c                     | 78 +++++++------------
>  lib/efi_loader/efi_memory.c                   | 16 ----
>  29 files changed, 36 insertions(+), 101 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index cf08fe63f1e7..bb946e69254c 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1417,6 +1417,7 @@ config TARGET_LS2080A_EMU
>  	select ARCH_LS2080A
>  	select ARM64
>  	select ARMV8_MULTIENTRY
> +	select BOUNCE_BUFFER
>  	select FSL_DDR_SYNC_REFRESH
>  	select GPIO_EXTRA_HEADER
>  	help
> @@ -1432,6 +1433,7 @@ config TARGET_LS1088AQDS
>  	select ARMV8_MULTIENTRY
>  	select ARCH_SUPPORT_TFABOOT
>  	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>  	select GPIO_EXTRA_HEADER
>  	select SUPPORT_SPL
>  	select FSL_DDR_INTERACTIVE if !SD_BOOT
> @@ -1448,6 +1450,7 @@ config TARGET_LS2080AQDS
>  	select ARMV8_MULTIENTRY
>  	select ARCH_SUPPORT_TFABOOT
>  	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>  	select GPIO_EXTRA_HEADER
>  	select SUPPORT_SPL
>  	imply SCSI
> @@ -1467,6 +1470,7 @@ config TARGET_LS2080ARDB
>  	select ARMV8_MULTIENTRY
>  	select ARCH_SUPPORT_TFABOOT
>  	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>  	select SUPPORT_SPL
>  	select FSL_DDR_BIST
>  	select FSL_DDR_INTERACTIVE if !SPL
> @@ -1485,6 +1489,7 @@ config TARGET_LS2081ARDB
>  	select ARM64
>  	select ARMV8_MULTIENTRY
>  	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>  	select GPIO_EXTRA_HEADER
>  	select SUPPORT_SPL
>  	help
> @@ -1500,6 +1505,7 @@ config TARGET_LX2160ARDB
>  	select ARMV8_MULTIENTRY
>  	select ARCH_SUPPORT_TFABOOT
>  	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>  	select GPIO_EXTRA_HEADER
>  	help
>  	  Support for NXP LX2160ARDB platform.
> @@ -1514,6 +1520,7 @@ config TARGET_LX2160AQDS
>  	select ARMV8_MULTIENTRY
>  	select ARCH_SUPPORT_TFABOOT
>  	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>  	select GPIO_EXTRA_HEADER
>  	help
>  	  Support for NXP LX2160AQDS platform.
> @@ -1529,6 +1536,7 @@ config TARGET_LX2162AQDS
>  	select ARMV8_MULTIENTRY
>  	select ARCH_SUPPORT_TFABOOT
>  	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>  	select GPIO_EXTRA_HEADER
>  	help
>  	  Support for NXP LX2162AQDS platform.
> diff --git a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
> index 97eb7d9dca4c..e3b1018f6f8e 100644
> --- a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
> @@ -23,7 +23,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>  CONFIG_SYS_MEMTEST_END=0x9fffffff
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1028aqds_tfa_defconfig b/configs/ls1028aqds_tfa_defconfig
> index cc53c17f543b..5b7dd4bf7dd5 100644
> --- a/configs/ls1028aqds_tfa_defconfig
> +++ b/configs/ls1028aqds_tfa_defconfig
> @@ -25,7 +25,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>  CONFIG_SYS_MEMTEST_END=0x9fffffff
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1028aqds_tfa_lpuart_defconfig b/configs/ls1028aqds_tfa_lpuart_defconfig
> index 8da7271c7f85..a38c81e83199 100644
> --- a/configs/ls1028aqds_tfa_lpuart_defconfig
> +++ b/configs/ls1028aqds_tfa_lpuart_defconfig
> @@ -24,7 +24,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>  CONFIG_SYS_MEMTEST_END=0x9fffffff
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
> index 3f71a37559be..da225e1dad8f 100644
> --- a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
> @@ -23,7 +23,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>  CONFIG_SYS_MEMTEST_END=0x9fffffff
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1028ardb_tfa_defconfig b/configs/ls1028ardb_tfa_defconfig
> index 89e452bdb1a8..829cc5204d34 100644
> --- a/configs/ls1028ardb_tfa_defconfig
> +++ b/configs/ls1028ardb_tfa_defconfig
> @@ -25,7 +25,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>  CONFIG_SYS_MEMTEST_END=0x9fffffff
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1043ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1043ardb_tfa_SECURE_BOOT_defconfig
> index 1b41fe608dae..04ca2a3afd81 100644
> --- a/configs/ls1043ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls1043ardb_tfa_SECURE_BOOT_defconfig
> @@ -24,7 +24,6 @@ CONFIG_PCIE2=y
>  CONFIG_PCIE3=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1043ardb_tfa_defconfig b/configs/ls1043ardb_tfa_defconfig
> index ca100bd4bca9..a6ca63cd8f79 100644
> --- a/configs/ls1043ardb_tfa_defconfig
> +++ b/configs/ls1043ardb_tfa_defconfig
> @@ -26,7 +26,6 @@ CONFIG_PCIE2=y
>  CONFIG_PCIE3=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1046ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1046ardb_tfa_SECURE_BOOT_defconfig
> index 6b4e834ad4a0..9c22d542a6f8 100644
> --- a/configs/ls1046ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls1046ardb_tfa_SECURE_BOOT_defconfig
> @@ -24,7 +24,6 @@ CONFIG_PCIE2=y
>  CONFIG_PCIE3=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1046ardb_tfa_defconfig b/configs/ls1046ardb_tfa_defconfig
> index a8f20f9b87b7..4eab8ea19e8e 100644
> --- a/configs/ls1046ardb_tfa_defconfig
> +++ b/configs/ls1046ardb_tfa_defconfig
> @@ -26,7 +26,6 @@ CONFIG_PCIE2=y
>  CONFIG_PCIE3=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1088aqds_tfa_defconfig b/configs/ls1088aqds_tfa_defconfig
> index df86bf982ee4..31d56f587af8 100644
> --- a/configs/ls1088aqds_tfa_defconfig
> +++ b/configs/ls1088aqds_tfa_defconfig
> @@ -29,7 +29,6 @@ CONFIG_SYS_MEMTEST_END=0x9fffffff
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
>  CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/ls1088ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1088ardb_tfa_SECURE_BOOT_defconfig
> index df472eeb404d..c910b73634f9 100644
> --- a/configs/ls1088ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls1088ardb_tfa_SECURE_BOOT_defconfig
> @@ -26,7 +26,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>  CONFIG_SYS_MEMTEST_END=0x9fffffff
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/ls1088ardb_tfa_defconfig b/configs/ls1088ardb_tfa_defconfig
> index 8d640f6594b2..682007e803d1 100644
> --- a/configs/ls1088ardb_tfa_defconfig
> +++ b/configs/ls1088ardb_tfa_defconfig
> @@ -28,7 +28,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>  CONFIG_SYS_MEMTEST_END=0x9fffffff
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/ls2088aqds_tfa_defconfig b/configs/ls2088aqds_tfa_defconfig
> index 8c86b8b11eb3..ea73a4cda971 100644
> --- a/configs/ls2088aqds_tfa_defconfig
> +++ b/configs/ls2088aqds_tfa_defconfig
> @@ -23,7 +23,6 @@ CONFIG_FSL_QIXIS=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
>  CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_BOOTDELAY=10
>  CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/ls2088ardb_tfa_SECURE_BOOT_defconfig b/configs/ls2088ardb_tfa_SECURE_BOOT_defconfig
> index 3fd1de967896..7f53397cdcb1 100644
> --- a/configs/ls2088ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls2088ardb_tfa_SECURE_BOOT_defconfig
> @@ -27,7 +27,6 @@ CONFIG_FSL_QIXIS=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
>  CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_BOOTDELAY=10
>  CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/ls2088ardb_tfa_defconfig b/configs/ls2088ardb_tfa_defconfig
> index f053044fa56e..398dfe068015 100644
> --- a/configs/ls2088ardb_tfa_defconfig
> +++ b/configs/ls2088ardb_tfa_defconfig
> @@ -29,7 +29,6 @@ CONFIG_FSL_QIXIS=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
>  CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_BOOTDELAY=10
>  CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig b/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig
> index 321ed71993b8..afb842978209 100644
> --- a/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig
> +++ b/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig
> @@ -28,7 +28,6 @@ CONFIG_FSL_QIXIS=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
>  CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_OF_BOARD_SETUP=y
>  CONFIG_OF_STDOUT_VIA_ALIAS=y
> diff --git a/configs/lx2160aqds_tfa_defconfig b/configs/lx2160aqds_tfa_defconfig
> index 465f07ee7f82..ec23ed384c44 100644
> --- a/configs/lx2160aqds_tfa_defconfig
> +++ b/configs/lx2160aqds_tfa_defconfig
> @@ -30,7 +30,6 @@ CONFIG_FSL_QIXIS=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
>  CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_BOOTDELAY=10
>  CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig b/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig
> index 478a01b566c6..71f4436eca06 100644
> --- a/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig
> @@ -29,7 +29,6 @@ CONFIG_FSL_QIXIS=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
>  CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_OF_BOARD_SETUP=y
>  CONFIG_OF_STDOUT_VIA_ALIAS=y
> diff --git a/configs/lx2160ardb_tfa_defconfig b/configs/lx2160ardb_tfa_defconfig
> index 8fd18825569c..6be3890de3ed 100644
> --- a/configs/lx2160ardb_tfa_defconfig
> +++ b/configs/lx2160ardb_tfa_defconfig
> @@ -31,7 +31,6 @@ CONFIG_FSL_QIXIS=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
>  CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_BOOTDELAY=10
>  CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/lx2160ardb_tfa_stmm_defconfig b/configs/lx2160ardb_tfa_stmm_defconfig
> index cf65897af898..ef89b0f34575 100644
> --- a/configs/lx2160ardb_tfa_stmm_defconfig
> +++ b/configs/lx2160ardb_tfa_stmm_defconfig
> @@ -32,7 +32,6 @@ CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
>  CONFIG_DYNAMIC_SYS_CLK_FREQ=y
>  CONFIG_EFI_MM_COMM_TEE=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_BOOTDELAY=10
>  CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/lx2162aqds_tfa_SECURE_BOOT_defconfig b/configs/lx2162aqds_tfa_SECURE_BOOT_defconfig
> index 46b6085a52fe..2b31be10899b 100644
> --- a/configs/lx2162aqds_tfa_SECURE_BOOT_defconfig
> +++ b/configs/lx2162aqds_tfa_SECURE_BOOT_defconfig
> @@ -28,7 +28,6 @@ CONFIG_FSL_QIXIS=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
>  CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_OF_BOARD_SETUP=y
>  CONFIG_OF_STDOUT_VIA_ALIAS=y
> diff --git a/configs/lx2162aqds_tfa_defconfig b/configs/lx2162aqds_tfa_defconfig
> index 41e0262f3ca5..43f8069cfde5 100644
> --- a/configs/lx2162aqds_tfa_defconfig
> +++ b/configs/lx2162aqds_tfa_defconfig
> @@ -30,7 +30,6 @@ CONFIG_FSL_QIXIS=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
>  CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_BOOTDELAY=10
>  CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/lx2162aqds_tfa_verified_boot_defconfig b/configs/lx2162aqds_tfa_verified_boot_defconfig
> index 7abfdbafbdf1..33b362c20b65 100644
> --- a/configs/lx2162aqds_tfa_verified_boot_defconfig
> +++ b/configs/lx2162aqds_tfa_verified_boot_defconfig
> @@ -30,7 +30,6 @@ CONFIG_FSL_QIXIS=y
>  CONFIG_REMAKE_ELF=y
>  CONFIG_MP=y
>  CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_SIGNATURE=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_BOOTDELAY=10
> diff --git a/configs/ten64_tfa_defconfig b/configs/ten64_tfa_defconfig
> index a65209e8335b..f35de31718f7 100644
> --- a/configs/ten64_tfa_defconfig
> +++ b/configs/ten64_tfa_defconfig
> @@ -16,7 +16,6 @@ CONFIG_PCI=y
>  CONFIG_AHCI=y
>  CONFIG_SYS_FSL_NUM_CC_PLLS=3
>  CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>  CONFIG_FIT_VERBOSE=y
>  CONFIG_BOOTSTD_FULL=y
>  CONFIG_BOOTSTD_BOOTCOMMAND=y
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index e9c10819ba26..3ed27e97a083 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -894,10 +894,6 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path,
>  				    struct efi_loaded_image_obj **handle_ptr,
>  				    struct efi_loaded_image **info_ptr);
> 
> -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> -extern void *efi_bounce_buffer;
> -#define EFI_LOADER_BOUNCE_BUFFER_SIZE (64 * 1024 * 1024)
> -#endif
> 
>  /* shorten device path */
>  struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp);
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 6130af14337e..bc08776ef4a6 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -490,13 +490,6 @@ config EFI_HTTP_PROTOCOL
>  endmenu
> 
>  menu "Misc options"
> -config EFI_LOADER_BOUNCE_BUFFER
> -	bool "EFI Applications use bounce buffers for DMA operations"
> -	help
> -	  Some hardware does not support DMA to full 64bit addresses. For this
> -	  hardware we can create a bounce buffer so that payloads don't have to
> -	  worry about platform details.
> -
>  config EFI_GRUB_ARM32_WORKAROUND
>  	bool "Workaround for GRUB on 32bit ARM"
>  	default n if ARCH_BCM283X || ARCH_SUNXI || ARCH_QEMU
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 5452640354e0..a0202032bb5a 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -105,6 +105,8 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
>  	int blksz;
>  	int blocks;
>  	unsigned long n;
> +	u64 bb = 0xffffffff;
> +	void *bb_ptr = buffer;
> 
>  	diskobj = container_of(this, struct efi_disk_obj, ops);
>  	blksz = diskobj->media.block_size;
> @@ -113,27 +115,35 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
>  	EFI_PRINT("blocks=%x lba=%llx blksz=%x dir=%d\n",
>  		  blocks, lba, blksz, direction);
> 
> +	if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && (uintptr_t)buffer >= SZ_4G + buffer_size - 1) {

Shouldn't that check be

	if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && (uintptr_t)buffer + buffer_size - 1 >= SZ_4G) {

?

> +		if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_BOOT_SERVICES_DATA,
> +				       buffer_size >> EFI_PAGE_SHIFT, &bb) != EFI_SUCCESS)
> +			return EFI_OUT_OF_RESOURCES;
> +
> +		bb_ptr = (void *)(uintptr_t)bb;
> +	}
>  	/* We only support full block access */
> -	if (buffer_size & (blksz - 1))
> +	if (buffer_size & (blksz - 1)) {
> +		if (buffer != bb_ptr)
> +			efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
>  		return EFI_BAD_BUFFER_SIZE;
> +	}

Any reason why you don't check the buffer_size check before allocating
the bounce buffer?  That way you don't have to worry about freeing it
here.

You're missing a memcpy() here in the case direction == EFI_DISK_WRITE.

>  	if (CONFIG_IS_ENABLED(PARTITIONS) &&
>  	    device_get_uclass_id(diskobj->header.dev) == UCLASS_PARTITION) {
>  		if (direction == EFI_DISK_READ)
> -			n = disk_blk_read(diskobj->header.dev, lba, blocks,
> -					  buffer);
> +			n = disk_blk_read(diskobj->header.dev, lba, blocks, bb_ptr);
>  		else
> -			n = disk_blk_write(diskobj->header.dev, lba, blocks,
> -					   buffer);
> +			n = disk_blk_write(diskobj->header.dev, lba, blocks, bb_ptr);
>  	} else {
>  		/* dev is a block device (UCLASS_BLK) */
>  		struct blk_desc *desc;
> 
>  		desc = dev_get_uclass_plat(diskobj->header.dev);
>  		if (direction == EFI_DISK_READ)
> -			n = blk_dread(desc, lba, blocks, buffer);
> +			n = blk_dread(desc, lba, blocks, bb_ptr);
>  		else
> -			n = blk_dwrite(desc, lba, blocks, buffer);
> +			n = blk_dwrite(desc, lba, blocks, bb_ptr);
>  	}
> 
>  	/* We don't do interrupts, so check for timers cooperatively */
> @@ -141,8 +151,16 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
> 
>  	EFI_PRINT("n=%lx blocks=%x\n", n, blocks);
> 
> -	if (n != blocks)
> +	if (n != blocks) {
> +		if (buffer != bb_ptr)
> +			efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
>  		return EFI_DEVICE_ERROR;
> +	}
> +
> +	if (buffer != bb_ptr) {
> +		memcpy(buffer, bb_ptr, buffer_size);

This memcpy() is only necessary if direction == EFI_DISK_READ.

> +		efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
> +	}
> 
>  	return EFI_SUCCESS;
>  }
> @@ -166,7 +184,6 @@ static efi_status_t EFIAPI efi_disk_read_blocks(struct efi_block_io *this,
>  			u32 media_id, u64 lba, efi_uintn_t buffer_size,
>  			void *buffer)
>  {
> -	void *real_buffer = buffer;
>  	efi_status_t r;
> 
>  	if (!this)
> @@ -184,31 +201,12 @@ static efi_status_t EFIAPI efi_disk_read_blocks(struct efi_block_io *this,
>  	    (this->media->last_block + 1) * this->media->block_size)
>  		return EFI_INVALID_PARAMETER;
> 
> -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> -	if (buffer_size > EFI_LOADER_BOUNCE_BUFFER_SIZE) {
> -		r = efi_disk_read_blocks(this, media_id, lba,
> -			EFI_LOADER_BOUNCE_BUFFER_SIZE, buffer);
> -		if (r != EFI_SUCCESS)
> -			return r;
> -		return efi_disk_read_blocks(this, media_id, lba +
> -			EFI_LOADER_BOUNCE_BUFFER_SIZE / this->media->block_size,
> -			buffer_size - EFI_LOADER_BOUNCE_BUFFER_SIZE,
> -			buffer + EFI_LOADER_BOUNCE_BUFFER_SIZE);
> -	}
> -
> -	real_buffer = efi_bounce_buffer;
> -#endif
> -
>  	EFI_ENTRY("%p, %x, %llx, %zx, %p", this, media_id, lba,
>  		  buffer_size, buffer);
> 
> -	r = efi_disk_rw_blocks(this, media_id, lba, buffer_size, real_buffer,
> +	r = efi_disk_rw_blocks(this, media_id, lba, buffer_size, buffer,
>  			       EFI_DISK_READ);
> 
> -	/* Copy from bounce buffer to real buffer if necessary */
> -	if ((r == EFI_SUCCESS) && (real_buffer != buffer))
> -		memcpy(buffer, real_buffer, buffer_size);
> -
>  	return EFI_EXIT(r);
>  }
> 
> @@ -232,7 +230,6 @@ static efi_status_t EFIAPI efi_disk_write_blocks(struct efi_block_io *this,
>  			u32 media_id, u64 lba, efi_uintn_t buffer_size,
>  			void *buffer)
>  {
> -	void *real_buffer = buffer;
>  	efi_status_t r;
> 
>  	if (!this)
> @@ -252,29 +249,10 @@ static efi_status_t EFIAPI efi_disk_write_blocks(struct efi_block_io *this,
>  	    (this->media->last_block + 1) * this->media->block_size)
>  		return EFI_INVALID_PARAMETER;
> 
> -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> -	if (buffer_size > EFI_LOADER_BOUNCE_BUFFER_SIZE) {
> -		r = efi_disk_write_blocks(this, media_id, lba,
> -			EFI_LOADER_BOUNCE_BUFFER_SIZE, buffer);
> -		if (r != EFI_SUCCESS)
> -			return r;
> -		return efi_disk_write_blocks(this, media_id, lba +
> -			EFI_LOADER_BOUNCE_BUFFER_SIZE / this->media->block_size,
> -			buffer_size - EFI_LOADER_BOUNCE_BUFFER_SIZE,
> -			buffer + EFI_LOADER_BOUNCE_BUFFER_SIZE);
> -	}
> -
> -	real_buffer = efi_bounce_buffer;
> -#endif
> -
>  	EFI_ENTRY("%p, %x, %llx, %zx, %p", this, media_id, lba,
>  		  buffer_size, buffer);
> 
> -	/* Populate bounce buffer if necessary */
> -	if (real_buffer != buffer)
> -		memcpy(real_buffer, buffer, buffer_size);
> -
> -	r = efi_disk_rw_blocks(this, media_id, lba, buffer_size, real_buffer,
> +	r = efi_disk_rw_blocks(this, media_id, lba, buffer_size, buffer,
>  			       EFI_DISK_WRITE);
> 
>  	return EFI_EXIT(r);
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index 6d00b1862505..d7a8cb89e5b5 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -40,10 +40,6 @@ struct efi_mem_list {
>  /* This list contains all memory map items */
>  static LIST_HEAD(efi_mem);
> 
> -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> -void *efi_bounce_buffer;
> -#endif
> -
>  /**
>   * struct efi_pool_allocation - memory block allocated from pool
>   *
> @@ -852,18 +848,6 @@ int efi_memory_init(void)
> 
>  	add_u_boot_and_runtime();
> 
> -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> -	/* Request a 32bit 64MB bounce buffer region */
> -	uint64_t efi_bounce_buffer_addr = 0xffffffff;
> -
> -	if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_BOOT_SERVICES_DATA,
> -			       (64 * 1024 * 1024) >> EFI_PAGE_SHIFT,
> -			       &efi_bounce_buffer_addr) != EFI_SUCCESS)
> -		return -1;
> -
> -	efi_bounce_buffer = (void*)(uintptr_t)efi_bounce_buffer_addr;
> -#endif
> -
>  	return 0;
>  }
> 
> --
> 2.47.2
> 
>

Hi Mark,
Thanks for taking a look

On Mon, 17 Mar 2025 at 18:18, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Date: Mon, 17 Mar 2025 15:38:36 +0200
> >
> > The EFI subsystem defines its own bounce buffer for devices that
> > can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> > which can be reused instead of defining another symbol.
> > The only limitation for EFI is that we don't know how big the file a user
> > chooses to transfer is and as a result we can't depend on allocating the
> > memory from the malloc area, which can prove too small.
> >
> > So allocate an EFI buffer of the correct size and pass it to the DM,
> > which already supports bounce buffering via bounce_buffer_start_extalign()
>
> The existing bounce buffer code servers a completely different purpose
> though.  It exists to make sure that hardware with cache-incoherent
> DMA can safely do the required cache flushes.
>
> This means that:
>
> * SoCs with cache-coherent DMA don't necessarily set BOUNCE_BUFFER.
>   Looks like you added that option to all the SoCs where you remove
>   EFI_LOADER_BOUNCE_BUFFER.

Yes, and that has a side effect I should have probably added to the
commit message. Using the existing bounce buffer will flush caches
even if it's pointless.

>
> * SoCs that (now) set BOUNCE_BUFFER may double bounce if the buffers
>   aren't properly aligned.  I suppose that this won't happen since
>   efi_disk_add_dev() sets medio.io_align to the device block size
>   which is typically larger than the cache line size.

I think it won't happen indeed but for a different reason. The EFI
memory we allocate and pass to bounce_buffer_start_extalign() is
page-aligned.
The DM subsystem will call that function with either
blk_buffer_aligned() which always returns 1 or whatever the device has
defined. The strictest one I found was the virtio one which requires
page alignment.

Allocating memory from EFI is needed, simply because the current
bounce buffer API will use the malloc area, which might not be enough.
Do you think the extra cache flush is a no-go and we should leave the
code as-is?

>
> Still the commit message is somewhat misleading; this code doesn't
> really make us use the DM bounce buffering code.
>
> I also spotted a few bugs in the implementation.  See below.

[...]

> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -105,6 +105,8 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
> >       int blksz;
> >       int blocks;
> >       unsigned long n;
> > +     u64 bb = 0xffffffff;
> > +     void *bb_ptr = buffer;
> >
> >       diskobj = container_of(this, struct efi_disk_obj, ops);
> >       blksz = diskobj->media.block_size;
> > @@ -113,27 +115,35 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
> >       EFI_PRINT("blocks=%x lba=%llx blksz=%x dir=%d\n",
> >                 blocks, lba, blksz, direction);
> >
> > +     if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && (uintptr_t)buffer >= SZ_4G + buffer_size - 1) {
>
> Shouldn't that check be
>
>         if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && (uintptr_t)buffer + buffer_size - 1 >= SZ_4G) {
>
> ?

Yes...
I originally had (uintptr_t)buffer > SZ_4G - buffer_size - 1 and avoid
potential overflows, but then I started to think what happens if
buffer_size is 4GB and completely messed this up ...
I think it's (uintptr_t)buffer + buffer_size + 1 >= SZ_4G though,
because SZ_4G is 0x100000000. Anyway yes, you are right, I'll fix it
in v2, but using subtractions.

>
> > +             if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_BOOT_SERVICES_DATA,
> > +                                    buffer_size >> EFI_PAGE_SHIFT, &bb) != EFI_SUCCESS)
> > +                     return EFI_OUT_OF_RESOURCES;
> > +
> > +             bb_ptr = (void *)(uintptr_t)bb;
> > +     }
> >       /* We only support full block access */
> > -     if (buffer_size & (blksz - 1))
> > +     if (buffer_size & (blksz - 1)) {
> > +             if (buffer != bb_ptr)
> > +                     efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
> >               return EFI_BAD_BUFFER_SIZE;
> > +     }
>
> Any reason why you don't check the buffer_size check before allocating
> the bounce buffer?  That way you don't have to worry about freeing it
> here.

Nop, that code was already there and I  didn't move it. I'll move it around.

>
> You're missing a memcpy() here in the case direction == EFI_DISK_WRITE.

ah thanks

>
> >       if (CONFIG_IS_ENABLED(PARTITIONS) &&
> >           device_get_uclass_id(diskobj->header.dev) == UCLASS_PARTITION) {
> >               if (direction == EFI_DISK_READ)

[...]

> > +                     efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
> >               return EFI_DEVICE_ERROR;
> > +     }
> > +
> > +     if (buffer != bb_ptr) {
> > +             memcpy(buffer, bb_ptr, buffer_size);
>
> This memcpy() is only necessary if direction == EFI_DISK_READ.

Ok

[...]

Thanks
/Ilias

> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Date: Mon, 17 Mar 2025 21:06:26 +0200

Hi Ilias,

> Hi Mark,
> Thanks for taking a look
> 
> On Mon, 17 Mar 2025 at 18:18, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >
> > > From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > Date: Mon, 17 Mar 2025 15:38:36 +0200
> > >
> > > The EFI subsystem defines its own bounce buffer for devices that
> > > can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> > > which can be reused instead of defining another symbol.
> > > The only limitation for EFI is that we don't know how big the file a user
> > > chooses to transfer is and as a result we can't depend on allocating the
> > > memory from the malloc area, which can prove too small.
> > >
> > > So allocate an EFI buffer of the correct size and pass it to the DM,
> > > which already supports bounce buffering via bounce_buffer_start_extalign()
> >
> > The existing bounce buffer code servers a completely different purpose
> > though.  It exists to make sure that hardware with cache-incoherent
> > DMA can safely do the required cache flushes.
> >
> > This means that:
> >
> > * SoCs with cache-coherent DMA don't necessarily set BOUNCE_BUFFER.
> >   Looks like you added that option to all the SoCs where you remove
> >   EFI_LOADER_BOUNCE_BUFFER.
> 
> Yes, and that has a side effect I should have probably added to the
> commit message. Using the existing bounce buffer will flush caches
> even if it's pointless.
> 
> >
> > * SoCs that (now) set BOUNCE_BUFFER may double bounce if the buffers
> >   aren't properly aligned.  I suppose that this won't happen since
> >   efi_disk_add_dev() sets medio.io_align to the device block size
> >   which is typically larger than the cache line size.
> 
> I think it won't happen indeed but for a different reason. The EFI
> memory we allocate and pass to bounce_buffer_start_extalign() is
> page-aligned.
> The DM subsystem will call that function with either
> blk_buffer_aligned() which always returns 1 or whatever the device has
> defined. The strictest one I found was the virtio one which requires
> page alignment.
> 
> Allocating memory from EFI is needed, simply because the current
> bounce buffer API will use the malloc area, which might not be enough.
> Do you think the extra cache flush is a no-go and we should leave the
> code as-is?

I think it is undesirable.  I believe it will measurably slow down
loading large files.

So probably best to leave the code as-is.  If somebody really cares,
they should probably separate out the cache flushing code from
CONFIG_BOUNCE_BUFFER first.  And maybe fix the DM bounce buffer code
such that it supports large buffers.

> > Still the commit message is somewhat misleading; this code doesn't
> > really make us use the DM bounce buffering code.
> >
> > I also spotted a few bugs in the implementation.  See below.
> 
> [...]
> 
> > > +++ b/lib/efi_loader/efi_disk.c
> > > @@ -105,6 +105,8 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
> > >       int blksz;
> > >       int blocks;
> > >       unsigned long n;
> > > +     u64 bb = 0xffffffff;
> > > +     void *bb_ptr = buffer;
> > >
> > >       diskobj = container_of(this, struct efi_disk_obj, ops);
> > >       blksz = diskobj->media.block_size;
> > > @@ -113,27 +115,35 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
> > >       EFI_PRINT("blocks=%x lba=%llx blksz=%x dir=%d\n",
> > >                 blocks, lba, blksz, direction);
> > >
> > > +     if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && (uintptr_t)buffer >= SZ_4G + buffer_size - 1) {
> >
> > Shouldn't that check be
> >
> >         if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && (uintptr_t)buffer + buffer_size - 1 >= SZ_4G) {
> >
> > ?
> 
> Yes...
> I originally had (uintptr_t)buffer > SZ_4G - buffer_size - 1 and avoid
> potential overflows, but then I started to think what happens if
> buffer_size is 4GB and completely messed this up ...
> I think it's (uintptr_t)buffer + buffer_size + 1 >= SZ_4G though,
> because SZ_4G is 0x100000000. Anyway yes, you are right, I'll fix it
> in v2, but using subtractions.
> 
> >
> > > +             if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_BOOT_SERVICES_DATA,
> > > +                                    buffer_size >> EFI_PAGE_SHIFT, &bb) != EFI_SUCCESS)
> > > +                     return EFI_OUT_OF_RESOURCES;
> > > +
> > > +             bb_ptr = (void *)(uintptr_t)bb;
> > > +     }
> > >       /* We only support full block access */
> > > -     if (buffer_size & (blksz - 1))
> > > +     if (buffer_size & (blksz - 1)) {
> > > +             if (buffer != bb_ptr)
> > > +                     efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
> > >               return EFI_BAD_BUFFER_SIZE;
> > > +     }
> >
> > Any reason why you don't check the buffer_size check before allocating
> > the bounce buffer?  That way you don't have to worry about freeing it
> > here.
> 
> Nop, that code was already there and I  didn't move it. I'll move it around.
> 
> >
> > You're missing a memcpy() here in the case direction == EFI_DISK_WRITE.
> 
> ah thanks
> 
> >
> > >       if (CONFIG_IS_ENABLED(PARTITIONS) &&
> > >           device_get_uclass_id(diskobj->header.dev) == UCLASS_PARTITION) {
> > >               if (direction == EFI_DISK_READ)
> 
> [...]
> 
> > > +                     efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
> > >               return EFI_DEVICE_ERROR;
> > > +     }
> > > +
> > > +     if (buffer != bb_ptr) {
> > > +             memcpy(buffer, bb_ptr, buffer_size);
> >
> > This memcpy() is only necessary if direction == EFI_DISK_READ.
> 
> Ok
> 
> [...]
> 
> Thanks
> /Ilias
>

On 17.03.25 14:38, Ilias Apalodimas wrote:

%s/EFI_BOUNCE_BUFFER/CONFIG_EFI_LOADER_BOUNCE_BUFFER/

> The EFI subsystem defines its own bounce buffer for devices that
> can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> which can be reused instead of defining another symbol.
> The only limitation for EFI is that we don't know how big the file a user
> chooses to transfer is and as a result we can't depend on allocating the
> memory from the malloc area, which can prove too small.
>
> So allocate an EFI buffer of the correct size and pass it to the DM,
> which already supports bounce buffering via bounce_buffer_start_extalign()

Looking at

     if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && desc->bb) {

in drivers/block/blk-uclass.c the bounce buffer has to be explicitly
enabled by the device driver. Only the scsi drivers sets bb = true.

Cf. 81bd22e935dc ("rockchip: block: blk-uclass: add bounce buffer flag
to blk_desc")

Which device-drivers of the boards mentioned below do actually need
bounce buffering?

Best regards

Heinrich

>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   arch/arm/Kconfig                              |  8 ++
>   configs/ls1028aqds_tfa_SECURE_BOOT_defconfig  |  1 -
>   configs/ls1028aqds_tfa_defconfig              |  1 -
>   configs/ls1028aqds_tfa_lpuart_defconfig       |  1 -
>   configs/ls1028ardb_tfa_SECURE_BOOT_defconfig  |  1 -
>   configs/ls1028ardb_tfa_defconfig              |  1 -
>   configs/ls1043ardb_tfa_SECURE_BOOT_defconfig  |  1 -
>   configs/ls1043ardb_tfa_defconfig              |  1 -
>   configs/ls1046ardb_tfa_SECURE_BOOT_defconfig  |  1 -
>   configs/ls1046ardb_tfa_defconfig              |  1 -
>   configs/ls1088aqds_tfa_defconfig              |  1 -
>   configs/ls1088ardb_tfa_SECURE_BOOT_defconfig  |  1 -
>   configs/ls1088ardb_tfa_defconfig              |  1 -
>   configs/ls2088aqds_tfa_defconfig              |  1 -
>   configs/ls2088ardb_tfa_SECURE_BOOT_defconfig  |  1 -
>   configs/ls2088ardb_tfa_defconfig              |  1 -
>   configs/lx2160aqds_tfa_SECURE_BOOT_defconfig  |  1 -
>   configs/lx2160aqds_tfa_defconfig              |  1 -
>   configs/lx2160ardb_tfa_SECURE_BOOT_defconfig  |  1 -
>   configs/lx2160ardb_tfa_defconfig              |  1 -
>   configs/lx2160ardb_tfa_stmm_defconfig         |  1 -
>   configs/lx2162aqds_tfa_SECURE_BOOT_defconfig  |  1 -
>   configs/lx2162aqds_tfa_defconfig              |  1 -
>   .../lx2162aqds_tfa_verified_boot_defconfig    |  1 -
>   configs/ten64_tfa_defconfig                   |  1 -
>   include/efi_loader.h                          |  4 -
>   lib/efi_loader/Kconfig                        |  7 --
>   lib/efi_loader/efi_disk.c                     | 78 +++++++------------
>   lib/efi_loader/efi_memory.c                   | 16 ----
>   29 files changed, 36 insertions(+), 101 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index cf08fe63f1e7..bb946e69254c 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1417,6 +1417,7 @@ config TARGET_LS2080A_EMU
>   	select ARCH_LS2080A
>   	select ARM64
>   	select ARMV8_MULTIENTRY
> +	select BOUNCE_BUFFER
>   	select FSL_DDR_SYNC_REFRESH
>   	select GPIO_EXTRA_HEADER
>   	help
> @@ -1432,6 +1433,7 @@ config TARGET_LS1088AQDS
>   	select ARMV8_MULTIENTRY
>   	select ARCH_SUPPORT_TFABOOT
>   	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>   	select GPIO_EXTRA_HEADER
>   	select SUPPORT_SPL
>   	select FSL_DDR_INTERACTIVE if !SD_BOOT
> @@ -1448,6 +1450,7 @@ config TARGET_LS2080AQDS
>   	select ARMV8_MULTIENTRY
>   	select ARCH_SUPPORT_TFABOOT
>   	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>   	select GPIO_EXTRA_HEADER
>   	select SUPPORT_SPL
>   	imply SCSI
> @@ -1467,6 +1470,7 @@ config TARGET_LS2080ARDB
>   	select ARMV8_MULTIENTRY
>   	select ARCH_SUPPORT_TFABOOT
>   	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>   	select SUPPORT_SPL
>   	select FSL_DDR_BIST
>   	select FSL_DDR_INTERACTIVE if !SPL
> @@ -1485,6 +1489,7 @@ config TARGET_LS2081ARDB
>   	select ARM64
>   	select ARMV8_MULTIENTRY
>   	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>   	select GPIO_EXTRA_HEADER
>   	select SUPPORT_SPL
>   	help
> @@ -1500,6 +1505,7 @@ config TARGET_LX2160ARDB
>   	select ARMV8_MULTIENTRY
>   	select ARCH_SUPPORT_TFABOOT
>   	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>   	select GPIO_EXTRA_HEADER
>   	help
>   	  Support for NXP LX2160ARDB platform.
> @@ -1514,6 +1520,7 @@ config TARGET_LX2160AQDS
>   	select ARMV8_MULTIENTRY
>   	select ARCH_SUPPORT_TFABOOT
>   	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>   	select GPIO_EXTRA_HEADER
>   	help
>   	  Support for NXP LX2160AQDS platform.
> @@ -1529,6 +1536,7 @@ config TARGET_LX2162AQDS
>   	select ARMV8_MULTIENTRY
>   	select ARCH_SUPPORT_TFABOOT
>   	select BOARD_LATE_INIT
> +	select BOUNCE_BUFFER
>   	select GPIO_EXTRA_HEADER
>   	help
>   	  Support for NXP LX2162AQDS platform.
> diff --git a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
> index 97eb7d9dca4c..e3b1018f6f8e 100644
> --- a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
> @@ -23,7 +23,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>   CONFIG_SYS_MEMTEST_END=0x9fffffff
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_DISTRO_DEFAULTS=y
>   CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1028aqds_tfa_defconfig b/configs/ls1028aqds_tfa_defconfig
> index cc53c17f543b..5b7dd4bf7dd5 100644
> --- a/configs/ls1028aqds_tfa_defconfig
> +++ b/configs/ls1028aqds_tfa_defconfig
> @@ -25,7 +25,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>   CONFIG_SYS_MEMTEST_END=0x9fffffff
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_DISTRO_DEFAULTS=y
>   CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1028aqds_tfa_lpuart_defconfig b/configs/ls1028aqds_tfa_lpuart_defconfig
> index 8da7271c7f85..a38c81e83199 100644
> --- a/configs/ls1028aqds_tfa_lpuart_defconfig
> +++ b/configs/ls1028aqds_tfa_lpuart_defconfig
> @@ -24,7 +24,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>   CONFIG_SYS_MEMTEST_END=0x9fffffff
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_DISTRO_DEFAULTS=y
>   CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
> index 3f71a37559be..da225e1dad8f 100644
> --- a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
> @@ -23,7 +23,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>   CONFIG_SYS_MEMTEST_END=0x9fffffff
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_DISTRO_DEFAULTS=y
>   CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1028ardb_tfa_defconfig b/configs/ls1028ardb_tfa_defconfig
> index 89e452bdb1a8..829cc5204d34 100644
> --- a/configs/ls1028ardb_tfa_defconfig
> +++ b/configs/ls1028ardb_tfa_defconfig
> @@ -25,7 +25,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>   CONFIG_SYS_MEMTEST_END=0x9fffffff
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_DISTRO_DEFAULTS=y
>   CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1043ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1043ardb_tfa_SECURE_BOOT_defconfig
> index 1b41fe608dae..04ca2a3afd81 100644
> --- a/configs/ls1043ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls1043ardb_tfa_SECURE_BOOT_defconfig
> @@ -24,7 +24,6 @@ CONFIG_PCIE2=y
>   CONFIG_PCIE3=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_DISTRO_DEFAULTS=y
>   CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1043ardb_tfa_defconfig b/configs/ls1043ardb_tfa_defconfig
> index ca100bd4bca9..a6ca63cd8f79 100644
> --- a/configs/ls1043ardb_tfa_defconfig
> +++ b/configs/ls1043ardb_tfa_defconfig
> @@ -26,7 +26,6 @@ CONFIG_PCIE2=y
>   CONFIG_PCIE3=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_DISTRO_DEFAULTS=y
>   CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1046ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1046ardb_tfa_SECURE_BOOT_defconfig
> index 6b4e834ad4a0..9c22d542a6f8 100644
> --- a/configs/ls1046ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls1046ardb_tfa_SECURE_BOOT_defconfig
> @@ -24,7 +24,6 @@ CONFIG_PCIE2=y
>   CONFIG_PCIE3=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_DISTRO_DEFAULTS=y
>   CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1046ardb_tfa_defconfig b/configs/ls1046ardb_tfa_defconfig
> index a8f20f9b87b7..4eab8ea19e8e 100644
> --- a/configs/ls1046ardb_tfa_defconfig
> +++ b/configs/ls1046ardb_tfa_defconfig
> @@ -26,7 +26,6 @@ CONFIG_PCIE2=y
>   CONFIG_PCIE3=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_DISTRO_DEFAULTS=y
>   CONFIG_BOOTDELAY=10
> diff --git a/configs/ls1088aqds_tfa_defconfig b/configs/ls1088aqds_tfa_defconfig
> index df86bf982ee4..31d56f587af8 100644
> --- a/configs/ls1088aqds_tfa_defconfig
> +++ b/configs/ls1088aqds_tfa_defconfig
> @@ -29,7 +29,6 @@ CONFIG_SYS_MEMTEST_END=0x9fffffff
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
>   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_DISTRO_DEFAULTS=y
>   CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/ls1088ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1088ardb_tfa_SECURE_BOOT_defconfig
> index df472eeb404d..c910b73634f9 100644
> --- a/configs/ls1088ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls1088ardb_tfa_SECURE_BOOT_defconfig
> @@ -26,7 +26,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>   CONFIG_SYS_MEMTEST_END=0x9fffffff
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_DISTRO_DEFAULTS=y
>   CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/ls1088ardb_tfa_defconfig b/configs/ls1088ardb_tfa_defconfig
> index 8d640f6594b2..682007e803d1 100644
> --- a/configs/ls1088ardb_tfa_defconfig
> +++ b/configs/ls1088ardb_tfa_defconfig
> @@ -28,7 +28,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
>   CONFIG_SYS_MEMTEST_END=0x9fffffff
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_DISTRO_DEFAULTS=y
>   CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/ls2088aqds_tfa_defconfig b/configs/ls2088aqds_tfa_defconfig
> index 8c86b8b11eb3..ea73a4cda971 100644
> --- a/configs/ls2088aqds_tfa_defconfig
> +++ b/configs/ls2088aqds_tfa_defconfig
> @@ -23,7 +23,6 @@ CONFIG_FSL_QIXIS=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
>   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_BOOTDELAY=10
>   CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/ls2088ardb_tfa_SECURE_BOOT_defconfig b/configs/ls2088ardb_tfa_SECURE_BOOT_defconfig
> index 3fd1de967896..7f53397cdcb1 100644
> --- a/configs/ls2088ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/ls2088ardb_tfa_SECURE_BOOT_defconfig
> @@ -27,7 +27,6 @@ CONFIG_FSL_QIXIS=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
>   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_BOOTDELAY=10
>   CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/ls2088ardb_tfa_defconfig b/configs/ls2088ardb_tfa_defconfig
> index f053044fa56e..398dfe068015 100644
> --- a/configs/ls2088ardb_tfa_defconfig
> +++ b/configs/ls2088ardb_tfa_defconfig
> @@ -29,7 +29,6 @@ CONFIG_FSL_QIXIS=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
>   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_BOOTDELAY=10
>   CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig b/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig
> index 321ed71993b8..afb842978209 100644
> --- a/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig
> +++ b/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig
> @@ -28,7 +28,6 @@ CONFIG_FSL_QIXIS=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
>   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_OF_BOARD_SETUP=y
>   CONFIG_OF_STDOUT_VIA_ALIAS=y
> diff --git a/configs/lx2160aqds_tfa_defconfig b/configs/lx2160aqds_tfa_defconfig
> index 465f07ee7f82..ec23ed384c44 100644
> --- a/configs/lx2160aqds_tfa_defconfig
> +++ b/configs/lx2160aqds_tfa_defconfig
> @@ -30,7 +30,6 @@ CONFIG_FSL_QIXIS=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
>   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_BOOTDELAY=10
>   CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig b/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig
> index 478a01b566c6..71f4436eca06 100644
> --- a/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig
> +++ b/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig
> @@ -29,7 +29,6 @@ CONFIG_FSL_QIXIS=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
>   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_OF_BOARD_SETUP=y
>   CONFIG_OF_STDOUT_VIA_ALIAS=y
> diff --git a/configs/lx2160ardb_tfa_defconfig b/configs/lx2160ardb_tfa_defconfig
> index 8fd18825569c..6be3890de3ed 100644
> --- a/configs/lx2160ardb_tfa_defconfig
> +++ b/configs/lx2160ardb_tfa_defconfig
> @@ -31,7 +31,6 @@ CONFIG_FSL_QIXIS=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
>   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_BOOTDELAY=10
>   CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/lx2160ardb_tfa_stmm_defconfig b/configs/lx2160ardb_tfa_stmm_defconfig
> index cf65897af898..ef89b0f34575 100644
> --- a/configs/lx2160ardb_tfa_stmm_defconfig
> +++ b/configs/lx2160ardb_tfa_stmm_defconfig
> @@ -32,7 +32,6 @@ CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
>   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
>   CONFIG_EFI_MM_COMM_TEE=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_BOOTDELAY=10
>   CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/lx2162aqds_tfa_SECURE_BOOT_defconfig b/configs/lx2162aqds_tfa_SECURE_BOOT_defconfig
> index 46b6085a52fe..2b31be10899b 100644
> --- a/configs/lx2162aqds_tfa_SECURE_BOOT_defconfig
> +++ b/configs/lx2162aqds_tfa_SECURE_BOOT_defconfig
> @@ -28,7 +28,6 @@ CONFIG_FSL_QIXIS=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
>   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_OF_BOARD_SETUP=y
>   CONFIG_OF_STDOUT_VIA_ALIAS=y
> diff --git a/configs/lx2162aqds_tfa_defconfig b/configs/lx2162aqds_tfa_defconfig
> index 41e0262f3ca5..43f8069cfde5 100644
> --- a/configs/lx2162aqds_tfa_defconfig
> +++ b/configs/lx2162aqds_tfa_defconfig
> @@ -30,7 +30,6 @@ CONFIG_FSL_QIXIS=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
>   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_BOOTDELAY=10
>   CONFIG_OF_BOARD_SETUP=y
> diff --git a/configs/lx2162aqds_tfa_verified_boot_defconfig b/configs/lx2162aqds_tfa_verified_boot_defconfig
> index 7abfdbafbdf1..33b362c20b65 100644
> --- a/configs/lx2162aqds_tfa_verified_boot_defconfig
> +++ b/configs/lx2162aqds_tfa_verified_boot_defconfig
> @@ -30,7 +30,6 @@ CONFIG_FSL_QIXIS=y
>   CONFIG_REMAKE_ELF=y
>   CONFIG_MP=y
>   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_SIGNATURE=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_BOOTDELAY=10
> diff --git a/configs/ten64_tfa_defconfig b/configs/ten64_tfa_defconfig
> index a65209e8335b..f35de31718f7 100644
> --- a/configs/ten64_tfa_defconfig
> +++ b/configs/ten64_tfa_defconfig
> @@ -16,7 +16,6 @@ CONFIG_PCI=y
>   CONFIG_AHCI=y
>   CONFIG_SYS_FSL_NUM_CC_PLLS=3
>   CONFIG_MP=y
> -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
>   CONFIG_FIT_VERBOSE=y
>   CONFIG_BOOTSTD_FULL=y
>   CONFIG_BOOTSTD_BOOTCOMMAND=y
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index e9c10819ba26..3ed27e97a083 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -894,10 +894,6 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path,
>   				    struct efi_loaded_image_obj **handle_ptr,
>   				    struct efi_loaded_image **info_ptr);
>
> -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> -extern void *efi_bounce_buffer;
> -#define EFI_LOADER_BOUNCE_BUFFER_SIZE (64 * 1024 * 1024)
> -#endif
>
>   /* shorten device path */
>   struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp);
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 6130af14337e..bc08776ef4a6 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -490,13 +490,6 @@ config EFI_HTTP_PROTOCOL
>   endmenu
>
>   menu "Misc options"
> -config EFI_LOADER_BOUNCE_BUFFER
> -	bool "EFI Applications use bounce buffers for DMA operations"
> -	help
> -	  Some hardware does not support DMA to full 64bit addresses. For this
> -	  hardware we can create a bounce buffer so that payloads don't have to
> -	  worry about platform details.
> -
>   config EFI_GRUB_ARM32_WORKAROUND
>   	bool "Workaround for GRUB on 32bit ARM"
>   	default n if ARCH_BCM283X || ARCH_SUNXI || ARCH_QEMU
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 5452640354e0..a0202032bb5a 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -105,6 +105,8 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
>   	int blksz;
>   	int blocks;
>   	unsigned long n;
> +	u64 bb = 0xffffffff;
> +	void *bb_ptr = buffer;
>
>   	diskobj = container_of(this, struct efi_disk_obj, ops);
>   	blksz = diskobj->media.block_size;
> @@ -113,27 +115,35 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
>   	EFI_PRINT("blocks=%x lba=%llx blksz=%x dir=%d\n",
>   		  blocks, lba, blksz, direction);
>
> +	if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && (uintptr_t)buffer >= SZ_4G + buffer_size - 1) {
> +		if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_BOOT_SERVICES_DATA,
> +				       buffer_size >> EFI_PAGE_SHIFT, &bb) != EFI_SUCCESS)
> +			return EFI_OUT_OF_RESOURCES;
> +
> +		bb_ptr = (void *)(uintptr_t)bb;
> +	}
>   	/* We only support full block access */
> -	if (buffer_size & (blksz - 1))
> +	if (buffer_size & (blksz - 1)) {
> +		if (buffer != bb_ptr)
> +			efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
>   		return EFI_BAD_BUFFER_SIZE;
> +	}
>
>   	if (CONFIG_IS_ENABLED(PARTITIONS) &&
>   	    device_get_uclass_id(diskobj->header.dev) == UCLASS_PARTITION) {
>   		if (direction == EFI_DISK_READ)
> -			n = disk_blk_read(diskobj->header.dev, lba, blocks,
> -					  buffer);
> +			n = disk_blk_read(diskobj->header.dev, lba, blocks, bb_ptr);
>   		else
> -			n = disk_blk_write(diskobj->header.dev, lba, blocks,
> -					   buffer);
> +			n = disk_blk_write(diskobj->header.dev, lba, blocks, bb_ptr);
>   	} else {
>   		/* dev is a block device (UCLASS_BLK) */
>   		struct blk_desc *desc;
>
>   		desc = dev_get_uclass_plat(diskobj->header.dev);
>   		if (direction == EFI_DISK_READ)
> -			n = blk_dread(desc, lba, blocks, buffer);
> +			n = blk_dread(desc, lba, blocks, bb_ptr);
>   		else
> -			n = blk_dwrite(desc, lba, blocks, buffer);
> +			n = blk_dwrite(desc, lba, blocks, bb_ptr);
>   	}
>
>   	/* We don't do interrupts, so check for timers cooperatively */
> @@ -141,8 +151,16 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
>
>   	EFI_PRINT("n=%lx blocks=%x\n", n, blocks);
>
> -	if (n != blocks)
> +	if (n != blocks) {
> +		if (buffer != bb_ptr)
> +			efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
>   		return EFI_DEVICE_ERROR;
> +	}
> +
> +	if (buffer != bb_ptr) {
> +		memcpy(buffer, bb_ptr, buffer_size);
> +		efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
> +	}
>
>   	return EFI_SUCCESS;
>   }
> @@ -166,7 +184,6 @@ static efi_status_t EFIAPI efi_disk_read_blocks(struct efi_block_io *this,
>   			u32 media_id, u64 lba, efi_uintn_t buffer_size,
>   			void *buffer)
>   {
> -	void *real_buffer = buffer;
>   	efi_status_t r;
>
>   	if (!this)
> @@ -184,31 +201,12 @@ static efi_status_t EFIAPI efi_disk_read_blocks(struct efi_block_io *this,
>   	    (this->media->last_block + 1) * this->media->block_size)
>   		return EFI_INVALID_PARAMETER;
>
> -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> -	if (buffer_size > EFI_LOADER_BOUNCE_BUFFER_SIZE) {
> -		r = efi_disk_read_blocks(this, media_id, lba,
> -			EFI_LOADER_BOUNCE_BUFFER_SIZE, buffer);
> -		if (r != EFI_SUCCESS)
> -			return r;
> -		return efi_disk_read_blocks(this, media_id, lba +
> -			EFI_LOADER_BOUNCE_BUFFER_SIZE / this->media->block_size,
> -			buffer_size - EFI_LOADER_BOUNCE_BUFFER_SIZE,
> -			buffer + EFI_LOADER_BOUNCE_BUFFER_SIZE);
> -	}
> -
> -	real_buffer = efi_bounce_buffer;
> -#endif
> -
>   	EFI_ENTRY("%p, %x, %llx, %zx, %p", this, media_id, lba,
>   		  buffer_size, buffer);
>
> -	r = efi_disk_rw_blocks(this, media_id, lba, buffer_size, real_buffer,
> +	r = efi_disk_rw_blocks(this, media_id, lba, buffer_size, buffer,
>   			       EFI_DISK_READ);
>
> -	/* Copy from bounce buffer to real buffer if necessary */
> -	if ((r == EFI_SUCCESS) && (real_buffer != buffer))
> -		memcpy(buffer, real_buffer, buffer_size);
> -
>   	return EFI_EXIT(r);
>   }
>
> @@ -232,7 +230,6 @@ static efi_status_t EFIAPI efi_disk_write_blocks(struct efi_block_io *this,
>   			u32 media_id, u64 lba, efi_uintn_t buffer_size,
>   			void *buffer)
>   {
> -	void *real_buffer = buffer;
>   	efi_status_t r;
>
>   	if (!this)
> @@ -252,29 +249,10 @@ static efi_status_t EFIAPI efi_disk_write_blocks(struct efi_block_io *this,
>   	    (this->media->last_block + 1) * this->media->block_size)
>   		return EFI_INVALID_PARAMETER;
>
> -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> -	if (buffer_size > EFI_LOADER_BOUNCE_BUFFER_SIZE) {
> -		r = efi_disk_write_blocks(this, media_id, lba,
> -			EFI_LOADER_BOUNCE_BUFFER_SIZE, buffer);
> -		if (r != EFI_SUCCESS)
> -			return r;
> -		return efi_disk_write_blocks(this, media_id, lba +
> -			EFI_LOADER_BOUNCE_BUFFER_SIZE / this->media->block_size,
> -			buffer_size - EFI_LOADER_BOUNCE_BUFFER_SIZE,
> -			buffer + EFI_LOADER_BOUNCE_BUFFER_SIZE);
> -	}
> -
> -	real_buffer = efi_bounce_buffer;
> -#endif
> -
>   	EFI_ENTRY("%p, %x, %llx, %zx, %p", this, media_id, lba,
>   		  buffer_size, buffer);
>
> -	/* Populate bounce buffer if necessary */
> -	if (real_buffer != buffer)
> -		memcpy(real_buffer, buffer, buffer_size);
> -
> -	r = efi_disk_rw_blocks(this, media_id, lba, buffer_size, real_buffer,
> +	r = efi_disk_rw_blocks(this, media_id, lba, buffer_size, buffer,
>   			       EFI_DISK_WRITE);
>
>   	return EFI_EXIT(r);
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index 6d00b1862505..d7a8cb89e5b5 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -40,10 +40,6 @@ struct efi_mem_list {
>   /* This list contains all memory map items */
>   static LIST_HEAD(efi_mem);
>
> -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> -void *efi_bounce_buffer;
> -#endif
> -
>   /**
>    * struct efi_pool_allocation - memory block allocated from pool
>    *
> @@ -852,18 +848,6 @@ int efi_memory_init(void)
>
>   	add_u_boot_and_runtime();
>
> -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> -	/* Request a 32bit 64MB bounce buffer region */
> -	uint64_t efi_bounce_buffer_addr = 0xffffffff;
> -
> -	if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_BOOT_SERVICES_DATA,
> -			       (64 * 1024 * 1024) >> EFI_PAGE_SHIFT,
> -			       &efi_bounce_buffer_addr) != EFI_SUCCESS)
> -		return -1;
> -
> -	efi_bounce_buffer = (void*)(uintptr_t)efi_bounce_buffer_addr;
> -#endif
> -
>   	return 0;
>   }
>
> --
> 2.47.2
>

Hi Heinrich,

On Mon, 24 Mar 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 17.03.25 14:38, Ilias Apalodimas wrote:
>
> %s/EFI_BOUNCE_BUFFER/CONFIG_EFI_LOADER_BOUNCE_BUFFER/
>
> > The EFI subsystem defines its own bounce buffer for devices that
> > can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> > which can be reused instead of defining another symbol.
> > The only limitation for EFI is that we don't know how big the file a user
> > chooses to transfer is and as a result we can't depend on allocating the
> > memory from the malloc area, which can prove too small.
> >
> > So allocate an EFI buffer of the correct size and pass it to the DM,
> > which already supports bounce buffering via bounce_buffer_start_extalign()
>
> Looking at
>
>      if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && desc->bb) {
>
> in drivers/block/blk-uclass.c the bounce buffer has to be explicitly
> enabled by the device driver. Only the scsi drivers sets bb = true.
>
> Cf. 81bd22e935dc ("rockchip: block: blk-uclass: add bounce buffer flag
> to blk_desc")
>
> Which device-drivers of the boards mentioned below do actually need
> bounce buffering?

Unfortunately, I don't have any of the hardware to test and I havent
worked with that platform much.
That 'bb' variable and the fact that EFI needs bigger allocations is
why I ended up allocationg properly aligned memory from the EFI
subsystem. But as Mark pointed out, the cache flush is a no go for
now, so I'll drop this and see if I find time to rework the bounce
buffer logic overall

Thanks
/Ilias

>
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >   arch/arm/Kconfig                              |  8 ++
> >   configs/ls1028aqds_tfa_SECURE_BOOT_defconfig  |  1 -
> >   configs/ls1028aqds_tfa_defconfig              |  1 -
> >   configs/ls1028aqds_tfa_lpuart_defconfig       |  1 -
> >   configs/ls1028ardb_tfa_SECURE_BOOT_defconfig  |  1 -
> >   configs/ls1028ardb_tfa_defconfig              |  1 -
> >   configs/ls1043ardb_tfa_SECURE_BOOT_defconfig  |  1 -
> >   configs/ls1043ardb_tfa_defconfig              |  1 -
> >   configs/ls1046ardb_tfa_SECURE_BOOT_defconfig  |  1 -
> >   configs/ls1046ardb_tfa_defconfig              |  1 -
> >   configs/ls1088aqds_tfa_defconfig              |  1 -
> >   configs/ls1088ardb_tfa_SECURE_BOOT_defconfig  |  1 -
> >   configs/ls1088ardb_tfa_defconfig              |  1 -
> >   configs/ls2088aqds_tfa_defconfig              |  1 -
> >   configs/ls2088ardb_tfa_SECURE_BOOT_defconfig  |  1 -
> >   configs/ls2088ardb_tfa_defconfig              |  1 -
> >   configs/lx2160aqds_tfa_SECURE_BOOT_defconfig  |  1 -
> >   configs/lx2160aqds_tfa_defconfig              |  1 -
> >   configs/lx2160ardb_tfa_SECURE_BOOT_defconfig  |  1 -
> >   configs/lx2160ardb_tfa_defconfig              |  1 -
> >   configs/lx2160ardb_tfa_stmm_defconfig         |  1 -
> >   configs/lx2162aqds_tfa_SECURE_BOOT_defconfig  |  1 -
> >   configs/lx2162aqds_tfa_defconfig              |  1 -
> >   .../lx2162aqds_tfa_verified_boot_defconfig    |  1 -
> >   configs/ten64_tfa_defconfig                   |  1 -
> >   include/efi_loader.h                          |  4 -
> >   lib/efi_loader/Kconfig                        |  7 --
> >   lib/efi_loader/efi_disk.c                     | 78 +++++++------------
> >   lib/efi_loader/efi_memory.c                   | 16 ----
> >   29 files changed, 36 insertions(+), 101 deletions(-)
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index cf08fe63f1e7..bb946e69254c 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -1417,6 +1417,7 @@ config TARGET_LS2080A_EMU
> >       select ARCH_LS2080A
> >       select ARM64
> >       select ARMV8_MULTIENTRY
> > +     select BOUNCE_BUFFER
> >       select FSL_DDR_SYNC_REFRESH
> >       select GPIO_EXTRA_HEADER
> >       help
> > @@ -1432,6 +1433,7 @@ config TARGET_LS1088AQDS
> >       select ARMV8_MULTIENTRY
> >       select ARCH_SUPPORT_TFABOOT
> >       select BOARD_LATE_INIT
> > +     select BOUNCE_BUFFER
> >       select GPIO_EXTRA_HEADER
> >       select SUPPORT_SPL
> >       select FSL_DDR_INTERACTIVE if !SD_BOOT
> > @@ -1448,6 +1450,7 @@ config TARGET_LS2080AQDS
> >       select ARMV8_MULTIENTRY
> >       select ARCH_SUPPORT_TFABOOT
> >       select BOARD_LATE_INIT
> > +     select BOUNCE_BUFFER
> >       select GPIO_EXTRA_HEADER
> >       select SUPPORT_SPL
> >       imply SCSI
> > @@ -1467,6 +1470,7 @@ config TARGET_LS2080ARDB
> >       select ARMV8_MULTIENTRY
> >       select ARCH_SUPPORT_TFABOOT
> >       select BOARD_LATE_INIT
> > +     select BOUNCE_BUFFER
> >       select SUPPORT_SPL
> >       select FSL_DDR_BIST
> >       select FSL_DDR_INTERACTIVE if !SPL
> > @@ -1485,6 +1489,7 @@ config TARGET_LS2081ARDB
> >       select ARM64
> >       select ARMV8_MULTIENTRY
> >       select BOARD_LATE_INIT
> > +     select BOUNCE_BUFFER
> >       select GPIO_EXTRA_HEADER
> >       select SUPPORT_SPL
> >       help
> > @@ -1500,6 +1505,7 @@ config TARGET_LX2160ARDB
> >       select ARMV8_MULTIENTRY
> >       select ARCH_SUPPORT_TFABOOT
> >       select BOARD_LATE_INIT
> > +     select BOUNCE_BUFFER
> >       select GPIO_EXTRA_HEADER
> >       help
> >         Support for NXP LX2160ARDB platform.
> > @@ -1514,6 +1520,7 @@ config TARGET_LX2160AQDS
> >       select ARMV8_MULTIENTRY
> >       select ARCH_SUPPORT_TFABOOT
> >       select BOARD_LATE_INIT
> > +     select BOUNCE_BUFFER
> >       select GPIO_EXTRA_HEADER
> >       help
> >         Support for NXP LX2160AQDS platform.
> > @@ -1529,6 +1536,7 @@ config TARGET_LX2162AQDS
> >       select ARMV8_MULTIENTRY
> >       select ARCH_SUPPORT_TFABOOT
> >       select BOARD_LATE_INIT
> > +     select BOUNCE_BUFFER
> >       select GPIO_EXTRA_HEADER
> >       help
> >         Support for NXP LX2162AQDS platform.
> > diff --git a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
> > index 97eb7d9dca4c..e3b1018f6f8e 100644
> > --- a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
> > +++ b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig
> > @@ -23,7 +23,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
> >   CONFIG_SYS_MEMTEST_END=0x9fffffff
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_DISTRO_DEFAULTS=y
> >   CONFIG_BOOTDELAY=10
> > diff --git a/configs/ls1028aqds_tfa_defconfig b/configs/ls1028aqds_tfa_defconfig
> > index cc53c17f543b..5b7dd4bf7dd5 100644
> > --- a/configs/ls1028aqds_tfa_defconfig
> > +++ b/configs/ls1028aqds_tfa_defconfig
> > @@ -25,7 +25,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
> >   CONFIG_SYS_MEMTEST_END=0x9fffffff
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_DISTRO_DEFAULTS=y
> >   CONFIG_BOOTDELAY=10
> > diff --git a/configs/ls1028aqds_tfa_lpuart_defconfig b/configs/ls1028aqds_tfa_lpuart_defconfig
> > index 8da7271c7f85..a38c81e83199 100644
> > --- a/configs/ls1028aqds_tfa_lpuart_defconfig
> > +++ b/configs/ls1028aqds_tfa_lpuart_defconfig
> > @@ -24,7 +24,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
> >   CONFIG_SYS_MEMTEST_END=0x9fffffff
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_DISTRO_DEFAULTS=y
> >   CONFIG_BOOTDELAY=10
> > diff --git a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
> > index 3f71a37559be..da225e1dad8f 100644
> > --- a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
> > +++ b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig
> > @@ -23,7 +23,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
> >   CONFIG_SYS_MEMTEST_END=0x9fffffff
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_DISTRO_DEFAULTS=y
> >   CONFIG_BOOTDELAY=10
> > diff --git a/configs/ls1028ardb_tfa_defconfig b/configs/ls1028ardb_tfa_defconfig
> > index 89e452bdb1a8..829cc5204d34 100644
> > --- a/configs/ls1028ardb_tfa_defconfig
> > +++ b/configs/ls1028ardb_tfa_defconfig
> > @@ -25,7 +25,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
> >   CONFIG_SYS_MEMTEST_END=0x9fffffff
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_DISTRO_DEFAULTS=y
> >   CONFIG_BOOTDELAY=10
> > diff --git a/configs/ls1043ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1043ardb_tfa_SECURE_BOOT_defconfig
> > index 1b41fe608dae..04ca2a3afd81 100644
> > --- a/configs/ls1043ardb_tfa_SECURE_BOOT_defconfig
> > +++ b/configs/ls1043ardb_tfa_SECURE_BOOT_defconfig
> > @@ -24,7 +24,6 @@ CONFIG_PCIE2=y
> >   CONFIG_PCIE3=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_DISTRO_DEFAULTS=y
> >   CONFIG_BOOTDELAY=10
> > diff --git a/configs/ls1043ardb_tfa_defconfig b/configs/ls1043ardb_tfa_defconfig
> > index ca100bd4bca9..a6ca63cd8f79 100644
> > --- a/configs/ls1043ardb_tfa_defconfig
> > +++ b/configs/ls1043ardb_tfa_defconfig
> > @@ -26,7 +26,6 @@ CONFIG_PCIE2=y
> >   CONFIG_PCIE3=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_DISTRO_DEFAULTS=y
> >   CONFIG_BOOTDELAY=10
> > diff --git a/configs/ls1046ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1046ardb_tfa_SECURE_BOOT_defconfig
> > index 6b4e834ad4a0..9c22d542a6f8 100644
> > --- a/configs/ls1046ardb_tfa_SECURE_BOOT_defconfig
> > +++ b/configs/ls1046ardb_tfa_SECURE_BOOT_defconfig
> > @@ -24,7 +24,6 @@ CONFIG_PCIE2=y
> >   CONFIG_PCIE3=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_DISTRO_DEFAULTS=y
> >   CONFIG_BOOTDELAY=10
> > diff --git a/configs/ls1046ardb_tfa_defconfig b/configs/ls1046ardb_tfa_defconfig
> > index a8f20f9b87b7..4eab8ea19e8e 100644
> > --- a/configs/ls1046ardb_tfa_defconfig
> > +++ b/configs/ls1046ardb_tfa_defconfig
> > @@ -26,7 +26,6 @@ CONFIG_PCIE2=y
> >   CONFIG_PCIE3=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_DISTRO_DEFAULTS=y
> >   CONFIG_BOOTDELAY=10
> > diff --git a/configs/ls1088aqds_tfa_defconfig b/configs/ls1088aqds_tfa_defconfig
> > index df86bf982ee4..31d56f587af8 100644
> > --- a/configs/ls1088aqds_tfa_defconfig
> > +++ b/configs/ls1088aqds_tfa_defconfig
> > @@ -29,7 +29,6 @@ CONFIG_SYS_MEMTEST_END=0x9fffffff
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> >   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_DISTRO_DEFAULTS=y
> >   CONFIG_OF_BOARD_SETUP=y
> > diff --git a/configs/ls1088ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1088ardb_tfa_SECURE_BOOT_defconfig
> > index df472eeb404d..c910b73634f9 100644
> > --- a/configs/ls1088ardb_tfa_SECURE_BOOT_defconfig
> > +++ b/configs/ls1088ardb_tfa_SECURE_BOOT_defconfig
> > @@ -26,7 +26,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
> >   CONFIG_SYS_MEMTEST_END=0x9fffffff
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_DISTRO_DEFAULTS=y
> >   CONFIG_OF_BOARD_SETUP=y
> > diff --git a/configs/ls1088ardb_tfa_defconfig b/configs/ls1088ardb_tfa_defconfig
> > index 8d640f6594b2..682007e803d1 100644
> > --- a/configs/ls1088ardb_tfa_defconfig
> > +++ b/configs/ls1088ardb_tfa_defconfig
> > @@ -28,7 +28,6 @@ CONFIG_SYS_MEMTEST_START=0x80000000
> >   CONFIG_SYS_MEMTEST_END=0x9fffffff
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_DISTRO_DEFAULTS=y
> >   CONFIG_OF_BOARD_SETUP=y
> > diff --git a/configs/ls2088aqds_tfa_defconfig b/configs/ls2088aqds_tfa_defconfig
> > index 8c86b8b11eb3..ea73a4cda971 100644
> > --- a/configs/ls2088aqds_tfa_defconfig
> > +++ b/configs/ls2088aqds_tfa_defconfig
> > @@ -23,7 +23,6 @@ CONFIG_FSL_QIXIS=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> >   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_BOOTDELAY=10
> >   CONFIG_OF_BOARD_SETUP=y
> > diff --git a/configs/ls2088ardb_tfa_SECURE_BOOT_defconfig b/configs/ls2088ardb_tfa_SECURE_BOOT_defconfig
> > index 3fd1de967896..7f53397cdcb1 100644
> > --- a/configs/ls2088ardb_tfa_SECURE_BOOT_defconfig
> > +++ b/configs/ls2088ardb_tfa_SECURE_BOOT_defconfig
> > @@ -27,7 +27,6 @@ CONFIG_FSL_QIXIS=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> >   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_BOOTDELAY=10
> >   CONFIG_OF_BOARD_SETUP=y
> > diff --git a/configs/ls2088ardb_tfa_defconfig b/configs/ls2088ardb_tfa_defconfig
> > index f053044fa56e..398dfe068015 100644
> > --- a/configs/ls2088ardb_tfa_defconfig
> > +++ b/configs/ls2088ardb_tfa_defconfig
> > @@ -29,7 +29,6 @@ CONFIG_FSL_QIXIS=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> >   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_BOOTDELAY=10
> >   CONFIG_OF_BOARD_SETUP=y
> > diff --git a/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig b/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig
> > index 321ed71993b8..afb842978209 100644
> > --- a/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig
> > +++ b/configs/lx2160aqds_tfa_SECURE_BOOT_defconfig
> > @@ -28,7 +28,6 @@ CONFIG_FSL_QIXIS=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> >   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_OF_BOARD_SETUP=y
> >   CONFIG_OF_STDOUT_VIA_ALIAS=y
> > diff --git a/configs/lx2160aqds_tfa_defconfig b/configs/lx2160aqds_tfa_defconfig
> > index 465f07ee7f82..ec23ed384c44 100644
> > --- a/configs/lx2160aqds_tfa_defconfig
> > +++ b/configs/lx2160aqds_tfa_defconfig
> > @@ -30,7 +30,6 @@ CONFIG_FSL_QIXIS=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> >   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_BOOTDELAY=10
> >   CONFIG_OF_BOARD_SETUP=y
> > diff --git a/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig b/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig
> > index 478a01b566c6..71f4436eca06 100644
> > --- a/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig
> > +++ b/configs/lx2160ardb_tfa_SECURE_BOOT_defconfig
> > @@ -29,7 +29,6 @@ CONFIG_FSL_QIXIS=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> >   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_OF_BOARD_SETUP=y
> >   CONFIG_OF_STDOUT_VIA_ALIAS=y
> > diff --git a/configs/lx2160ardb_tfa_defconfig b/configs/lx2160ardb_tfa_defconfig
> > index 8fd18825569c..6be3890de3ed 100644
> > --- a/configs/lx2160ardb_tfa_defconfig
> > +++ b/configs/lx2160ardb_tfa_defconfig
> > @@ -31,7 +31,6 @@ CONFIG_FSL_QIXIS=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> >   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_BOOTDELAY=10
> >   CONFIG_OF_BOARD_SETUP=y
> > diff --git a/configs/lx2160ardb_tfa_stmm_defconfig b/configs/lx2160ardb_tfa_stmm_defconfig
> > index cf65897af898..ef89b0f34575 100644
> > --- a/configs/lx2160ardb_tfa_stmm_defconfig
> > +++ b/configs/lx2160ardb_tfa_stmm_defconfig
> > @@ -32,7 +32,6 @@ CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> >   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> >   CONFIG_EFI_MM_COMM_TEE=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_BOOTDELAY=10
> >   CONFIG_OF_BOARD_SETUP=y
> > diff --git a/configs/lx2162aqds_tfa_SECURE_BOOT_defconfig b/configs/lx2162aqds_tfa_SECURE_BOOT_defconfig
> > index 46b6085a52fe..2b31be10899b 100644
> > --- a/configs/lx2162aqds_tfa_SECURE_BOOT_defconfig
> > +++ b/configs/lx2162aqds_tfa_SECURE_BOOT_defconfig
> > @@ -28,7 +28,6 @@ CONFIG_FSL_QIXIS=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> >   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_OF_BOARD_SETUP=y
> >   CONFIG_OF_STDOUT_VIA_ALIAS=y
> > diff --git a/configs/lx2162aqds_tfa_defconfig b/configs/lx2162aqds_tfa_defconfig
> > index 41e0262f3ca5..43f8069cfde5 100644
> > --- a/configs/lx2162aqds_tfa_defconfig
> > +++ b/configs/lx2162aqds_tfa_defconfig
> > @@ -30,7 +30,6 @@ CONFIG_FSL_QIXIS=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> >   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_BOOTDELAY=10
> >   CONFIG_OF_BOARD_SETUP=y
> > diff --git a/configs/lx2162aqds_tfa_verified_boot_defconfig b/configs/lx2162aqds_tfa_verified_boot_defconfig
> > index 7abfdbafbdf1..33b362c20b65 100644
> > --- a/configs/lx2162aqds_tfa_verified_boot_defconfig
> > +++ b/configs/lx2162aqds_tfa_verified_boot_defconfig
> > @@ -30,7 +30,6 @@ CONFIG_FSL_QIXIS=y
> >   CONFIG_REMAKE_ELF=y
> >   CONFIG_MP=y
> >   CONFIG_DYNAMIC_SYS_CLK_FREQ=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_SIGNATURE=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_BOOTDELAY=10
> > diff --git a/configs/ten64_tfa_defconfig b/configs/ten64_tfa_defconfig
> > index a65209e8335b..f35de31718f7 100644
> > --- a/configs/ten64_tfa_defconfig
> > +++ b/configs/ten64_tfa_defconfig
> > @@ -16,7 +16,6 @@ CONFIG_PCI=y
> >   CONFIG_AHCI=y
> >   CONFIG_SYS_FSL_NUM_CC_PLLS=3
> >   CONFIG_MP=y
> > -CONFIG_EFI_LOADER_BOUNCE_BUFFER=y
> >   CONFIG_FIT_VERBOSE=y
> >   CONFIG_BOOTSTD_FULL=y
> >   CONFIG_BOOTSTD_BOOTCOMMAND=y
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index e9c10819ba26..3ed27e97a083 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -894,10 +894,6 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path,
> >                                   struct efi_loaded_image_obj **handle_ptr,
> >                                   struct efi_loaded_image **info_ptr);
> >
> > -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> > -extern void *efi_bounce_buffer;
> > -#define EFI_LOADER_BOUNCE_BUFFER_SIZE (64 * 1024 * 1024)
> > -#endif
> >
> >   /* shorten device path */
> >   struct efi_device_path *efi_dp_shorten(struct efi_device_path *dp);
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 6130af14337e..bc08776ef4a6 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -490,13 +490,6 @@ config EFI_HTTP_PROTOCOL
> >   endmenu
> >
> >   menu "Misc options"
> > -config EFI_LOADER_BOUNCE_BUFFER
> > -     bool "EFI Applications use bounce buffers for DMA operations"
> > -     help
> > -       Some hardware does not support DMA to full 64bit addresses. For this
> > -       hardware we can create a bounce buffer so that payloads don't have to
> > -       worry about platform details.
> > -
> >   config EFI_GRUB_ARM32_WORKAROUND
> >       bool "Workaround for GRUB on 32bit ARM"
> >       default n if ARCH_BCM283X || ARCH_SUNXI || ARCH_QEMU
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index 5452640354e0..a0202032bb5a 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -105,6 +105,8 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
> >       int blksz;
> >       int blocks;
> >       unsigned long n;
> > +     u64 bb = 0xffffffff;
> > +     void *bb_ptr = buffer;
> >
> >       diskobj = container_of(this, struct efi_disk_obj, ops);
> >       blksz = diskobj->media.block_size;
> > @@ -113,27 +115,35 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
> >       EFI_PRINT("blocks=%x lba=%llx blksz=%x dir=%d\n",
> >                 blocks, lba, blksz, direction);
> >
> > +     if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && (uintptr_t)buffer >= SZ_4G + buffer_size - 1) {
> > +             if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_BOOT_SERVICES_DATA,
> > +                                    buffer_size >> EFI_PAGE_SHIFT, &bb) != EFI_SUCCESS)
> > +                     return EFI_OUT_OF_RESOURCES;
> > +
> > +             bb_ptr = (void *)(uintptr_t)bb;
> > +     }
> >       /* We only support full block access */
> > -     if (buffer_size & (blksz - 1))
> > +     if (buffer_size & (blksz - 1)) {
> > +             if (buffer != bb_ptr)
> > +                     efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
> >               return EFI_BAD_BUFFER_SIZE;
> > +     }
> >
> >       if (CONFIG_IS_ENABLED(PARTITIONS) &&
> >           device_get_uclass_id(diskobj->header.dev) == UCLASS_PARTITION) {
> >               if (direction == EFI_DISK_READ)
> > -                     n = disk_blk_read(diskobj->header.dev, lba, blocks,
> > -                                       buffer);
> > +                     n = disk_blk_read(diskobj->header.dev, lba, blocks, bb_ptr);
> >               else
> > -                     n = disk_blk_write(diskobj->header.dev, lba, blocks,
> > -                                        buffer);
> > +                     n = disk_blk_write(diskobj->header.dev, lba, blocks, bb_ptr);
> >       } else {
> >               /* dev is a block device (UCLASS_BLK) */
> >               struct blk_desc *desc;
> >
> >               desc = dev_get_uclass_plat(diskobj->header.dev);
> >               if (direction == EFI_DISK_READ)
> > -                     n = blk_dread(desc, lba, blocks, buffer);
> > +                     n = blk_dread(desc, lba, blocks, bb_ptr);
> >               else
> > -                     n = blk_dwrite(desc, lba, blocks, buffer);
> > +                     n = blk_dwrite(desc, lba, blocks, bb_ptr);
> >       }
> >
> >       /* We don't do interrupts, so check for timers cooperatively */
> > @@ -141,8 +151,16 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
> >
> >       EFI_PRINT("n=%lx blocks=%x\n", n, blocks);
> >
> > -     if (n != blocks)
> > +     if (n != blocks) {
> > +             if (buffer != bb_ptr)
> > +                     efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
> >               return EFI_DEVICE_ERROR;
> > +     }
> > +
> > +     if (buffer != bb_ptr) {
> > +             memcpy(buffer, bb_ptr, buffer_size);
> > +             efi_free_pages(bb, buffer_size >> EFI_PAGE_SHIFT);
> > +     }
> >
> >       return EFI_SUCCESS;
> >   }
> > @@ -166,7 +184,6 @@ static efi_status_t EFIAPI efi_disk_read_blocks(struct efi_block_io *this,
> >                       u32 media_id, u64 lba, efi_uintn_t buffer_size,
> >                       void *buffer)
> >   {
> > -     void *real_buffer = buffer;
> >       efi_status_t r;
> >
> >       if (!this)
> > @@ -184,31 +201,12 @@ static efi_status_t EFIAPI efi_disk_read_blocks(struct efi_block_io *this,
> >           (this->media->last_block + 1) * this->media->block_size)
> >               return EFI_INVALID_PARAMETER;
> >
> > -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> > -     if (buffer_size > EFI_LOADER_BOUNCE_BUFFER_SIZE) {
> > -             r = efi_disk_read_blocks(this, media_id, lba,
> > -                     EFI_LOADER_BOUNCE_BUFFER_SIZE, buffer);
> > -             if (r != EFI_SUCCESS)
> > -                     return r;
> > -             return efi_disk_read_blocks(this, media_id, lba +
> > -                     EFI_LOADER_BOUNCE_BUFFER_SIZE / this->media->block_size,
> > -                     buffer_size - EFI_LOADER_BOUNCE_BUFFER_SIZE,
> > -                     buffer + EFI_LOADER_BOUNCE_BUFFER_SIZE);
> > -     }
> > -
> > -     real_buffer = efi_bounce_buffer;
> > -#endif
> > -
> >       EFI_ENTRY("%p, %x, %llx, %zx, %p", this, media_id, lba,
> >                 buffer_size, buffer);
> >
> > -     r = efi_disk_rw_blocks(this, media_id, lba, buffer_size, real_buffer,
> > +     r = efi_disk_rw_blocks(this, media_id, lba, buffer_size, buffer,
> >                              EFI_DISK_READ);
> >
> > -     /* Copy from bounce buffer to real buffer if necessary */
> > -     if ((r == EFI_SUCCESS) && (real_buffer != buffer))
> > -             memcpy(buffer, real_buffer, buffer_size);
> > -
> >       return EFI_EXIT(r);
> >   }
> >
> > @@ -232,7 +230,6 @@ static efi_status_t EFIAPI efi_disk_write_blocks(struct efi_block_io *this,
> >                       u32 media_id, u64 lba, efi_uintn_t buffer_size,
> >                       void *buffer)
> >   {
> > -     void *real_buffer = buffer;
> >       efi_status_t r;
> >
> >       if (!this)
> > @@ -252,29 +249,10 @@ static efi_status_t EFIAPI efi_disk_write_blocks(struct efi_block_io *this,
> >           (this->media->last_block + 1) * this->media->block_size)
> >               return EFI_INVALID_PARAMETER;
> >
> > -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> > -     if (buffer_size > EFI_LOADER_BOUNCE_BUFFER_SIZE) {
> > -             r = efi_disk_write_blocks(this, media_id, lba,
> > -                     EFI_LOADER_BOUNCE_BUFFER_SIZE, buffer);
> > -             if (r != EFI_SUCCESS)
> > -                     return r;
> > -             return efi_disk_write_blocks(this, media_id, lba +
> > -                     EFI_LOADER_BOUNCE_BUFFER_SIZE / this->media->block_size,
> > -                     buffer_size - EFI_LOADER_BOUNCE_BUFFER_SIZE,
> > -                     buffer + EFI_LOADER_BOUNCE_BUFFER_SIZE);
> > -     }
> > -
> > -     real_buffer = efi_bounce_buffer;
> > -#endif
> > -
> >       EFI_ENTRY("%p, %x, %llx, %zx, %p", this, media_id, lba,
> >                 buffer_size, buffer);
> >
> > -     /* Populate bounce buffer if necessary */
> > -     if (real_buffer != buffer)
> > -             memcpy(real_buffer, buffer, buffer_size);
> > -
> > -     r = efi_disk_rw_blocks(this, media_id, lba, buffer_size, real_buffer,
> > +     r = efi_disk_rw_blocks(this, media_id, lba, buffer_size, buffer,
> >                              EFI_DISK_WRITE);
> >
> >       return EFI_EXIT(r);
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index 6d00b1862505..d7a8cb89e5b5 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -40,10 +40,6 @@ struct efi_mem_list {
> >   /* This list contains all memory map items */
> >   static LIST_HEAD(efi_mem);
> >
> > -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> > -void *efi_bounce_buffer;
> > -#endif
> > -
> >   /**
> >    * struct efi_pool_allocation - memory block allocated from pool
> >    *
> > @@ -852,18 +848,6 @@ int efi_memory_init(void)
> >
> >       add_u_boot_and_runtime();
> >
> > -#ifdef CONFIG_EFI_LOADER_BOUNCE_BUFFER
> > -     /* Request a 32bit 64MB bounce buffer region */
> > -     uint64_t efi_bounce_buffer_addr = 0xffffffff;
> > -
> > -     if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_BOOT_SERVICES_DATA,
> > -                            (64 * 1024 * 1024) >> EFI_PAGE_SHIFT,
> > -                            &efi_bounce_buffer_addr) != EFI_SUCCESS)
> > -             return -1;
> > -
> > -     efi_bounce_buffer = (void*)(uintptr_t)efi_bounce_buffer_addr;
> > -#endif
> > -
> >       return 0;
> >   }
> >
> > --
> > 2.47.2
> >
>

Hi Ilias,

On Wed, 26 Mar 2025 at 02:37, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Heinrich,
>
> On Mon, 24 Mar 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > On 17.03.25 14:38, Ilias Apalodimas wrote:
> >
> > %s/EFI_BOUNCE_BUFFER/CONFIG_EFI_LOADER_BOUNCE_BUFFER/
> >
> > > The EFI subsystem defines its own bounce buffer for devices that
> > > can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> > > which can be reused instead of defining another symbol.
> > > The only limitation for EFI is that we don't know how big the file a user
> > > chooses to transfer is and as a result we can't depend on allocating the
> > > memory from the malloc area, which can prove too small.
> > >
> > > So allocate an EFI buffer of the correct size and pass it to the DM,
> > > which already supports bounce buffering via bounce_buffer_start_extalign()
> >
> > Looking at
> >
> >      if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && desc->bb) {
> >
> > in drivers/block/blk-uclass.c the bounce buffer has to be explicitly
> > enabled by the device driver. Only the scsi drivers sets bb = true.
> >
> > Cf. 81bd22e935dc ("rockchip: block: blk-uclass: add bounce buffer flag
> > to blk_desc")
> >
> > Which device-drivers of the boards mentioned below do actually need
> > bounce buffering?
>
> Unfortunately, I don't have any of the hardware to test and I havent
> worked with that platform much.
> That 'bb' variable and the fact that EFI needs bigger allocations is
> why I ended up allocationg properly aligned memory from the EFI
> subsystem. But as Mark pointed out, the cache flush is a no go for
> now, so I'll drop this and see if I find time to rework the bounce
> buffer logic overall

There was quite a bit of discussion about all this in the context of
my attempt to just add a message to warn the user[1]

We might consider adding an event to reserve memory before relocation,
along with a way to discover (in board_r) where the memory was
allocated. That would make the solution more generic.

For the <4GB case we could perhaps add generic support for that in
board_f, i.e. the ability to reserve a region for boards that need it.

Regards,
SImon

[1] https://lore.kernel.org/u-boot/?q=%22Show+the+location+of+the+bounce+buffer+when+debugging%22

Hi Simon

On Thu, 27 Mar 2025 at 15:33, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ilias,
>
> On Wed, 26 Mar 2025 at 02:37, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Heinrich,
> >
> > On Mon, 24 Mar 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > >
> > > On 17.03.25 14:38, Ilias Apalodimas wrote:
> > >
> > > %s/EFI_BOUNCE_BUFFER/CONFIG_EFI_LOADER_BOUNCE_BUFFER/
> > >
> > > > The EFI subsystem defines its own bounce buffer for devices that
> > > > can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> > > > which can be reused instead of defining another symbol.
> > > > The only limitation for EFI is that we don't know how big the file a user
> > > > chooses to transfer is and as a result we can't depend on allocating the
> > > > memory from the malloc area, which can prove too small.
> > > >
> > > > So allocate an EFI buffer of the correct size and pass it to the DM,
> > > > which already supports bounce buffering via bounce_buffer_start_extalign()
> > >
> > > Looking at
> > >
> > >      if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && desc->bb) {
> > >
> > > in drivers/block/blk-uclass.c the bounce buffer has to be explicitly
> > > enabled by the device driver. Only the scsi drivers sets bb = true.
> > >
> > > Cf. 81bd22e935dc ("rockchip: block: blk-uclass: add bounce buffer flag
> > > to blk_desc")
> > >
> > > Which device-drivers of the boards mentioned below do actually need
> > > bounce buffering?
> >
> > Unfortunately, I don't have any of the hardware to test and I havent
> > worked with that platform much.
> > That 'bb' variable and the fact that EFI needs bigger allocations is
> > why I ended up allocationg properly aligned memory from the EFI
> > subsystem. But as Mark pointed out, the cache flush is a no go for
> > now, so I'll drop this and see if I find time to rework the bounce
> > buffer logic overall
>
> There was quite a bit of discussion about all this in the context of
> my attempt to just add a message to warn the user[1]
>
> We might consider adding an event to reserve memory before relocation,
> along with a way to discover (in board_r) where the memory was
> allocated. That would make the solution more generic.

I am not sure what you are trying to solve here. The EFI bounce buffer
after the LMB patches can't overwrite memory, nor can it be
overwritten.

Thanks
/Ilias

>
> For the <4GB case we could perhaps add generic support for that in
> board_f, i.e. the ability to reserve a region for boards that need it.
>
> Regards,
> SImon
>
> [1] https://lore.kernel.org/u-boot/?q=%22Show+the+location+of+the+bounce+buffer+when+debugging%22

Hi Ilias,

On Thu, 27 Mar 2025 at 15:19, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Simon
>
> On Thu, 27 Mar 2025 at 15:33, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Ilias,
> >
> > On Wed, 26 Mar 2025 at 02:37, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > Hi Heinrich,
> > >
> > > On Mon, 24 Mar 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > >
> > > > On 17.03.25 14:38, Ilias Apalodimas wrote:
> > > >
> > > > %s/EFI_BOUNCE_BUFFER/CONFIG_EFI_LOADER_BOUNCE_BUFFER/
> > > >
> > > > > The EFI subsystem defines its own bounce buffer for devices that
> > > > > can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> > > > > which can be reused instead of defining another symbol.
> > > > > The only limitation for EFI is that we don't know how big the file a user
> > > > > chooses to transfer is and as a result we can't depend on allocating the
> > > > > memory from the malloc area, which can prove too small.
> > > > >
> > > > > So allocate an EFI buffer of the correct size and pass it to the DM,
> > > > > which already supports bounce buffering via bounce_buffer_start_extalign()
> > > >
> > > > Looking at
> > > >
> > > >      if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && desc->bb) {
> > > >
> > > > in drivers/block/blk-uclass.c the bounce buffer has to be explicitly
> > > > enabled by the device driver. Only the scsi drivers sets bb = true.
> > > >
> > > > Cf. 81bd22e935dc ("rockchip: block: blk-uclass: add bounce buffer flag
> > > > to blk_desc")
> > > >
> > > > Which device-drivers of the boards mentioned below do actually need
> > > > bounce buffering?
> > >
> > > Unfortunately, I don't have any of the hardware to test and I havent
> > > worked with that platform much.
> > > That 'bb' variable and the fact that EFI needs bigger allocations is
> > > why I ended up allocationg properly aligned memory from the EFI
> > > subsystem. But as Mark pointed out, the cache flush is a no go for
> > > now, so I'll drop this and see if I find time to rework the bounce
> > > buffer logic overall
> >
> > There was quite a bit of discussion about all this in the context of
> > my attempt to just add a message to warn the user[1]
> >
> > We might consider adding an event to reserve memory before relocation,
> > along with a way to discover (in board_r) where the memory was
> > allocated. That would make the solution more generic.
>
> I am not sure what you are trying to solve here. The EFI bounce buffer
> after the LMB patches can't overwrite memory, nor can it be
> overwritten.

I am thinking of we can create a single implementation of the
bouncebuf logic which also works for EFI.

I think the two sane things to do are:
- restrict U-Boot to using memory below 4GB for platforms which have
the DMA limitation
- create (in board_f) a special region below 4GB for use with the
bouncebuf logic


>
> Thanks
> /Ilias
>
> >
> > For the <4GB case we could perhaps add generic support for that in
> > board_f, i.e. the ability to reserve a region for boards that need it.

Regards,
SImon

> > [1] https://lore.kernel.org/u-boot/?q=%22Show+the+location+of+the+bounce+buffer+when+debugging%22

On Fri, 28 Mar 2025 at 13:34, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ilias,
>
> On Thu, 27 Mar 2025 at 15:19, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Simon
> >
> > On Thu, 27 Mar 2025 at 15:33, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Ilias,
> > >
> > > On Wed, 26 Mar 2025 at 02:37, Ilias Apalodimas
> > > <ilias.apalodimas@linaro.org> wrote:
> > > >
> > > > Hi Heinrich,
> > > >
> > > > On Mon, 24 Mar 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > >
> > > > > On 17.03.25 14:38, Ilias Apalodimas wrote:
> > > > >
> > > > > %s/EFI_BOUNCE_BUFFER/CONFIG_EFI_LOADER_BOUNCE_BUFFER/
> > > > >
> > > > > > The EFI subsystem defines its own bounce buffer for devices that
> > > > > > can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> > > > > > which can be reused instead of defining another symbol.
> > > > > > The only limitation for EFI is that we don't know how big the file a user
> > > > > > chooses to transfer is and as a result we can't depend on allocating the
> > > > > > memory from the malloc area, which can prove too small.
> > > > > >
> > > > > > So allocate an EFI buffer of the correct size and pass it to the DM,
> > > > > > which already supports bounce buffering via bounce_buffer_start_extalign()
> > > > >
> > > > > Looking at
> > > > >
> > > > >      if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && desc->bb) {
> > > > >
> > > > > in drivers/block/blk-uclass.c the bounce buffer has to be explicitly
> > > > > enabled by the device driver. Only the scsi drivers sets bb = true.
> > > > >
> > > > > Cf. 81bd22e935dc ("rockchip: block: blk-uclass: add bounce buffer flag
> > > > > to blk_desc")
> > > > >
> > > > > Which device-drivers of the boards mentioned below do actually need
> > > > > bounce buffering?
> > > >
> > > > Unfortunately, I don't have any of the hardware to test and I havent
> > > > worked with that platform much.
> > > > That 'bb' variable and the fact that EFI needs bigger allocations is
> > > > why I ended up allocationg properly aligned memory from the EFI
> > > > subsystem. But as Mark pointed out, the cache flush is a no go for
> > > > now, so I'll drop this and see if I find time to rework the bounce
> > > > buffer logic overall
> > >
> > > There was quite a bit of discussion about all this in the context of
> > > my attempt to just add a message to warn the user[1]
> > >
> > > We might consider adding an event to reserve memory before relocation,
> > > along with a way to discover (in board_r) where the memory was
> > > allocated. That would make the solution more generic.
> >
> > I am not sure what you are trying to solve here. The EFI bounce buffer
> > after the LMB patches can't overwrite memory, nor can it be
> > overwritten.
>
> I am thinking of we can create a single implementation of the
> bouncebuf logic which also works for EFI.
>
> I think the two sane things to do are:
> - restrict U-Boot to using memory below 4GB for platforms which have
> the DMA limitation

You don't need that. The bounce buf code has a callback you can use to
define the limitations

> - create (in board_f) a special region below 4GB for use with the
> bouncebuf logic

The only problem with EFI is that you don't know how much memory it
needs and we can't use the existing memalign calls. So if we replace
that memalign in the bounce buffer code, with an lmb reservation we
have everything we need.

/Ilias
>
>
> >
> > Thanks
> > /Ilias
> >
> > >
> > > For the <4GB case we could perhaps add generic support for that in
> > > board_f, i.e. the ability to reserve a region for boards that need it.
>
> Regards,
> SImon
>
> > > [1] https://lore.kernel.org/u-boot/?q=%22Show+the+location+of+the+bounce+buffer+when+debugging%22

On Fri, Mar 28, 2025 at 02:26:39PM +0200, Ilias Apalodimas wrote:
> On Fri, 28 Mar 2025 at 13:34, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Ilias,
> >
> > On Thu, 27 Mar 2025 at 15:19, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > Hi Simon
> > >
> > > On Thu, 27 Mar 2025 at 15:33, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Ilias,
> > > >
> > > > On Wed, 26 Mar 2025 at 02:37, Ilias Apalodimas
> > > > <ilias.apalodimas@linaro.org> wrote:
> > > > >
> > > > > Hi Heinrich,
> > > > >
> > > > > On Mon, 24 Mar 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > > >
> > > > > > On 17.03.25 14:38, Ilias Apalodimas wrote:
> > > > > >
> > > > > > %s/EFI_BOUNCE_BUFFER/CONFIG_EFI_LOADER_BOUNCE_BUFFER/
> > > > > >
> > > > > > > The EFI subsystem defines its own bounce buffer for devices that
> > > > > > > can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> > > > > > > which can be reused instead of defining another symbol.
> > > > > > > The only limitation for EFI is that we don't know how big the file a user
> > > > > > > chooses to transfer is and as a result we can't depend on allocating the
> > > > > > > memory from the malloc area, which can prove too small.
> > > > > > >
> > > > > > > So allocate an EFI buffer of the correct size and pass it to the DM,
> > > > > > > which already supports bounce buffering via bounce_buffer_start_extalign()
> > > > > >
> > > > > > Looking at
> > > > > >
> > > > > >      if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && desc->bb) {
> > > > > >
> > > > > > in drivers/block/blk-uclass.c the bounce buffer has to be explicitly
> > > > > > enabled by the device driver. Only the scsi drivers sets bb = true.
> > > > > >
> > > > > > Cf. 81bd22e935dc ("rockchip: block: blk-uclass: add bounce buffer flag
> > > > > > to blk_desc")
> > > > > >
> > > > > > Which device-drivers of the boards mentioned below do actually need
> > > > > > bounce buffering?
> > > > >
> > > > > Unfortunately, I don't have any of the hardware to test and I havent
> > > > > worked with that platform much.
> > > > > That 'bb' variable and the fact that EFI needs bigger allocations is
> > > > > why I ended up allocationg properly aligned memory from the EFI
> > > > > subsystem. But as Mark pointed out, the cache flush is a no go for
> > > > > now, so I'll drop this and see if I find time to rework the bounce
> > > > > buffer logic overall
> > > >
> > > > There was quite a bit of discussion about all this in the context of
> > > > my attempt to just add a message to warn the user[1]
> > > >
> > > > We might consider adding an event to reserve memory before relocation,
> > > > along with a way to discover (in board_r) where the memory was
> > > > allocated. That would make the solution more generic.
> > >
> > > I am not sure what you are trying to solve here. The EFI bounce buffer
> > > after the LMB patches can't overwrite memory, nor can it be
> > > overwritten.
> >
> > I am thinking of we can create a single implementation of the
> > bouncebuf logic which also works for EFI.
> >
> > I think the two sane things to do are:
> > - restrict U-Boot to using memory below 4GB for platforms which have
> > the DMA limitation
> 
> You don't need that. The bounce buf code has a callback you can use to
> define the limitations
> 
> > - create (in board_f) a special region below 4GB for use with the
> > bouncebuf logic
> 
> The only problem with EFI is that you don't know how much memory it
> needs and we can't use the existing memalign calls. So if we replace
> that memalign in the bounce buffer code, with an lmb reservation we
> have everything we need.

It's not even an EFI problem is it? You could hit the same problem
reading a file from a filesystem outside of EFI too. These specific
SoCs just aren't heavily exercised is one of the challenges I think and
so it's possible that we have a few things to yet improve in the
bounce buffer code (which was added for other SoCs and done as generic
enough starting point for others).

On 28.03.25 17:04, Tom Rini wrote:
> On Fri, Mar 28, 2025 at 02:26:39PM +0200, Ilias Apalodimas wrote:
>> On Fri, 28 Mar 2025 at 13:34, Simon Glass <sjg@chromium.org> wrote:
>>>
>>> Hi Ilias,
>>>
>>> On Thu, 27 Mar 2025 at 15:19, Ilias Apalodimas
>>> <ilias.apalodimas@linaro.org> wrote:
>>>>
>>>> Hi Simon
>>>>
>>>> On Thu, 27 Mar 2025 at 15:33, Simon Glass <sjg@chromium.org> wrote:
>>>>>
>>>>> Hi Ilias,
>>>>>
>>>>> On Wed, 26 Mar 2025 at 02:37, Ilias Apalodimas
>>>>> <ilias.apalodimas@linaro.org> wrote:
>>>>>>
>>>>>> Hi Heinrich,
>>>>>>
>>>>>> On Mon, 24 Mar 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>>>>
>>>>>>> On 17.03.25 14:38, Ilias Apalodimas wrote:
>>>>>>>
>>>>>>> %s/EFI_BOUNCE_BUFFER/CONFIG_EFI_LOADER_BOUNCE_BUFFER/
>>>>>>>
>>>>>>>> The EFI subsystem defines its own bounce buffer for devices that
>>>>>>>> can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
>>>>>>>> which can be reused instead of defining another symbol.
>>>>>>>> The only limitation for EFI is that we don't know how big the file a user
>>>>>>>> chooses to transfer is and as a result we can't depend on allocating the
>>>>>>>> memory from the malloc area, which can prove too small.
>>>>>>>>
>>>>>>>> So allocate an EFI buffer of the correct size and pass it to the DM,
>>>>>>>> which already supports bounce buffering via bounce_buffer_start_extalign()
>>>>>>>
>>>>>>> Looking at
>>>>>>>
>>>>>>>       if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && desc->bb) {
>>>>>>>
>>>>>>> in drivers/block/blk-uclass.c the bounce buffer has to be explicitly
>>>>>>> enabled by the device driver. Only the scsi drivers sets bb = true.
>>>>>>>
>>>>>>> Cf. 81bd22e935dc ("rockchip: block: blk-uclass: add bounce buffer flag
>>>>>>> to blk_desc")
>>>>>>>
>>>>>>> Which device-drivers of the boards mentioned below do actually need
>>>>>>> bounce buffering?
>>>>>>
>>>>>> Unfortunately, I don't have any of the hardware to test and I havent
>>>>>> worked with that platform much.
>>>>>> That 'bb' variable and the fact that EFI needs bigger allocations is
>>>>>> why I ended up allocationg properly aligned memory from the EFI
>>>>>> subsystem. But as Mark pointed out, the cache flush is a no go for
>>>>>> now, so I'll drop this and see if I find time to rework the bounce
>>>>>> buffer logic overall
>>>>>
>>>>> There was quite a bit of discussion about all this in the context of
>>>>> my attempt to just add a message to warn the user[1]
>>>>>
>>>>> We might consider adding an event to reserve memory before relocation,
>>>>> along with a way to discover (in board_r) where the memory was
>>>>> allocated. That would make the solution more generic.
>>>>
>>>> I am not sure what you are trying to solve here. The EFI bounce buffer
>>>> after the LMB patches can't overwrite memory, nor can it be
>>>> overwritten.
>>>
>>> I am thinking of we can create a single implementation of the
>>> bouncebuf logic which also works for EFI.
>>>
>>> I think the two sane things to do are:
>>> - restrict U-Boot to using memory below 4GB for platforms which have
>>> the DMA limitation
>>
>> You don't need that. The bounce buf code has a callback you can use to
>> define the limitations
>>
>>> - create (in board_f) a special region below 4GB for use with the
>>> bouncebuf logic
>>
>> The only problem with EFI is that you don't know how much memory it
>> needs and we can't use the existing memalign calls. So if we replace
>> that memalign in the bounce buffer code, with an lmb reservation we
>> have everything we need.
>
> It's not even an EFI problem is it? You could hit the same problem
> reading a file from a filesystem outside of EFI too. These specific
> SoCs just aren't heavily exercised is one of the challenges I think and
> so it's possible that we have a few things to yet improve in the
> bounce buffer code (which was added for other SoCs and done as generic
> enough starting point for others).
>

EFI or LMB allocate memory top down. So EFI applications have a good
chance of loading files into high memory. Other file-system operations
typically use predefined addresses line $loadaddr. This is why the
problem became more visible in EFI.

It is evident that the bounce-buffer functionality is not EFI specific
per se.

Best regards

Heinrich

On Fri, Mar 28, 2025 at 05:17:36PM +0100, Heinrich Schuchardt wrote:
> On 28.03.25 17:04, Tom Rini wrote:
> > On Fri, Mar 28, 2025 at 02:26:39PM +0200, Ilias Apalodimas wrote:
> > > On Fri, 28 Mar 2025 at 13:34, Simon Glass <sjg@chromium.org> wrote:
> > > > 
> > > > Hi Ilias,
> > > > 
> > > > On Thu, 27 Mar 2025 at 15:19, Ilias Apalodimas
> > > > <ilias.apalodimas@linaro.org> wrote:
> > > > > 
> > > > > Hi Simon
> > > > > 
> > > > > On Thu, 27 Mar 2025 at 15:33, Simon Glass <sjg@chromium.org> wrote:
> > > > > > 
> > > > > > Hi Ilias,
> > > > > > 
> > > > > > On Wed, 26 Mar 2025 at 02:37, Ilias Apalodimas
> > > > > > <ilias.apalodimas@linaro.org> wrote:
> > > > > > > 
> > > > > > > Hi Heinrich,
> > > > > > > 
> > > > > > > On Mon, 24 Mar 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > > > > > 
> > > > > > > > On 17.03.25 14:38, Ilias Apalodimas wrote:
> > > > > > > > 
> > > > > > > > %s/EFI_BOUNCE_BUFFER/CONFIG_EFI_LOADER_BOUNCE_BUFFER/
> > > > > > > > 
> > > > > > > > > The EFI subsystem defines its own bounce buffer for devices that
> > > > > > > > > can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> > > > > > > > > which can be reused instead of defining another symbol.
> > > > > > > > > The only limitation for EFI is that we don't know how big the file a user
> > > > > > > > > chooses to transfer is and as a result we can't depend on allocating the
> > > > > > > > > memory from the malloc area, which can prove too small.
> > > > > > > > > 
> > > > > > > > > So allocate an EFI buffer of the correct size and pass it to the DM,
> > > > > > > > > which already supports bounce buffering via bounce_buffer_start_extalign()
> > > > > > > > 
> > > > > > > > Looking at
> > > > > > > > 
> > > > > > > >       if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && desc->bb) {
> > > > > > > > 
> > > > > > > > in drivers/block/blk-uclass.c the bounce buffer has to be explicitly
> > > > > > > > enabled by the device driver. Only the scsi drivers sets bb = true.
> > > > > > > > 
> > > > > > > > Cf. 81bd22e935dc ("rockchip: block: blk-uclass: add bounce buffer flag
> > > > > > > > to blk_desc")
> > > > > > > > 
> > > > > > > > Which device-drivers of the boards mentioned below do actually need
> > > > > > > > bounce buffering?
> > > > > > > 
> > > > > > > Unfortunately, I don't have any of the hardware to test and I havent
> > > > > > > worked with that platform much.
> > > > > > > That 'bb' variable and the fact that EFI needs bigger allocations is
> > > > > > > why I ended up allocationg properly aligned memory from the EFI
> > > > > > > subsystem. But as Mark pointed out, the cache flush is a no go for
> > > > > > > now, so I'll drop this and see if I find time to rework the bounce
> > > > > > > buffer logic overall
> > > > > > 
> > > > > > There was quite a bit of discussion about all this in the context of
> > > > > > my attempt to just add a message to warn the user[1]
> > > > > > 
> > > > > > We might consider adding an event to reserve memory before relocation,
> > > > > > along with a way to discover (in board_r) where the memory was
> > > > > > allocated. That would make the solution more generic.
> > > > > 
> > > > > I am not sure what you are trying to solve here. The EFI bounce buffer
> > > > > after the LMB patches can't overwrite memory, nor can it be
> > > > > overwritten.
> > > > 
> > > > I am thinking of we can create a single implementation of the
> > > > bouncebuf logic which also works for EFI.
> > > > 
> > > > I think the two sane things to do are:
> > > > - restrict U-Boot to using memory below 4GB for platforms which have
> > > > the DMA limitation
> > > 
> > > You don't need that. The bounce buf code has a callback you can use to
> > > define the limitations
> > > 
> > > > - create (in board_f) a special region below 4GB for use with the
> > > > bouncebuf logic
> > > 
> > > The only problem with EFI is that you don't know how much memory it
> > > needs and we can't use the existing memalign calls. So if we replace
> > > that memalign in the bounce buffer code, with an lmb reservation we
> > > have everything we need.
> > 
> > It's not even an EFI problem is it? You could hit the same problem
> > reading a file from a filesystem outside of EFI too. These specific
> > SoCs just aren't heavily exercised is one of the challenges I think and
> > so it's possible that we have a few things to yet improve in the
> > bounce buffer code (which was added for other SoCs and done as generic
> > enough starting point for others).
> > 
> 
> EFI or LMB allocate memory top down. So EFI applications have a good
> chance of loading files into high memory. Other file-system operations
> typically use predefined addresses line $loadaddr. This is why the
> problem became more visible in EFI.
> 
> It is evident that the bounce-buffer functionality is not EFI specific
> per se.

Right. But it's not the destination address we're talking about but
rather that for "fun" system design reasons we need to have the storage
IP block read from device to one address and then bounce it over to the
final address. Or have I missed understood which kind of bounce buffer
we're needing something here for?

> Date: Fri, 28 Mar 2025 10:04:19 -0600
> From: Tom Rini <trini@konsulko.com>
> 
> On Fri, Mar 28, 2025 at 02:26:39PM +0200, Ilias Apalodimas wrote:
> > On Fri, 28 Mar 2025 at 13:34, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Ilias,
> > >
> > > On Thu, 27 Mar 2025 at 15:19, Ilias Apalodimas
> > > <ilias.apalodimas@linaro.org> wrote:
> > > >
> > > > Hi Simon
> > > >
> > > > On Thu, 27 Mar 2025 at 15:33, Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Ilias,
> > > > >
> > > > > On Wed, 26 Mar 2025 at 02:37, Ilias Apalodimas
> > > > > <ilias.apalodimas@linaro.org> wrote:
> > > > > >
> > > > > > Hi Heinrich,
> > > > > >
> > > > > > On Mon, 24 Mar 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > > > >
> > > > > > > On 17.03.25 14:38, Ilias Apalodimas wrote:
> > > > > > >
> > > > > > > %s/EFI_BOUNCE_BUFFER/CONFIG_EFI_LOADER_BOUNCE_BUFFER/
> > > > > > >
> > > > > > > > The EFI subsystem defines its own bounce buffer for devices that
> > > > > > > > can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> > > > > > > > which can be reused instead of defining another symbol.
> > > > > > > > The only limitation for EFI is that we don't know how big the file a user
> > > > > > > > chooses to transfer is and as a result we can't depend on allocating the
> > > > > > > > memory from the malloc area, which can prove too small.
> > > > > > > >
> > > > > > > > So allocate an EFI buffer of the correct size and pass it to the DM,
> > > > > > > > which already supports bounce buffering via bounce_buffer_start_extalign()
> > > > > > >
> > > > > > > Looking at
> > > > > > >
> > > > > > >      if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && desc->bb) {
> > > > > > >
> > > > > > > in drivers/block/blk-uclass.c the bounce buffer has to be explicitly
> > > > > > > enabled by the device driver. Only the scsi drivers sets bb = true.
> > > > > > >
> > > > > > > Cf. 81bd22e935dc ("rockchip: block: blk-uclass: add bounce buffer flag
> > > > > > > to blk_desc")
> > > > > > >
> > > > > > > Which device-drivers of the boards mentioned below do actually need
> > > > > > > bounce buffering?
> > > > > >
> > > > > > Unfortunately, I don't have any of the hardware to test and I havent
> > > > > > worked with that platform much.
> > > > > > That 'bb' variable and the fact that EFI needs bigger allocations is
> > > > > > why I ended up allocationg properly aligned memory from the EFI
> > > > > > subsystem. But as Mark pointed out, the cache flush is a no go for
> > > > > > now, so I'll drop this and see if I find time to rework the bounce
> > > > > > buffer logic overall
> > > > >
> > > > > There was quite a bit of discussion about all this in the context of
> > > > > my attempt to just add a message to warn the user[1]
> > > > >
> > > > > We might consider adding an event to reserve memory before relocation,
> > > > > along with a way to discover (in board_r) where the memory was
> > > > > allocated. That would make the solution more generic.
> > > >
> > > > I am not sure what you are trying to solve here. The EFI bounce buffer
> > > > after the LMB patches can't overwrite memory, nor can it be
> > > > overwritten.
> > >
> > > I am thinking of we can create a single implementation of the
> > > bouncebuf logic which also works for EFI.
> > >
> > > I think the two sane things to do are:
> > > - restrict U-Boot to using memory below 4GB for platforms which have
> > > the DMA limitation
> > 
> > You don't need that. The bounce buf code has a callback you can use to
> > define the limitations
> > 
> > > - create (in board_f) a special region below 4GB for use with the
> > > bouncebuf logic
> > 
> > The only problem with EFI is that you don't know how much memory it
> > needs and we can't use the existing memalign calls. So if we replace
> > that memalign in the bounce buffer code, with an lmb reservation we
> > have everything we need.
> 
> It's not even an EFI problem is it?

Fundamentall, yes, this isn't an EFI problem.

> You could hit the same problem reading a file from a filesystem
> outside of EFI too.

Yes, but we tend to choose the addresses in the env variables that are
used in the more traditional boot methods to prevent this.

> These specific SoCs just aren't heavily exercised is one of the
> challenges I think and so it's possible that we have a few things to
> yet improve in the bounce buffer code (which was added for other
> SoCs and done as generic enough starting point for others).

The existing bounce buffer code was written to solve a completely
different problem.  But it could indeed be generalized to solve this
problem as well.  That requires somebody willing to work on a larger
set of actual hardware that includes SoC with cache-coherency
challanges (which is what the current bounce buffer implementation is
there for) and SoCs with DMA addressing challenges.

Thanks all this is helpful

On Fri, 28 Mar 2025 at 19:15, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>
> > Date: Fri, 28 Mar 2025 10:04:19 -0600
> > From: Tom Rini <trini@konsulko.com>
> >
> > On Fri, Mar 28, 2025 at 02:26:39PM +0200, Ilias Apalodimas wrote:
> > > On Fri, 28 Mar 2025 at 13:34, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Ilias,
> > > >
> > > > On Thu, 27 Mar 2025 at 15:19, Ilias Apalodimas
> > > > <ilias.apalodimas@linaro.org> wrote:
> > > > >
> > > > > Hi Simon
> > > > >
> > > > > On Thu, 27 Mar 2025 at 15:33, Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Ilias,
> > > > > >
> > > > > > On Wed, 26 Mar 2025 at 02:37, Ilias Apalodimas
> > > > > > <ilias.apalodimas@linaro.org> wrote:
> > > > > > >
> > > > > > > Hi Heinrich,
> > > > > > >
> > > > > > > On Mon, 24 Mar 2025 at 19:50, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > > > > >
> > > > > > > > On 17.03.25 14:38, Ilias Apalodimas wrote:
> > > > > > > >
> > > > > > > > %s/EFI_BOUNCE_BUFFER/CONFIG_EFI_LOADER_BOUNCE_BUFFER/
> > > > > > > >
> > > > > > > > > The EFI subsystem defines its own bounce buffer for devices that
> > > > > > > > > can't transfer data > 4GB. U-Boot already has a generic BOUNCE_BUFFER
> > > > > > > > > which can be reused instead of defining another symbol.
> > > > > > > > > The only limitation for EFI is that we don't know how big the file a user
> > > > > > > > > chooses to transfer is and as a result we can't depend on allocating the
> > > > > > > > > memory from the malloc area, which can prove too small.
> > > > > > > > >
> > > > > > > > > So allocate an EFI buffer of the correct size and pass it to the DM,
> > > > > > > > > which already supports bounce buffering via bounce_buffer_start_extalign()
> > > > > > > >
> > > > > > > > Looking at
> > > > > > > >
> > > > > > > >      if (IS_ENABLED(CONFIG_BOUNCE_BUFFER) && desc->bb) {
> > > > > > > >
> > > > > > > > in drivers/block/blk-uclass.c the bounce buffer has to be explicitly
> > > > > > > > enabled by the device driver. Only the scsi drivers sets bb = true.
> > > > > > > >
> > > > > > > > Cf. 81bd22e935dc ("rockchip: block: blk-uclass: add bounce buffer flag
> > > > > > > > to blk_desc")
> > > > > > > >
> > > > > > > > Which device-drivers of the boards mentioned below do actually need
> > > > > > > > bounce buffering?
> > > > > > >
> > > > > > > Unfortunately, I don't have any of the hardware to test and I havent
> > > > > > > worked with that platform much.
> > > > > > > That 'bb' variable and the fact that EFI needs bigger allocations is
> > > > > > > why I ended up allocationg properly aligned memory from the EFI
> > > > > > > subsystem. But as Mark pointed out, the cache flush is a no go for
> > > > > > > now, so I'll drop this and see if I find time to rework the bounce
> > > > > > > buffer logic overall
> > > > > >
> > > > > > There was quite a bit of discussion about all this in the context of
> > > > > > my attempt to just add a message to warn the user[1]
> > > > > >
> > > > > > We might consider adding an event to reserve memory before relocation,
> > > > > > along with a way to discover (in board_r) where the memory was
> > > > > > allocated. That would make the solution more generic.
> > > > >
> > > > > I am not sure what you are trying to solve here. The EFI bounce buffer
> > > > > after the LMB patches can't overwrite memory, nor can it be
> > > > > overwritten.
> > > >
> > > > I am thinking of we can create a single implementation of the
> > > > bouncebuf logic which also works for EFI.
> > > >
> > > > I think the two sane things to do are:
> > > > - restrict U-Boot to using memory below 4GB for platforms which have
> > > > the DMA limitation
> > >
> > > You don't need that. The bounce buf code has a callback you can use to
> > > define the limitations
> > >
> > > > - create (in board_f) a special region below 4GB for use with the
> > > > bouncebuf logic
> > >
> > > The only problem with EFI is that you don't know how much memory it
> > > needs and we can't use the existing memalign calls. So if we replace
> > > that memalign in the bounce buffer code, with an lmb reservation we
> > > have everything we need.
> >
> > It's not even an EFI problem is it?
>
> Fundamentall, yes, this isn't an EFI problem.

+1 it's not. As Heinrich explained EFI just makes it a lot easier to trigger

>
> > You could hit the same problem reading a file from a filesystem
> > outside of EFI too.
>
> Yes, but we tend to choose the addresses in the env variables that are
> used in the more traditional boot methods to prevent this.
>
> > These specific SoCs just aren't heavily exercised is one of the
> > challenges I think and so it's possible that we have a few things to
> > yet improve in the bounce buffer code (which was added for other
> > SoCs and done as generic enough starting point for others).
>
> The existing bounce buffer code was written to solve a completely
> different problem.  But it could indeed be generalized to solve this
> problem as well.  That requires somebody willing to work on a larger
> set of actual hardware that includes SoC with cache-coherency
> challanges (which is what the current bounce buffer implementation is
> there for) and SoCs with DMA addressing challenges.

So, I think if we
- Use LMB instead of memalign for the buffer allocation
- Add an explicit flag on the API to cache flush or not. We could add
this to the existing DM code
- Revisit the 'bb' variable usage and set it correctly on
drivers/platforms that need the bounce buffer

We should be covered, no?

Thanks
/Ilias