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

Hi Simon,

On Tue, Aug 4, 2026 at 5:02 PM Simon Glass <[email protected]> wrote:
>
> 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.

Will do, thanks.

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

Nope, I went through both the mainline U-Boot codebase and TF-A -
nothing expects MPID there. TF-A stores whatever it received and puts
it back before handing over to the next stage. In U-Boot proper it's
just saved in save_boot_params, and can be claimed by either
CONFIG_BLOBLIST (which expects it to be an FDT pointer, not MPIDR) or
SAVE_PREV_BL_FDT_ADDR (same).

Per-platform overrides never mention MPIDR either - only DTB pointers.

The comment and the code was added in the initial commit bcc1726a7bdd
("spl: add support to booting with ATF") by Kever Yang back in 2017
and was never really propagated anywhere in the tree.

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

Will do, thanks.

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

Indeed, will add an error path for the zero case here.

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

Hmm, adding a future board but relying on a default which will have
predated the addition of the said board sounds weird, doesn't it? :)

I'm not sure I see a clean way to route around BOOTI from within
spl_invoke_atf(), given that BOOTI just jumps to the kernel image
without starting the firmware first. It might kind-of-boot, but it
will result in degraded functionality, so I thought removing a footgun
from being enabled by default made more sense. It's not needed in the
FIT (TF-A+Linux) flow either, so removing a "default y" from Kconfig
is a one-liner which saves [num_boards] lines of defconfig noise
disabling it in every board individually.

Best regards,
Alexey