Re: [PATCH 3/3] arm64: renesas: Add TFA BL31 handoff support

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Marek,

On 1/12/25 11:36 PM, Marek Vasut wrote:
> Implement custom U_BOOT_FIT_LOADABLE_HANDLER and armv8_switch_to_el2_prep()
> handling in case the TFA was loaded. The loadables handler sets up custom
> handoff structure used by Renesas TFA fork in fixed location in DRAM and
> indicates the TFA has been loaded.
> 
> The custom armv8_switch_to_el2_prep() handling tests whether the TFA BL31
> was previously loaded and the custom handoff structure was set up, and if
> so, jumps to TFA BL31 which switches from EL3 to EL2 and then returns to
> U-Boot just past bl in armv8_switch_to_el2() to finish starting the Linux
> kernel.
> 
> The jump to Linux through TFA works in such a way that the custom
> armv8_switch_to_el2_prep() handler configures the custom handoff structure
> such that the target jump address of the TFA BL31 on exit is set to the
> armv8_switch_to_el2() + 4, which is just past the bl, and just before the
> U-Boot code which implements the Linux kernel boot from either EL. The
> registers passed through the TFA BL31 are all the registers passed into
> armv8_switch_to_el2_prep() to assure maximum compatibility with all the
> boot modes. The armv8_switch_to_el2_prep() handler jumps to the TFA BL31,
> which does its setup, drops EL from EL3 to EL2 and finally jumps to the
> armv8_switch_to_el2() + 4 entry point, which then allows U-Boot to boot
> the Linux kernel the usual way.
> 
> In order to build suitable kernel fitImage, build TFA first, temporarily
> from downstream repository:
> remote: https://github.com/renesas-rcar/arm-trusted-firmware.git
> branch: rcar_gen4_v2.7_v4x
> 
> ```
> $ git clean -fqdx
> $ MBEDTLS_DIR=/path/to/mbedtls/ make -j$(nproc) bl31 \
> 	PLAT=rcar_gen4 ARCH=aarch64 LSI=V4H SPD=none \
> 	CTX_INCLUDE_AARCH32_REGS=0 MBEDTLS_COMMON_MK=1 \
> 	PTP_NONSECURE_ACCESS=1 LOG_LEVEL=20 DEBUG=0 \
> 	ENABLE_ASSERTIONS=0 E=0
> ```
> 
> Build Linux kernel Image and device tree from current mainline Linux
> kernel repository, obtain 'Image' and 'r8a779g0-white-hawk.dtb' .
> 
> Bundle the files together using provided fit-image.its fitImage description:
> ```
> $ mkimage -f fit-image.its fitImage
> ```
> 
> To start the kernel fiImage generated in previous step, load fitImage
> to DRAM and use the 'bootm' command to start it:
> => load 0x58000000 ... fitImage && bootm 0x58000000
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Andre Przywara <andre.przywara@arm.com>
> Cc: Caleb Connolly <caleb.connolly@linaro.org>
> Cc: Igor Opaniuk <igor.opaniuk@gmail.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Julien Masson <jmasson@baylibre.com>
> Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Cc: Maxim Moskalets <maximmosk4@gmail.com>
> Cc: Michael Walle <mwalle@kernel.org>
> Cc: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
> Cc: Patrick Rudolph <patrick.rudolph@9elements.com>
> Cc: Paul Barker <paul.barker.ct@bp.renesas.com>
> Cc: Paul-Erwan Rio <paulerwan.rio@gmail.com>
> Cc: Peter Hoyes <Peter.Hoyes@arm.com>
> Cc: Raymond Mao <raymond.mao@linaro.org>
> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: u-boot@lists.denx.de
> ---
>   board/renesas/common/gen4-common.c | 118 +++++++++++++++++++++++++++++
>   1 file changed, 118 insertions(+)
> 
> diff --git a/board/renesas/common/gen4-common.c b/board/renesas/common/gen4-common.c
> index 52a0639073b..c7f3f9a30ab 100644
> --- a/board/renesas/common/gen4-common.c
> +++ b/board/renesas/common/gen4-common.c
> @@ -7,11 +7,13 @@
>   
>   #include <asm/arch/renesas.h>
>   #include <asm/arch/sys_proto.h>
> +#include <asm/armv8/mmu.h>
>   #include <asm/global_data.h>
>   #include <asm/io.h>
>   #include <asm/mach-types.h>
>   #include <asm/processor.h>
>   #include <asm/system.h>
> +#include <image.h>
>   #include <linux/errno.h>
>   
>   #define RST_BASE	0xE6160000 /* Domain0 */
> @@ -88,3 +90,119 @@ int ft_board_setup(void *blob, struct bd_info *bd)
>   {
>   	return 0;
>   }
> +
> +/* R-Car Gen4 TFA BL31 handoff structure and handling. */
> +struct param_header {
> +	u8			type;
> +	u8			version;
> +	u16			size;
> +	u32			attr;
> +};
> +
> +struct tfa_image_info {
> +	struct param_header	h;
> +	uintptr_t		image_base;
> +	u32			image_size;
> +	u32			image_max_size;
> +};
> +
> +struct aapcs64_params {
> +	u64			arg0;
> +	u64			arg1;
> +	u64			arg2;
> +	u64			arg3;
> +	u64			arg4;
> +	u64			arg5;
> +	u64			arg6;
> +	u64			arg7;
> +};
> +
> +struct entry_point_info {
> +	struct param_header	h;
> +	uintptr_t		pc;
> +	u32			spsr;
> +	struct aapcs64_params	args;
> +};
> +
> +struct bl2_to_bl31_params_mem {
> +	struct tfa_image_info	bl32_image_info;
> +	struct tfa_image_info	bl33_image_info;
> +	struct entry_point_info	bl33_ep_info;
> +	struct entry_point_info	bl32_ep_info;
> +};
> +
> +/* Default jump address, return to U-Boot */
> +#define BL33_BASE	0x44100000
> +/* Custom parameters address passed to TFA by ICUMXA loader */
> +#define PARAMS_BASE	0x46422200
> +

Shouldn't we rather set those as offset compared to the load/entry 
address passed in the ITS?

> +/* Usually such a structure is produced by ICUMXA and passed in at 0x46422200 */
> +static const struct bl2_to_bl31_params_mem blinfo_template = {
> +	.bl33_ep_info.h.type = 1,	/* PARAM_EP */
> +	.bl33_ep_info.h.version = 2,	/* Version 2 */
> +	.bl33_ep_info.h.size = sizeof(struct entry_point_info),
> +	.bl33_ep_info.h.attr = 0x81,	/* Executable | Non-Secure */
> +	.bl33_ep_info.spsr = 0x2c9,	/* Mode=EL2, SP=ELX, Exceptions=OFF */
> +	.bl33_ep_info.pc = BL33_BASE,
> +
> +	.bl33_image_info.h.type = 1,	/* PARAM_EP */
> +	.bl33_image_info.h.version = 2,	/* Version 2 */
> +	.bl33_image_info.h.size = sizeof(struct image_info),
> +	.bl33_image_info.h.attr = 0,
> +	.bl33_image_info.image_base = BL33_BASE,
> +};
> +
> +static bool tfa_bl31_image_loaded;
> +static ulong tfa_bl31_image_addr;
> +
> +static void tfa_bl31_image_process(ulong image, size_t size)
> +{
> +	/* Custom parameters address passed to TFA by ICUMXA loader */
> +	struct bl2_to_bl31_params_mem *blinfo = (struct bl2_to_bl31_params_mem *)PARAMS_BASE;
> +
> +	/* Clear a page and copy template */
> +	memset((void *)PARAMS_BASE, 0, PAGE_SIZE);
> +	memcpy(blinfo, &blinfo_template, sizeof(*blinfo));
> +	tfa_bl31_image_addr = image;
> +	tfa_bl31_image_loaded = true;
> +}
> +
> +U_BOOT_FIT_LOADABLE_HANDLER(IH_TYPE_TFA_BL31, tfa_bl31_image_process);
> +
> +void armv8_switch_to_el2_prep(u64 args, u64 mach_nr, u64 fdt_addr,
> +			      u64 arg4, u64 entry_point, u64 es_flag)
> +{
> +	typedef void __noreturn (*image_entry_noargs_t)(void);
> +	image_entry_noargs_t image_entry =
> +		(image_entry_noargs_t)(void *)tfa_bl31_image_addr;
> +	struct bl2_to_bl31_params_mem *blinfo =
> +		(struct bl2_to_bl31_params_mem *)PARAMS_BASE;
> +
> +	/*
> +	 * Destination address in arch/arm/cpu/armv8/transition.S
> +	 * right past the first bl in armv8_switch_to_el2() to let
> +	 * the rest of U-Boot pre-Linux code run. The code does run
> +	 * without stack pointer!
> +	 */
> +	const u64 ep = ((u64)(uintptr_t)&armv8_switch_to_el2) + 4;
> +
> +	/* If TFA BL31 was not part of the fitImage, do regular boot. */
> +	if (!tfa_bl31_image_loaded)
> +		return;
> +
> +	/*
> +	 * Set up kernel entry point and parameters:
> +	 * x0 is FDT address, x1..x3 must be 0
> +	 */
> +	blinfo->bl33_ep_info.pc = ep;
> +	blinfo->bl33_ep_info.args.arg0 = args;
> +	blinfo->bl33_ep_info.args.arg1 = mach_nr;
> +	blinfo->bl33_ep_info.args.arg2 = fdt_addr;
> +	blinfo->bl33_ep_info.args.arg3 = arg4;
> +	blinfo->bl33_ep_info.args.arg4 = entry_point;
> +	blinfo->bl33_ep_info.args.arg5 = es_flag;
> +	blinfo->bl33_image_info.image_base = ep;
> +
> +	/* Jump to TFA BL31 */
> +	image_entry();
> +}

Shouldn't we have a weak implementation that SoC vendor can override if 
they feel like it?

To me this feels like it could be much quicker adopted if we had some 
default people could try out for their boards.

We already have a generic spl_invoke_atf() so why not the same for this 
mechanism? As far as I could tell, we get the load address of TEE and 
U-Boot proper (bl32 and bl33) and pass it the FDT (or nothing if 
SPL_ATF_NO_PLATFORM_PARAM symbol set).

I am not too sure who and how TEE will be started as it needs to be 
executing in EL3 too I believe? So we may still have to provide that 
one? BL33 would be the kernel params like you did in this patch. Then 
the bl31 param could also be either the fdt or nothing if 
ATF_NO_PLATFORM_PARAM is set.

I guess the issue is that Renesas doesn't use any of that generic stuff?

What do you think?

Cheers,
Quentin