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

[Lucas Stach <l.stach@pengutronix.de>]

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")
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/arm/cpu/common.c        | 16 ++++++++++++++++
>  arch/arm/cpu/entry_ll_32.S   |  2 +-
>  arch/arm/cpu/entry_ll_64.S   |  2 +-
>  arch/arm/include/asm/cache.h |  3 +++
>  4 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/cpu/common.c b/arch/arm/cpu/common.c
> index adb5d6a02bc8..d41da73d7e83 100644
> --- a/arch/arm/cpu/common.c
> +++ b/arch/arm/cpu/common.c
> @@ -37,6 +37,22 @@ void sync_caches_for_execution(void)
>  	arm_early_mmu_cache_flush();
>  }
>  
> +/**
> + * 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.

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.

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()?

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