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

Marek Vasut
2025-01-29T17:01:00Z

On 1/29/25 5:02 PM, Quentin Schulz wrote:
> Hi Marek,

Hi,

[...]

>> +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?

We do have one. Please do have a look at 1/3 in this series:

WEAK(armv8_switch_to_el2_prep)

> 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).

Looking at spl_invoke_atf(), it _almost_ looks like something I could
use, but from what I see there and here, the handoff structure layout is
slightly different, it seems to be board specific.

But that is fine, as this is a board specific strong implementation of
weak armv8_switch_to_el2_prep function.

> 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?

The board does ship with a custom loader which starts on a security
core, which starts the BL31 on an application core. That custom loader
does set up the handoff structure before releasing the application core
from reset, and then the application core jumps right into the BL31 .

But mainline U-Boot now has SPL which can run on another realtime core,
bring up DRAM, application core, start U-Boot on it ... and then U-Boot
has to set up the handoff structure before starting TFA, in the same
form like the custom loader.

--
Best regards,
Marek Vasut