[PATCH] MIPS: mm: Use SMP safe operations for flush_cache_vmap

[PATCH] MIPS: mm: Use SMP safe operations for flush_cache_vmap

[Chris Packham]

flush_cache_vmap() and flush_cache_vunmap() were calling
r4k_blast_dcache() which is not safe to do on a SMP system. Redefine
them to call r4k_flush_kernel_vmap_range() which will correctly handle
the SMP/UP cases.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
I don't know if passing 0, dcache_size to r4k_flush_kernel_vmap_range is
a good idea. It does have the desired outcome but it could be a bit
fragile.

Getting the address and size through to r4k__flush_cache_vmap involves
updating the prototype for __flush_cache_vmap() which meant touching
more code than I was comfortable doing. It would be relatively straight
forward to do but then I'd hit things I have no way of testing.

 arch/mips/mm/c-r4k.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
index 5166e38cd1c6..976c54268456 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.
@@ -986,6 +976,16 @@ static void r4k_flush_kernel_vmap_range(unsigned long vaddr, int size)
 				&args);
 }
 
+static void r4k__flush_cache_vmap(void)
+{
+	r4k_flush_kernel_vmap_range(0, dcache_size);
+}
+
+static void r4k__flush_cache_vunmap(void)
+{
+	r4k_flush_kernel_vmap_range(0, dcache_size);
+}
+
 static inline void rm7k_erratum31(void)
 {
 	const unsigned long ic_lsize = 32;
-- 
2.21.0

[PATCH] MIPS: mm: Implement flush_cache_v(un)map using __flush_kernel_vmap_range

[Paul Burton]

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(-)

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);
 
-- 
2.21.0

Re: [PATCH] MIPS: mm: Implement flush_cache_v(un)map using __flush_kernel_vmap_range

[Chris Packham]

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);
>   
>