[RFC PATCH 1/1] arm: add invalidate_dcache_all() after disable cache

[Heinrich Schuchardt]

On 5/31/23 00:08, Emanuele Ghidoli wrote:
> From: Emanuele Ghidoli <emanuele.ghidoli at toradex.com>
>
> On Cortex-R5 flushing and disabling cache is not enough to avoid
> cache and memory incoherence.
>
> In particular, when the cache is enabled after a disable, and if there
> are entry in the cache the value from the cache is used instead of the
> value from the memory. This, in particular, lead to stack corruption if
> the stack is in a cached area.
>
> The commit 44df5e8d30a2 ("arm: move flush_dcache_all() to just before disable cache")
> already states that following a cache disable an invalidate is expected
> to be done but without coping with cache enable. So just add it explicitly.
>
> Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli at toradex.com>
> ---
>   arch/arm/lib/cache-cp15.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
> index 0893915b3004..2921277d69e0 100644
> --- a/arch/arm/lib/cache-cp15.c
> +++ b/arch/arm/lib/cache-cp15.c
> @@ -254,7 +254,15 @@ static void cache_disable(uint32_t cache_bit)
>   	if (cache_bit == CR_C)
>   #endif
>   		flush_dcache_all();
> +
>   	set_cr(reg & ~cache_bit);
> +
> +#ifdef CONFIG_SYS_ARM_MMU
> +	if (cache_bit == (CR_C | CR_M))
> +#elif defined(CONFIG_SYS_ARM_MPU)
> +	if (cache_bit == CR_C)
> +#endif

We prefer 'if' over '#ifdef'. So you should use something like:

if (IS_ENABLED(CONFIG_SYS_ARM_MMU) && (cache_bit == (CR_C | CR_M) ||
     IS_ENABLED(CONFIG_SYS_ARM_MPU) && (cache_bit == CR_C)) {
	flush_dcache_all();
         invalidate_dcache_all();
}
set_cr(reg & ~cache_bit);

(Assuming set_cr() can be called after invalidate_dcache_all.)

Best regards

Heinrich

> +		invalidate_dcache_all();
>   }
>   #endif
>