[U-Boot] [Resend RFC PATCH 1/2] armv8: Fix dcache disable function

[Stephen Warren]

On 10/14/2016 02:17 PM, York Sun wrote:
> Current code turns off d-cache first, then flush all levels of cache.
> This results data loss. As soon as d-cache is off, the dirty cache
> is discarded according to the test on LS2080A. This issue was not
> seen as long as external L3 cache was flushed to push the data to
> main memory. However, external L3 cache is not guaranteed to have
> the data. To fix this, flush the d-cache by way/set first to make
> sure cache is clean before turning it off.

> diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c

> @@ -478,9 +478,9 @@ void dcache_disable(void)

> +	flush_dcache_all();
>  	set_sctlr(sctlr & ~(CR_C|CR_M));
>
> -	flush_dcache_all();
>  	__asm_invalidate_tlb_all();

I talked to Mark Rutland at ARM, and I believe the current code is 
correct. Here's my interpretation of what he said:

The dcache must be disabled first. This prevents allocation of new 
entries in the cache during the flush operation, which prevents the race 
conditions that were mentioned in the other thread.

Then, the flush operation must be invoked. Since the cache is now 
disabled, this can fully flush the cache without worrying about racing 
with things being added to the cache.

This all implies that the implementation of dcache_disable(), 
set_sctlr(), flush_dcache_all(), and any code they call must not access 
data in DRAM at all; since because the dcache is off, any DRAM access 
will[1] read potentially stale data from DRAM, rather than any dirty 
data that might be in the cache.

[1] I'm not sure if that's "will" or "may", i.e. whether this is 
architecturally guaranteed in ARMv8 or is implementation defined. At 
least the Cortex A72 TRM says "will" for that CPU; not sure about others.

Perhaps the most obvious upshot of this is that the stack can't be used. 
This implies to me that we need to recode all those functions purely in 
assembly, or just possibly play some tricks to 100% force gcc not to 
touch memory anywhere inside dcache_disable() or the functions it calls. 
We're just getting lucky here right now since everything happens to be 
inlined, but I don't think we're doing anything to 100% guarantee this.

What worries me here is that at least on Tegra, a "flush the entire 
dcache" operation requires an SMC call to the secure monitor. That will 
certainly access DRAM when the secure monitor runs, but perhaps this 
doesn't matter since that's at a different exception level, and we know 
the secure monitor accesses DRAM regions that are separate from U-Boot's 
DRAM? I suspect life isn't that convenient. I'm wondering if this all 
implies that, like patch 2 in this series, we need to get 100% away from 
flush-by-set/way, even with SoC-specific hooks to make that work 
reliably, and just flush everything by VA, which IIRC is architecturally 
guaranteed to work without SoC-specific logic. That way, we can 
encapsulate everything into an assembly function without worrying about 
calling SMCs or SoC-specific hook functions without using DRAM. Of 
course, how that assembly function knows which VAs to flush without 
decoding the page tables or other data structure is another matter:-(