Re: [PATCH 2/2] ARM: optee-early: invalidate caches before jump to OP-TEE

[Rouven Czerwinski <rouven.czerwinski@linaro.org>]

Hi,

On Tue, 3 Jun 2025 at 17:20, Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Am Dienstag, dem 03.06.2025 um 16:51 +0200 schrieb Ahmad Fatoum:
> [...]
> > > > > I guess it would be much better to simply have the
> > > > > arm_early_mmu_cache_invalidate() as part of the Cortex A9 lowlevel CPU
> > > > > initialization at the very start of the PBL entry.

To add my few cents:
I have had customer systems where without cache invalidation,
imx-usb-loader would
sporadically hang. This hang was fixed by calling
arm_early_mmu_cache_invalidate()
in the lowlevel pbl code as well. Barebox already has i.MX6 boards
which to call the
invalidation early in PBL, but unfortunately none have a comment or
commit description
why.

> > > >
> > > > We don't have a dedicated Cortex-A9 lowlevel entry function
> > > > unfortunately, just some for specific processors, e.g. the
> > > > imx6_cpu_lowlevel_init.
> > > >
> > > > We could add CONFIG_CPU_CORTEX_A9, select it from the relevant SoC
> > > > options and depending on it, add the invalidation to
> > > > arm_cpu_lowlevel_init()? What do you think?
> > > >
> > > This would then trigger the invalidation even on systems that don't
> > > need it in case of a multiarch Barebox. There aren't that many Cortex
> > > A9 based SoCs supported in Barebox and all of them should have a SoC
> > > specific init function to apply the necessary workarounds, so I think
> > > it would be fine to call the cache invalidate from the SoC specific
> > > lowlevel init of those few SoCs?
> >
> > Fair enough. How do we know we only need this for Cortex-A9 though?
> > Couldn't e.g. the Cortex-A8 also be affected?
>
> We can't be 100% sure without specific knowledge about each SoC
> integration. Both the Cortex A8 [1] and Cortex A15 [2] TRMs define a
> reset sequence that mandates the straps to be set in such a way that
> the processor will clear all L1 and L2 memory arrays on power-on reset.
>
> The only odd one where the TRM doesn't even mention memory arrays in
> the reset sequence is the Cortex A9 [3], which pretty much lines up
> with the number of SoCs where we have seen issues due to uninitialized
> cache content.
>
> Regards,
> Lucas
>
> [1] https://developer.arm.com/documentation/ddi0344/k/Cihcbcgi
> [2] https://developer.arm.com/documentation/ddi0438/i/functional-description/clocking-and-resets/resets
> [3] https://developer.arm.com/documentation/100511/0401/functional-description/clocking-and-resets/reset

Adding this to imx6_cpu_lowlevel_init() is IMO the correct way as well.

- Rouven