Re: [PATCH 1/3] ARM: cpu: suppress arm_early_mmu_cache_invalidate if dcache enabled

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

Hello Lucas,

On 7/7/26 5:22 PM, Lucas Stach wrote:
> Am Dienstag, dem 07.07.2026 um 10:05 +0200 schrieb Ahmad Fatoum:
>> barebox built as EFI payload on ARM invalidates the data caches inside
>> barebox_arm_entry(), which may lead to memory corruption.
>>
>> Generally, calling arm_early_mmu_cache_invalidate() while the caches are
>> enabled is a bad idea, so add a function that protects against that and
>> use it in common code.
>>
>> Fixes: 742e78976dd4 ("ARM64: add optional EFI stub")

>> +/**
>> + * dcache_invalidate_stale - invalidate data cache prior to enabling it
>> + *
>> + * Some SoCs can come up with invalid entries, but with the valid bit set.
>> + * This function discards them, as that would lead to memory corruption
>> + * otherwise.
>> + */
>> +void dcache_invalidate_stale(void)
> 
> I don't like the naming of this function, as
> arm_early_mmu_cache_invalidate() invalidates both the D and I cache, as
> well as a unified cache after the PoU if it's part of the architected
> hierarchy. This is the desired behavior, as both the I and D side can
> come up with invalid entries.

Ah, I had not thought much about the I cache having invalid entries as well.

How about cache_invalidate_stale()?

> It's okay to only check for CR_C, as invalidating the I side when there
> are already cached entries only has a minor impact on performance, but
> won't affect correctness.

I thought that it might be too surprising to change the semantics of
this function in such a way, especially as it has a number of users in
low level board code.

But yes, if we don't see a legitimate use case for calling the function
with caches enabled, we can also just turn it into a no-op in that case.

> Also, by moving the call to a later point in the init flow later in the
> series, are you sure that the invalidate happens before the I cache
> gets enabled in arm_cpu_lowlevel_init()?

arm_cpu_lowlevel_init() invalidates I cache on ARMv6 and ARMv7.
I believe on ARMv8, it's not necessary?

Interestingly, for ARMv8, I-Cache looks like it is only enabled early
for EL3, but for EL2/1, it's enabled in mmu_early_enable(). Any reason
for that?

Thanks,
Ahmad

> 
> Regards,
> Lucas
> 
>> +{
>> +	/* if caches are already enabled, don't cause data loss */
>> +	if (get_cr() & CR_C)
>> +		return;
>> +
>> +	arm_early_mmu_cache_invalidate();
>> +}
>> +
>>  void pbl_barebox_break(void)
>>  {
>>  	__asm__ __volatile__ (
>> diff --git a/arch/arm/cpu/entry_ll_32.S b/arch/arm/cpu/entry_ll_32.S
>> index 0d4c47c1c870..eb1793b54e66 100644
>> --- a/arch/arm/cpu/entry_ll_32.S
>> +++ b/arch/arm/cpu/entry_ll_32.S
>> @@ -15,7 +15,7 @@ ENTRY(__barebox_arm_entry)
>>  	mov	r4, r0
>>  	mov	r5, r1
>>  	mov	r6, r2
>> -	bl	arm_early_mmu_cache_invalidate
>> +	bl	dcache_invalidate_stale
>>  	mov	r0, r4
>>  	mov	r1, r5
>>  	mov	r2, r6
>> diff --git a/arch/arm/cpu/entry_ll_64.S b/arch/arm/cpu/entry_ll_64.S
>> index 5eb6efed5baf..3404f6d05802 100644
>> --- a/arch/arm/cpu/entry_ll_64.S
>> +++ b/arch/arm/cpu/entry_ll_64.S
>> @@ -15,7 +15,7 @@ ENTRY(__barebox_arm_entry)
>>  	mov	x19, x0
>>  	mov	x20, x1
>>  	mov	x21, x2
>> -	bl	arm_early_mmu_cache_invalidate
>> +	bl	dcache_invalidate_stale
>>  	mov	x0, x19
>>  	mov	x1, x20
>>  	mov	x2, x21
>> diff --git a/arch/arm/include/asm/cache.h b/arch/arm/include/asm/cache.h
>> index ea78ae123aec..64f369f865db 100644
>> --- a/arch/arm/include/asm/cache.h
>> +++ b/arch/arm/include/asm/cache.h
>> @@ -26,6 +26,9 @@ static inline void icache_invalidate(void)
>>  #endif
>>  }
>>  
>> +
>> +void dcache_invalidate_stale(void);
>> +
>>  void arm_early_mmu_cache_flush(void);
>>  void arm_early_mmu_cache_invalidate(void);
>>  
> 

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