Re: [PATCH v2 02/10] spl: atf: support Linux as BL33 with TFA

Hi Alexey,

On 2026-07-31T17:10:44, Alexey Charkov <[email protected]> wrote:
> spl: atf: support Linux as BL33 with TFA
>
> Modern SoCs such as Rockchip RK3576 need TFA to be running to provide
> firmware services to the OS.
>
> Enable the TFA boot flow to allow using Linux as BL33 (including its
> calling convention) to facilitate Falcon mode boot on such SoCs.
>
> Signed-off-by: Alexey Charkov <[email protected]>
>
> common/spl/Kconfig   | 22 ++++++++++++++++++++--
>  common/spl/spl_atf.c | 49 +++++++++++++++++++++++++++++++++----------------
>  2 files changed, 53 insertions(+), 18 deletions(-)

>     Enable the TFA boot flow to allow using Linux as BL33 (including its
>     calling convention) to facilitate Falcon mode boot on such SoCs.

The commit message says 'including its calling convention', but the
diff drops read_mpidr() unconditionally - every existing SPL_ATF user
now gets the FDT address in x0 rather than the MPID. Please spell this
out and note why it is safe (i.e. U-Boot proper as BL33 does not
consume x0, and the previous MPID value was a vestige, as you
mentioned in the v2 changelog). Right now that context lives only in
the changelog below the '---', which does not land in git history.

> diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
> @@ -87,8 +88,8 @@ struct bl31_params *bl2_plat_get_bl31_params_default(ulong bl32_entry,
>       SET_PARAM_HEAD(bl33_ep_info, ATF_PARAM_EP, ATF_VERSION_1,
>                      ATF_EP_NON_SECURE);
>
> -     /* BL33 expects to receive the primary CPU MPID (through x0) */
> -     bl33_ep_info->args.arg0 = 0xffff & read_mpidr();
> +     /* Pass the FDT address in x0, per the TF-A BL33 / Linux boot protocol. */
> +     bl33_ep_info->args.arg0 = fdt_addr;

Just to check - for the U-Boot-as-BL33 case where CONFIG_OF_BOARD or
CONFIG_OF_SEPARATE is used and the FDT comes from elsewhere, does
anything in-tree currently rely on x0 being the MPID? A quick look of
the arm64 crt0 suggests not, but worth confirming across the SPL_ATF
users (Rockchip, TI K3, socfpga, meson etc.) rather than assuming.

> diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
> @@ -189,7 +190,8 @@ static inline void raw_write_daif(unsigned int daif)
>  typedef void __noreturn (*atf_entry_t)(struct bl31_params *params, void *plat_params);
>
>  static void __noreturn bl31_entry(ulong bl31_entry, ulong bl32_entry,
> -                               ulong bl33_entry, ulong fdt_addr)
> +                               ulong bl33_entry, ulong fdt_addr,
> +                               ulong plat_param)

The two arguments now mean quite different things - fdt_addr is what
BL33 receives in x0 (and BL32 in arg3), plat_param is what BL31
receives as its second parameter. A short kernel-doc comment on
bl31_entry() spelling that out would help; the split is easy to miss
and the ATF_NO_PLATFORM_PARAM path now diverges the two values
silently.

> diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
> @@ -253,10 +255,11 @@ ulong spl_fit_images_get_entry(void *blob, int node)
>
>  void __noreturn spl_invoke_atf(struct spl_image_info *spl_image)
>  {
> -     ulong  bl32_entry = 0;
> -     ulong  bl33_entry = CONFIG_TEXT_BASE;
> -     void *blob = spl_image->fdt_addr;
> +     bool falcon = CONFIG_IS_ENABLED(OS_BOOT) && spl_falcon_boot();
> +     void *blob = spl_image_fdt_addr(spl_image);

Switching to spl_image_fdt_addr() is a good change, but the helper
returns 0 when neither LOAD_FIT nor LOAD_FIT_FULL is enabled. If
somebody hits spl_invoke_atf() in that configuration we now pass
blob=0 as x0 to BL33 (which for Linux is fatal) rather than the
arbitrary junk the old code passed. Not a regression, but if that
combination is not supposed to reach here at all, a build-time or
runtime guard would be clearer than relying on the caller.

> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> @@ -1230,22 +1230,40 @@ config SPL_BOOTZ
>  config SPL_BOOTI
>       bool "Allow booting an Image style Linux kernel from SPL"
>       depends on SPL_OS_BOOT && !SPL_OS_BOOT_SECURE
> -     default y if ARM64 || RISCV
> +     default y if (ARM64 || RISCV) && !SPL_ATF

Flipping the default based on SPL_ATF will silently disable SPL_BOOTI
on any future board that combines SPL_ATF + SPL_OS_BOOT + ARM64 and
relies on the old default. No in-tree defconfig hits that today, but
it is a bit of a trap. Would it be cleaner to leave the default alone
and rely on the code path in spl_invoke_atf() to route around BOOTI
when TF-A is in play? The help text already explains the option
remains selectable.

Regards,
Simon