
Hi,
Sorry for joining this a bit late; apologies if the below re-treads ground already covered.
On Wed, Oct 19, 2016 at 09:25:02AM -0600, Stephen Warren wrote:
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.
Well, almost, but not quite. It's a long story... ;)
I gave a primer [1,2] on the details at ELC earlier this year, which may or may not be useful.
The big details are:
* Generaly "Flush" is ambiguous/meaningless. Here you seem to want clean+invalidate.
* Set/Way operations are for IMPLEMENTATION DEFINED (i.e. SoC-specific) cache maintenance sequences, and are not truly portable (e.g. not affecting system caches).
I assume that an earlier boot stage initialised the caches prior to U-Boot. Given that, you *only* need to perform maintenance for the memory you have (at any point) mapped with cacheable attrbiutes, which should be a small subset of the PA space. With ARMv8-A, broadcast maintenance to the PoC should affect all relevant caches (assuming you use the correct shareability attributes).
* You *cannot* write a dcache disable routine in C, as the compiler can perform a number of implicit memory accesses (e.g. stack, globals, GOT). For that alone, I do not believe the code above is correct.
Note that we have seen this being an issue in practice, before we got rid of Set/Way ops from arm64 Linux (see commit 5e051531447259e5).
* Your dcache disable code *must* be clean to the PoC, prior to execution, or instruction fetches could see stale data. You can first *clean* this to the PoC, which is sufficient to avoid the problems above.
* The SCTLR_ELx.{C,I} bits do not enable/disable caches; they merely activate/deactiveate cacheable attributes on data/instruction fetches.
Note that cacheable instruction fetches can allocate into unified/data caches.
Also, note that the I bit is independent of the C bit, and the attributes it provides differ when the M bit is clear. Generally, I would advise that at all times M == C == I, as that leads to the least surprise.
Thanks, Mark.
[1] http://events.linuxfoundation.org/sites/events/files/slides/slides_17.pdf [2] https://www.youtube.com/watch?v=F0SlIMHRnLk