On 04/09/2013 12:45:31 PM, Wolfgang Denk wrote:
Dear Scott,
In message 1365449512.28843.10@snotra you wrote:
I think the terminology is just fine. It's not just invalidating
the
icache (flushing and invalidating are the same thing for cache lines which are not modified -- or are incapable of being modified). It's flushing the region out of *all* caches.
There is a well-defined difference between flushing and invalidating a cache, and I don't intend to go into hair-splitting mode here just to follow your path which would lead me to where I d not want to go.
If that means you have some other reason for objecting that you *do* want to go into, could you elaborate?
I do agree that flush is not a good name for what we do on the data side for PPC, given that we use dcbst and not dcbf.
See above. In such a case "icache" and "dcache" can just call the same underlying code.
And then we end up having to do the flush twice, if the user follows the "first flush dcache, then flush icache" instructions. That's
not
an ideal interface.
OK, this is indeed a good argument. But is this really the case? When looking at the current code of flash_cache() for PPC, it seems actually easy to split:
28 void flush_cache(ulong start_addr, ulong size) 29 { 30 #ifndef CONFIG_5xx 31 ulong addr, start, end; 32 33 start = start_addr & ~(CONFIG_SYS_CACHELINE_SIZE - 1); 34 end = start_addr + size - 1; 35 36 for (addr = start; (addr <= end) && (addr >= start); 37 addr += CONFIG_SYS_CACHELINE_SIZE) { 38 asm volatile("dcbst 0,%0" : : "r" (addr) : "memory"); 39 WATCHDOG_RESET(); 40 } 41 /* wait for all dcbst to complete on bus */ 42 asm volatile("sync" : : : "memory"); 43 44 for (addr = start; (addr <= end) && (addr >= start); 45 addr += CONFIG_SYS_CACHELINE_SIZE) { 46 asm volatile("icbi 0,%0" : : "r" (addr) : "memory"); 47 WATCHDOG_RESET(); 48 } 49 asm volatile("sync" : : : "memory"); 50 /* flush prefetch queue */ 51 asm volatile("isync" : : : "memory"); 52 #endif 53 }
would go into the "icache" command. Lines 36..42 would go into the "dcache" command, while lines 44..51
So far this code appears to work fine. What am I missing?
This is just one implementation of flush_cache(). Here are all of them:
arch/sparc/lib/cache.c:void flush_cache(ulong start_addr, ulong size) arch/openrisc/cpu/cache.c:void flush_cache(unsigned long addr, unsigned long size) arch/powerpc/lib/cache.c:void flush_cache(ulong start_addr, ulong size) arch/nds32/lib/cache.c:void flush_cache(unsigned long addr, unsigned long size) arch/m68k/lib/cache.c:void flush_cache(ulong start_addr, ulong size) arch/sh/cpu/sh4/cpu.c:void flush_cache (unsigned long addr, unsigned long size) arch/sh/cpu/sh2/cpu.c:void flush_cache(unsigned long addr, unsigned long size) arch/sh/cpu/sh3/cpu.c:void flush_cache(unsigned long addr, unsigned long size) arch/microblaze/cpu/cache.c:void flush_cache (ulong addr, ulong size) arch/arm/cpu/arm1136/cpu.c:void flush_cache(unsigned long start, unsigned long size) arch/arm/cpu/arm1136/cpu.c:void flush_cache(unsigned long start, unsigned long size) arch/arm/cpu/arm926ejs/cache.c:void flush_cache(unsigned long start, unsigned long size) arch/arm/cpu/arm926ejs/cache.c:void flush_cache(unsigned long start, unsigned long size) arch/blackfin/lib/cache.c:void flush_cache(unsigned long addr, unsigned long size) arch/mips/cpu/mips64/cpu.c:void flush_cache(ulong start_addr, ulong size) arch/mips/cpu/mips32/cpu.c:void flush_cache(ulong start_addr, ulong size) arch/mips/cpu/xburst/cpu.c:void flush_cache(ulong start_addr, ulong size)
This is what we're trying to expose as a command. If you want it split into icache and dcache, it needs to be rewritten for every architecture (or else we'd need to limit the command to only running on certain architectures), and not all of them are as easily splittable. What is the actual need for splitting it? The semantics we're trying to expose as a command are the same as this function is meant to implement, regardless of the choice of name. Those semantics are "make the data we just wrote to this range visible to instruction fetches".
-Scott