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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Lucas,

On 6/3/25 16:47, Lucas Stach wrote:
> Am Dienstag, dem 03.06.2025 um 12:18 +0200 schrieb Ahmad Fatoum:
>> On 6/3/25 11:57, Lucas Stach wrote:
>> Ok, so if CR_C is unset, the cache is still used when reading/writing,
>> provided that the cache line is valid.
>>
> Exactly. Clearing CR_C disables cache allocation, but lookups in the
> cache still proceed as normal.

I hope to remember that this time. I am fairly sure you explained this
to me once before already...

>> We don't have many objects that are accessed both before and after
>> arm_early_mmu_cache_invalidate, so maybe that's why we didn't run into
>> more problems?
>>
> Yea, I would guess that the probability of hitting this issue with the
> handoff data, which isn't that big, is quite low. At least from the
> description above I think we can hit the same issues with the handoff
> data.

Ack.

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

Cheers,
Ahmad

> 
> Regards,
> Lucas
> 
>> Thanks,
>> Ahmad
>>
>>
>>>
>>> Regards,
>>> Lucas
>>>
>>>> This means on e.g. the i.MX6UL, we will now do one extra cache invalidation
>>>> that's not needed. This should be negligible and we are already had an
>>>> unconditional invalidation in __barebox_arm_entry.
>>>>
>>>> Note that this is a different implementation than what we do on ARM64,
>>>> there we load TF-A before it jumps to OP-TEE and assuming
>>>> non-architected caches or caches with uninitialized content on power-on
>>>> to be a dying breed, our ARM64 implementation is likely not affected.
>>>>
>>>> Co-authored-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>> Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
>>>> ---
>>>>  arch/arm/lib32/optee-early.c | 13 +++++++++++++
>>>>  1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/arch/arm/lib32/optee-early.c b/arch/arm/lib32/optee-early.c
>>>> index 0cda0ab163..b1dba67d42 100644
>>>> --- a/arch/arm/lib32/optee-early.c
>>>> +++ b/arch/arm/lib32/optee-early.c
>>>> @@ -35,6 +35,19 @@ int start_optee_early(void *fdt, void *tee)
>>>>  	/* We use setjmp/longjmp here because OP-TEE clobbers most registers */
>>>>  	ret = setjmp(tee_buf);
>>>>  	if (ret == 0) {
>>>> +		/*
>>>> +		 * At least OP-TEE v4.1.0 seems to not invalidate all dirty cache
>>>> +		 * lines before enabling the MMU. This can lead to spurious hangs
>>>> +		 * on return to barebox on systems where there might be left-over
>>>> +		 * dirty cache lines, whether from BootROM or because L2 cache
>>>> +		 * is non-architected and powers on with unpredictable content
>>>> +		 * like is the case with PL310 on i.MX6Q.
>>>> +		 *
>>>> +		 * Let's invalidate the caches here, so board entry points need
>>>> +		 * not bother.
>>>> +		 */
>>>> +		arm_early_mmu_cache_invalidate();
>>>> +
>>>>  		tee_start(0, 0, fdt);
>>>>  		longjmp(tee_buf, 1);
>>>>  	}
>>>
>>>
>>
> 
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |