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

Marek Vasut
2025-01-29T17:03:59Z

On 1/29/25 5:28 PM, Quentin Schulz wrote:
>
>
> On 1/29/25 5:02 PM, Quentin Schulz wrote:
>> 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://eur02.safelinks.protection.outlook.com/?
>>> url=https%3A%2F%2Fgithub.com%2Frenesas-rcar%2Farm-trusted-
>>> firmware.git&data=05%7C02%7Cquentin.schulz%40cherry.de%7C6ab2eb623e32425379f108dd407e5a7c%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C638737633628498421%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=DWQfUm5mp9%2FMCUxvFFbO1yfjKlvmtfiOD8VTkN%2BEZKk%3D&reserved=0
>>> 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.
>>
>
> Actually, I would highly prefer we don't even have this symbol and have
> instead a property in the TF-A image node in the ITS to say whether
> we're allowed to pass parameters or not to BL31? The user bundling TF-A
> inside the linux.itb should know (well, we should probably know also
> when building BL31 within u-boot.itb....).

This is something user should decide in their own board/arch specific
implementation of tfa_bl31_image_process()/armv8_switch_to_el2_prep().
These two functions are architecture/board specific because of the
various forks of TFA BL31 and the need to handle those various handoff
structures.

> We actually need this symbol on all Rockchip boards using the Rockchip
> blob and not upstream. But this also prevents us from using some
> featureset of the upstream TF-A if we rebuild without disabling the
> symbol. It'd be nice if we don't have to rebuild U-Boot depending on
> which variant of BL31 we use on Rockchip, this info should be stored in
> the linux.itb is what I'm saying. This doesn't need to be in this
> introducing commit though I believe, we can work something out later I
> guess.
Maybe this comment is not really applicable ?