Re: [PATCH v2 8/9] ARM: rockchip: pass device tree to TF-A

[Sascha Hauer <s.hauer@pengutronix.de>]

On Wed, May 28, 2025 at 04:11:26PM +0200, Michael Tretter wrote:
> The upstream OP-TEE expects a device tree to be able to initialize the
> dynamic shared memory. Therefore, barebox should pass a device tree that
> contains memory nodes through the TF-A to OP-TEE.
> 
> OP-TEE may modify the passed fdt. Thus, barebox copies the fdt from the
> fixed size rodata area to the rk_scratch area and allocates a
> configurable memory area for the fdt to allow fdt modification.
> 
> OP-TEE has the CFG_DTB_MAX_SIZE config option for the maximum size of
> the fdt. barebox must reserve at least that much memory for the fdt to
> avoid out of bounds accesses from OP-TEE.
> 
> Unfortunately, the downstream TF-A fails to start if barebox passes its
> device tree and it is not possible to detect if the loaded TF-A is able
> to handle the device tree. Add a config option to pass the device tree
> if it is possible.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> Changes in v2:
> - pass copy of fdt in scratch space to TF-A
> - add config option for fdt size
> - add documentation for CONFIG_ARCH_ROCKCHIP_ATF_PASS_FDT
> ---
>  Documentation/boards/rockchip.rst |  4 ++++
>  arch/arm/mach-rockchip/Kconfig    | 23 +++++++++++++++++++++++
>  arch/arm/mach-rockchip/atf.c      | 24 ++++++++++++------------
>  include/mach/rockchip/bootrom.h   |  3 +++
>  4 files changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/boards/rockchip.rst b/Documentation/boards/rockchip.rst
> index b2b04abb03cd1926bb59799af5f6a8c11d410cc2..8bce92865e3eda193412180c7295b5a35b3531e8 100644
> --- a/Documentation/boards/rockchip.rst
> +++ b/Documentation/boards/rockchip.rst
> @@ -94,6 +94,10 @@ With these barebox can be compiled as:
>  .. note:: The RK3566 and RK3568 seem to share the bl31 and bl32 firmware files,
>    whereas the memory initialization blob is different.
>  
> +.. note:: The bl31 from the rkbin repository seems to be unable to handle
> +  device trees of a larger size (for example, if CONFIG_OF_OVERLAY_LIVE is
> +  enabled). Disable CONFIG_ARCH_ROCKCHIP_ATF_PASS_FDT in this case.
> +
>  Creating a bootable SD card
>  ---------------------------
>  
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index 98dfd11c182b9fee6e3c958653ad4fa8a7d98d84..1b72dc4b8a16ce9dca742ff1173fc2208a0e619d 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -141,6 +141,29 @@ config ARCH_ROCKCHIP_ATF
>  	  useful for debugging early startup, but for all other cases,
>  	  say y here.
>  
> +config ARCH_ROCKCHIP_ATF_PASS_FDT
> +	bool "Pass device tree to TF-A"
> +	depends on ARCH_ROCKCHIP_ATF
> +	help
> +	  Enable this option if you are using an upstream OP-TEE that uses the
> +	  device tree to initialize dynamic shared memory, which is passed
> +	  through the upstream TF-A.
> +
> +	  Disable the option if you are using a downstream TF-A since it
> +	  doesn't always cope with device trees. Supposedly this happens if the
> +	  device tree is too large, for example if CONFIG_OF_OVERLAY_LIVE is
> +	  enabled.
> +
> +config ARCH_ROCKCHIP_ATF_FDT_SIZE
> +	hex
> +	default 0x0
> +	default 0x60000 if ARCH_ROCKCHIP_ATF_PASS_FDT
> +	prompt "Reserved size for the FDT blob passed to the TF-A"
> +	help
> +	  Set this size to CFG_DTB_MAX_SIZE in the OP-TEE configuration. OP-TEE
> +	  may modify the passed device tree and increase it's size. This
> +	  ensures that barebox reserves enough memory for the modifications.
> +
>  config ARCH_ROCKCHIP_OPTEE
>  	bool "Build rockchip OP-TEE binary into barebox"
>  	depends on ARCH_ROCKCHIP_ATF
> diff --git a/arch/arm/mach-rockchip/atf.c b/arch/arm/mach-rockchip/atf.c
> index cfa6df043b34c0f36919048237c7ecf33dfe0724..f4a71ef2dc8dffc6ceb4f97ee542f5b83858120b 100644
> --- a/arch/arm/mach-rockchip/atf.c
> +++ b/arch/arm/mach-rockchip/atf.c
> @@ -183,21 +183,21 @@ void __noreturn rk3588_barebox_entry(void *fdt)
>  	rk_scratch = (void *)arm_mem_scratch(endmem);
>  
>  	if (current_el() == 3) {
> +		void *fdt_scratch = NULL;
> +
>  		rk3588_lowlevel_init();
>  		rockchip_store_bootrom_iram(IOMEM(RK3588_IRAM_BASE));
>  
> -		/*
> -		 * The downstream TF-A doesn't cope with our device tree when
> -		 * CONFIG_OF_OVERLAY_LIVE is enabled, supposedly because it is
> -		 * too big for some reason. Otherwise it doesn't have any visible
> -		 * effect if we pass a device tree or not, except that the TF-A
> -		 * fills in the ethernet MAC address into the device tree.
> -		 * The upstream TF-A doesn't use the device tree at all.
> -		 *
> -		 * Pass NULL for now until we have a good reason to pass a real
> -		 * device tree.
> -		 */
> -		rk3588_atf_load_bl31(NULL);
> +#ifdef CONFIG_ARCH_ROCKCHIP_ATF_PASS_FDT
> +		pr_debug("Copy fdt to scratch area 0x%p (%zu bytes)\n",
> +			 rk_scratch->fdt, sizeof(rk_scratch->fdt));
> +		if (fdt_open_into(fdt, rk_scratch->fdt, sizeof(rk_scratch->fdt)) == 0)
> +			fdt_scratch = rk_scratch->fdt;
> +		else
> +			pr_warn("Failed to copy fdt to scratch: Continue without fdt\n");
> +#endif

I converted this to if (IS_ENABLED(CONFIG_ARCH_ROCKCHIP_ATF_PASS_FDT))
while applying.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |