| Message ID | 20190529224659.27614-1-paul.burton@mips.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | MIPS: mm: Implement flush_cache_v(un)map using __flush_kernel_vmap_range | expand |
Hi Paul, On 30/05/19 10:47 AM, Paul Burton wrote: > Our flush_cache_vmap() & flush_cache_vunmap() implementations for R4k > style systems simply call r4k_blast_dcache() to wipe out the whole L1 > dcache if it suffers from aliases. This is unsafe on SMP for 2 reasons: > > 1) r4k_blast_dcache() relies upon preemption being disabled so that it > can use current_cpu_data/smp_processor_id() to discover the > properties of the current CPU's dcache & ensure the whole flush > operation happens on one CPU. This may not be the case when > flush_cache_vmap() or flush_cache_vunmap() are called. > > 2) It only flushes caches on one CPU, which means the caches for other > CPUs may still contain stale data. > > We already have an implementation of flush_kernel_vmap_range() which > does exactly what we need - it invalidates dcache entries on all CPUs > safely, and is better optimized to avoid wiping out the entire cache for > small flushes. > > Reimplement flush_cache_vmap() & flush_cache_vunmap() using > __flush_kernel_vmap_range() which does what we need already. > > For tx39 __flush_kernel_vmap_range() will simply BUG(), but so far as I > can see tx39 systems don't suffer from dcache aliasing so this should be > fine since it should never be called. > > Signed-off-by: Paul Burton <paul.burton@mips.com> > Reported-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- > Chris, would you mind giving this a try? This doesn't quite seem to work. It avoids the BUG() but I get other bad behavior. I can't discount it being something odd about my half finished port but I can replicate the badness without this patch. One is a kernel panic in response to activity on the console (output below). The other seems to be a result of receiving garbage on the console. I wonder if the range needs to be rounded up to a cacheline boundary? Run /sbin/init as init process overlayfs: upper fs does not support xattr, falling back to index=off and metacopy=off. overlayfs: upper fs does not support xattr, falling back to index=off and metacopy=off. overlayfs: upper fs does not support xattr, falling back to index=off and metacopy=off. random: crng init done CPU 0 Unable to handle kernel paging request at virtual address 00000000, epc == 80652e00, ra == 80652dc4 Oops[#1]: CPU: 0 PID: 7 Comm: kworker/u4:0 Not tainted 5.1.0-at1 #15 Workqueue: events_unbound flush_to_ldisc $ 0 : 00000000 00000001 00000000 00000000 $ 4 : 8f82aa00 fffffff8 fffffffd 00000001 $ 8 : 8da08818 00000000 00000001 00000018 $12 : fefefeff 7f7f7f7f 00000001 00000001 $16 : c00e1268 c00e126c 00000002 8f84bd18 $20 : 8da08819 83fa6e00 00000001 c00e1270 $24 : 0000c3b6 80336694 $28 : 8f84a000 8f84bcf0 0000000d 80652dc4 Hi : d49c5168 Lo : 7f29931c epc : 80652e00 __mutex_lock.isra.1+0xa8/0x464 ra : 80652dc4 __mutex_lock.isra.1+0x6c/0x464 Status: 10008f03 KERNEL EXL IE Cause : 0080000c (ExcCode 03) BadVA : 00000000 PrId : 0002a080 (Broadcom BMIPS4350) Modules linked in: Process kworker/u4:0 (pid: 7, threadinfo=f89f9771, task=a0a456a2, tls=00000000) Stack : 00000002 8064f534 8f82aa80 8f82aa80 6b2dc12d 8006b2c8 8090fc78 83fa6e6c 83fa6e6c 8064f4a4 c00e1270 00000000 83fa6e6c 8064f534 c00df000 83fa6e00 c00e1268 00000000 8da08819 83fa6e00 00000001 c00df018 0000000d 80334b50 83fa6e6c 80654aa0 fffb8a00 80790000 83fa6e00 0000000a c00df000 803358a0 8f84bd70 00000000 00000000 8da08818 00000000 00000000 8da08818 80336520 ... Call Trace: [<80652e00>] __mutex_lock.isra.1+0xa8/0x464 [<80334b50>] commit_echoes+0x28/0xc4 [<803358a0>] n_tty_receive_char_special+0xa20/0xc04 [<80336520>] n_tty_receive_buf_common+0xa9c/0xc10 [<803366ac>] n_tty_receive_buf2+0x18/0x24 [<8033a6b8>] tty_port_default_receive_buf+0x50/0xa4 [<80339fc4>] flush_to_ldisc+0xb8/0x12c [<80052098>] process_one_work+0x224/0x424 [<80052400>] worker_thread+0x168/0x5a0 [<8005882c>] kthread+0x13c/0x144 [<80015e6c>] ret_from_kernel_thread+0x14/0x1c Code: afa2002c ae13000c afb70028 <ac530000> 8e020008 105300b6 00000000 8f820000 afa20030 > --- > arch/mips/include/asm/cacheflush.h | 28 ++++++++++++---------------- > arch/mips/mm/c-r4k.c | 15 --------------- > arch/mips/mm/c-tx39.c | 16 ---------------- > arch/mips/mm/cache.c | 3 --- > 4 files changed, 12 insertions(+), 50 deletions(-) > > diff --git a/arch/mips/include/asm/cacheflush.h b/arch/mips/include/asm/cacheflush.h > index d687b40b9fbb..6285c830c9f2 100644 > --- a/arch/mips/include/asm/cacheflush.h > +++ b/arch/mips/include/asm/cacheflush.h > @@ -85,22 +85,6 @@ extern void (*__flush_icache_user_range)(unsigned long start, > extern void (*__local_flush_icache_user_range)(unsigned long start, > unsigned long end); > > -extern void (*__flush_cache_vmap)(void); > - > -static inline void flush_cache_vmap(unsigned long start, unsigned long end) > -{ > - if (cpu_has_dc_aliases) > - __flush_cache_vmap(); > -} > - > -extern void (*__flush_cache_vunmap)(void); > - > -static inline void flush_cache_vunmap(unsigned long start, unsigned long end) > -{ > - if (cpu_has_dc_aliases) > - __flush_cache_vunmap(); > -} > - > extern void copy_to_user_page(struct vm_area_struct *vma, > struct page *page, unsigned long vaddr, void *dst, const void *src, > unsigned long len); > @@ -150,4 +134,16 @@ static inline void invalidate_kernel_vmap_range(void *vaddr, int size) > __flush_kernel_vmap_range((unsigned long) vaddr, size); > } > > +static inline void flush_cache_vmap(unsigned long start, unsigned long end) > +{ > + if (cpu_has_dc_aliases) > + __flush_kernel_vmap_range(start, end - start); > +} > + > +static inline void flush_cache_vunmap(unsigned long start, unsigned long end) > +{ > + if (cpu_has_dc_aliases) > + __flush_kernel_vmap_range(start, end - start); > +} > + > #endif /* _ASM_CACHEFLUSH_H */ > diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c > index 5166e38cd1c6..2b2953d3949d 100644 > --- a/arch/mips/mm/c-r4k.c > +++ b/arch/mips/mm/c-r4k.c > @@ -559,16 +559,6 @@ static inline int has_valid_asid(const struct mm_struct *mm, unsigned int type) > return 0; > } > > -static void r4k__flush_cache_vmap(void) > -{ > - r4k_blast_dcache(); > -} > - > -static void r4k__flush_cache_vunmap(void) > -{ > - r4k_blast_dcache(); > -} > - > /* > * Note: flush_tlb_range() assumes flush_cache_range() sufficiently flushes > * whole caches when vma is executable. > @@ -1854,9 +1844,6 @@ void r4k_cache_init(void) > else > shm_align_mask = PAGE_SIZE-1; > > - __flush_cache_vmap = r4k__flush_cache_vmap; > - __flush_cache_vunmap = r4k__flush_cache_vunmap; > - > flush_cache_all = cache_noop; > __flush_cache_all = r4k___flush_cache_all; > flush_cache_mm = r4k_flush_cache_mm; > @@ -1931,8 +1918,6 @@ void r4k_cache_init(void) > case CPU_LOONGSON3: > /* Loongson-3 maintains cache coherency by hardware */ > __flush_cache_all = cache_noop; > - __flush_cache_vmap = cache_noop; > - __flush_cache_vunmap = cache_noop; > __flush_kernel_vmap_range = (void *)cache_noop; > flush_cache_mm = (void *)cache_noop; > flush_cache_page = (void *)cache_noop; > diff --git a/arch/mips/mm/c-tx39.c b/arch/mips/mm/c-tx39.c > index b7c8a9d79c35..6bf13a7db485 100644 > --- a/arch/mips/mm/c-tx39.c > +++ b/arch/mips/mm/c-tx39.c > @@ -121,16 +121,6 @@ static inline void tx39_blast_icache(void) > local_irq_restore(flags); > } > > -static void tx39__flush_cache_vmap(void) > -{ > - tx39_blast_dcache(); > -} > - > -static void tx39__flush_cache_vunmap(void) > -{ > - tx39_blast_dcache(); > -} > - > static inline void tx39_flush_cache_all(void) > { > if (!cpu_has_dc_aliases) > @@ -339,8 +329,6 @@ void tx39_cache_init(void) > switch (current_cpu_type()) { > case CPU_TX3912: > /* TX39/H core (writethru direct-map cache) */ > - __flush_cache_vmap = tx39__flush_cache_vmap; > - __flush_cache_vunmap = tx39__flush_cache_vunmap; > flush_cache_all = tx39h_flush_icache_all; > __flush_cache_all = tx39h_flush_icache_all; > flush_cache_mm = (void *) tx39h_flush_icache_all; > @@ -363,10 +351,6 @@ void tx39_cache_init(void) > default: > /* TX39/H2,H3 core (writeback 2way-set-associative cache) */ > /* board-dependent init code may set WBON */ > - > - __flush_cache_vmap = tx39__flush_cache_vmap; > - __flush_cache_vunmap = tx39__flush_cache_vunmap; > - > flush_cache_all = tx39_flush_cache_all; > __flush_cache_all = tx39___flush_cache_all; > flush_cache_mm = tx39_flush_cache_mm; > diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c > index 3da216988672..c6c0ef539d3a 100644 > --- a/arch/mips/mm/cache.c > +++ b/arch/mips/mm/cache.c > @@ -40,9 +40,6 @@ EXPORT_SYMBOL_GPL(__flush_icache_user_range); > void (*__local_flush_icache_user_range)(unsigned long start, unsigned long end); > EXPORT_SYMBOL_GPL(__local_flush_icache_user_range); > > -void (*__flush_cache_vmap)(void); > -void (*__flush_cache_vunmap)(void); > - > void (*__flush_kernel_vmap_range)(unsigned long vaddr, int size); > EXPORT_SYMBOL_GPL(__flush_kernel_vmap_range); > >
diff --git a/arch/mips/include/asm/cacheflush.h b/arch/mips/include/asm/cacheflush.h index d687b40b9fbb..6285c830c9f2 100644 --- a/arch/mips/include/asm/cacheflush.h +++ b/arch/mips/include/asm/cacheflush.h @@ -85,22 +85,6 @@ extern void (*__flush_icache_user_range)(unsigned long start, extern void (*__local_flush_icache_user_range)(unsigned long start, unsigned long end); -extern void (*__flush_cache_vmap)(void); - -static inline void flush_cache_vmap(unsigned long start, unsigned long end) -{ - if (cpu_has_dc_aliases) - __flush_cache_vmap(); -} - -extern void (*__flush_cache_vunmap)(void); - -static inline void flush_cache_vunmap(unsigned long start, unsigned long end) -{ - if (cpu_has_dc_aliases) - __flush_cache_vunmap(); -} - extern void copy_to_user_page(struct vm_area_struct *vma, struct page *page, unsigned long vaddr, void *dst, const void *src, unsigned long len); @@ -150,4 +134,16 @@ static inline void invalidate_kernel_vmap_range(void *vaddr, int size) __flush_kernel_vmap_range((unsigned long) vaddr, size); } +static inline void flush_cache_vmap(unsigned long start, unsigned long end) +{ + if (cpu_has_dc_aliases) + __flush_kernel_vmap_range(start, end - start); +} + +static inline void flush_cache_vunmap(unsigned long start, unsigned long end) +{ + if (cpu_has_dc_aliases) + __flush_kernel_vmap_range(start, end - start); +} + #endif /* _ASM_CACHEFLUSH_H */ diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c index 5166e38cd1c6..2b2953d3949d 100644 --- a/arch/mips/mm/c-r4k.c +++ b/arch/mips/mm/c-r4k.c @@ -559,16 +559,6 @@ static inline int has_valid_asid(const struct mm_struct *mm, unsigned int type) return 0; } -static void r4k__flush_cache_vmap(void) -{ - r4k_blast_dcache(); -} - -static void r4k__flush_cache_vunmap(void) -{ - r4k_blast_dcache(); -} - /* * Note: flush_tlb_range() assumes flush_cache_range() sufficiently flushes * whole caches when vma is executable. @@ -1854,9 +1844,6 @@ void r4k_cache_init(void) else shm_align_mask = PAGE_SIZE-1; - __flush_cache_vmap = r4k__flush_cache_vmap; - __flush_cache_vunmap = r4k__flush_cache_vunmap; - flush_cache_all = cache_noop; __flush_cache_all = r4k___flush_cache_all; flush_cache_mm = r4k_flush_cache_mm; @@ -1931,8 +1918,6 @@ void r4k_cache_init(void) case CPU_LOONGSON3: /* Loongson-3 maintains cache coherency by hardware */ __flush_cache_all = cache_noop; - __flush_cache_vmap = cache_noop; - __flush_cache_vunmap = cache_noop; __flush_kernel_vmap_range = (void *)cache_noop; flush_cache_mm = (void *)cache_noop; flush_cache_page = (void *)cache_noop; diff --git a/arch/mips/mm/c-tx39.c b/arch/mips/mm/c-tx39.c index b7c8a9d79c35..6bf13a7db485 100644 --- a/arch/mips/mm/c-tx39.c +++ b/arch/mips/mm/c-tx39.c @@ -121,16 +121,6 @@ static inline void tx39_blast_icache(void) local_irq_restore(flags); } -static void tx39__flush_cache_vmap(void) -{ - tx39_blast_dcache(); -} - -static void tx39__flush_cache_vunmap(void) -{ - tx39_blast_dcache(); -} - static inline void tx39_flush_cache_all(void) { if (!cpu_has_dc_aliases) @@ -339,8 +329,6 @@ void tx39_cache_init(void) switch (current_cpu_type()) { case CPU_TX3912: /* TX39/H core (writethru direct-map cache) */ - __flush_cache_vmap = tx39__flush_cache_vmap; - __flush_cache_vunmap = tx39__flush_cache_vunmap; flush_cache_all = tx39h_flush_icache_all; __flush_cache_all = tx39h_flush_icache_all; flush_cache_mm = (void *) tx39h_flush_icache_all; @@ -363,10 +351,6 @@ void tx39_cache_init(void) default: /* TX39/H2,H3 core (writeback 2way-set-associative cache) */ /* board-dependent init code may set WBON */ - - __flush_cache_vmap = tx39__flush_cache_vmap; - __flush_cache_vunmap = tx39__flush_cache_vunmap; - flush_cache_all = tx39_flush_cache_all; __flush_cache_all = tx39___flush_cache_all; flush_cache_mm = tx39_flush_cache_mm; diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c index 3da216988672..c6c0ef539d3a 100644 --- a/arch/mips/mm/cache.c +++ b/arch/mips/mm/cache.c @@ -40,9 +40,6 @@ EXPORT_SYMBOL_GPL(__flush_icache_user_range); void (*__local_flush_icache_user_range)(unsigned long start, unsigned long end); EXPORT_SYMBOL_GPL(__local_flush_icache_user_range); -void (*__flush_cache_vmap)(void); -void (*__flush_cache_vunmap)(void); - void (*__flush_kernel_vmap_range)(unsigned long vaddr, int size); EXPORT_SYMBOL_GPL(__flush_kernel_vmap_range);
Our flush_cache_vmap() & flush_cache_vunmap() implementations for R4k style systems simply call r4k_blast_dcache() to wipe out the whole L1 dcache if it suffers from aliases. This is unsafe on SMP for 2 reasons: 1) r4k_blast_dcache() relies upon preemption being disabled so that it can use current_cpu_data/smp_processor_id() to discover the properties of the current CPU's dcache & ensure the whole flush operation happens on one CPU. This may not be the case when flush_cache_vmap() or flush_cache_vunmap() are called. 2) It only flushes caches on one CPU, which means the caches for other CPUs may still contain stale data. We already have an implementation of flush_kernel_vmap_range() which does exactly what we need - it invalidates dcache entries on all CPUs safely, and is better optimized to avoid wiping out the entire cache for small flushes. Reimplement flush_cache_vmap() & flush_cache_vunmap() using __flush_kernel_vmap_range() which does what we need already. For tx39 __flush_kernel_vmap_range() will simply BUG(), but so far as I can see tx39 systems don't suffer from dcache aliasing so this should be fine since it should never be called. Signed-off-by: Paul Burton <paul.burton@mips.com> Reported-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- Chris, would you mind giving this a try? --- arch/mips/include/asm/cacheflush.h | 28 ++++++++++++---------------- arch/mips/mm/c-r4k.c | 15 --------------- arch/mips/mm/c-tx39.c | 16 ---------------- arch/mips/mm/cache.c | 3 --- 4 files changed, 12 insertions(+), 50 deletions(-)