All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/6] powerpc: convert cache asm to C
@ 2019-11-04  2:32 ` Alastair D'Silva
  0 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Christophe Leroy, Greg Kroah-Hartman, Thomas Gleixner, Qian Cai,
	Nicholas Piggin, Allison Randal, Andrew Morton,
	David Hildenbrand, linuxppc-dev, linux-kernel

From: Alastair D'Silva <alastair@d-silva.org>

This series addresses a few issues discovered in how we flush caches:
1. Flushes were truncated at 4GB, so larger flushes were incorrect.
2. Flushing the dcache in arch_add_memory was unnecessary

This series also converts much of the cache assembler to C, with the
aim of making it easier to maintain.

Changelog:
 V5:
    - in "Chunk Calls to flush_dcache_range ...":
        - Honour the 'chunk' parameter
    - Rebase to latest master
 V4:
     - Split out VDSO patch
     - Pass/cast the correct types in 'powerpc: Convert
       flush_icache_range & friends to C'
 V3:
     - factor out chunking loop
     - Replace __asm__ __volatile__ with asm volatile
     - Replace flush_coherent_icache_or_return macro with
       flush_coherent_icache function
     - factor our invalidate_icache_range
     - Replace code duplicating clean_dcache_range() in
       __flush_dcache_icache() with a call to clean_dcache_range()
     - Remove redundant #ifdef CONFIG_44x
     - Fix preprocessor logic:
         #if !defined(CONFIG_PPC_8xx) & !defined(CONFIG_PPC64)
     - Added loop(1|2) to earlyclobbers in flush_dcache_icache_phys
     - Drop "Remove extern" patch
     - Replace 32 bit shifts in 64 bit VDSO with 64 bit ones
 V2:
     - Replace C implementation of flush_dcache_icache_phys() with
       inline assembler authored by Christophe Leroy
     - Add memory clobbers for iccci implementation
     - Give __flush_dcache_icache a real implementation, it can't
       just be a wrapper around flush_icache_range()
     - Remove PPC64_CACHES from misc_64.S
     - Replace code duplicating clean_dcache_range() in
       flush_icache_range() with a call to clean_dcache_range()
     - Replace #ifdef CONFIG_44x with IS_ENABLED(...) in
       flush_icache_cange()
     - Use 1GB chunks instead of 16GB in arch_*_memory

Alastair D'Silva (6):
  powerpc: Allow flush_icache_range to work across ranges >4GB
  powerpc: Allow 64bit VDSO __kernel_sync_dicache to work across ranges
    >4GB
  powerpc: define helpers to get L1 icache sizes
  powerpc: Convert flush_icache_range & friends to C
  powerpc: Chunk calls to flush_dcache_range in arch_*_memory
  powerpc: Don't flush caches when adding memory

 arch/powerpc/include/asm/cache.h        |  55 +++++---
 arch/powerpc/include/asm/cacheflush.h   |  36 +++--
 arch/powerpc/kernel/misc_32.S           | 120 ----------------
 arch/powerpc/kernel/misc_64.S           | 102 --------------
 arch/powerpc/kernel/vdso64/cacheflush.S |   4 +-
 arch/powerpc/mm/mem.c                   | 176 +++++++++++++++++++++++-
 6 files changed, 228 insertions(+), 265 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v5 0/6] powerpc: convert cache asm to C
@ 2019-11-04  2:32 ` Alastair D'Silva
  0 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: David Hildenbrand, linux-kernel, Nicholas Piggin, Paul Mackerras,
	Greg Kroah-Hartman, Qian Cai, Thomas Gleixner, linuxppc-dev,
	Andrew Morton, Allison Randal

From: Alastair D'Silva <alastair@d-silva.org>

This series addresses a few issues discovered in how we flush caches:
1. Flushes were truncated at 4GB, so larger flushes were incorrect.
2. Flushing the dcache in arch_add_memory was unnecessary

This series also converts much of the cache assembler to C, with the
aim of making it easier to maintain.

Changelog:
 V5:
    - in "Chunk Calls to flush_dcache_range ...":
        - Honour the 'chunk' parameter
    - Rebase to latest master
 V4:
     - Split out VDSO patch
     - Pass/cast the correct types in 'powerpc: Convert
       flush_icache_range & friends to C'
 V3:
     - factor out chunking loop
     - Replace __asm__ __volatile__ with asm volatile
     - Replace flush_coherent_icache_or_return macro with
       flush_coherent_icache function
     - factor our invalidate_icache_range
     - Replace code duplicating clean_dcache_range() in
       __flush_dcache_icache() with a call to clean_dcache_range()
     - Remove redundant #ifdef CONFIG_44x
     - Fix preprocessor logic:
         #if !defined(CONFIG_PPC_8xx) & !defined(CONFIG_PPC64)
     - Added loop(1|2) to earlyclobbers in flush_dcache_icache_phys
     - Drop "Remove extern" patch
     - Replace 32 bit shifts in 64 bit VDSO with 64 bit ones
 V2:
     - Replace C implementation of flush_dcache_icache_phys() with
       inline assembler authored by Christophe Leroy
     - Add memory clobbers for iccci implementation
     - Give __flush_dcache_icache a real implementation, it can't
       just be a wrapper around flush_icache_range()
     - Remove PPC64_CACHES from misc_64.S
     - Replace code duplicating clean_dcache_range() in
       flush_icache_range() with a call to clean_dcache_range()
     - Replace #ifdef CONFIG_44x with IS_ENABLED(...) in
       flush_icache_cange()
     - Use 1GB chunks instead of 16GB in arch_*_memory

Alastair D'Silva (6):
  powerpc: Allow flush_icache_range to work across ranges >4GB
  powerpc: Allow 64bit VDSO __kernel_sync_dicache to work across ranges
    >4GB
  powerpc: define helpers to get L1 icache sizes
  powerpc: Convert flush_icache_range & friends to C
  powerpc: Chunk calls to flush_dcache_range in arch_*_memory
  powerpc: Don't flush caches when adding memory

 arch/powerpc/include/asm/cache.h        |  55 +++++---
 arch/powerpc/include/asm/cacheflush.h   |  36 +++--
 arch/powerpc/kernel/misc_32.S           | 120 ----------------
 arch/powerpc/kernel/misc_64.S           | 102 --------------
 arch/powerpc/kernel/vdso64/cacheflush.S |   4 +-
 arch/powerpc/mm/mem.c                   | 176 +++++++++++++++++++++++-
 6 files changed, 228 insertions(+), 265 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v5 1/6] powerpc: Allow flush_icache_range to work across ranges >4GB
  2019-11-04  2:32 ` Alastair D'Silva
@ 2019-11-04  2:32   ` Alastair D'Silva
  -1 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: stable, Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Christophe Leroy, Allison Randal, Greg Kroah-Hartman, Qian Cai,
	Thomas Gleixner, Nicholas Piggin, Andrew Morton,
	David Hildenbrand, linuxppc-dev, linux-kernel

From: Alastair D'Silva <alastair@d-silva.org>

When calling flush_icache_range with a size >4GB, we were masking
off the upper 32 bits, so we would incorrectly flush a range smaller
than intended.

This patch replaces the 32 bit shifts with 64 bit ones, so that
the full size is accounted for.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Cc: stable@vger.kernel.org
---
 arch/powerpc/kernel/misc_64.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
index b55a7b4cb543..9bc0aa9aeb65 100644
--- a/arch/powerpc/kernel/misc_64.S
+++ b/arch/powerpc/kernel/misc_64.S
@@ -82,7 +82,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
 	subf	r8,r6,r4		/* compute length */
 	add	r8,r8,r5		/* ensure we get enough */
 	lwz	r9,DCACHEL1LOGBLOCKSIZE(r10)	/* Get log-2 of cache block size */
-	srw.	r8,r8,r9		/* compute line count */
+	srd.	r8,r8,r9		/* compute line count */
 	beqlr				/* nothing to do? */
 	mtctr	r8
 1:	dcbst	0,r6
@@ -98,7 +98,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
 	subf	r8,r6,r4		/* compute length */
 	add	r8,r8,r5
 	lwz	r9,ICACHEL1LOGBLOCKSIZE(r10)	/* Get log-2 of Icache block size */
-	srw.	r8,r8,r9		/* compute line count */
+	srd.	r8,r8,r9		/* compute line count */
 	beqlr				/* nothing to do? */
 	mtctr	r8
 2:	icbi	0,r6
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v5 1/6] powerpc: Allow flush_icache_range to work across ranges >4GB
@ 2019-11-04  2:32   ` Alastair D'Silva
  0 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: Greg Kroah-Hartman, David Hildenbrand, linux-kernel, stable,
	Paul Mackerras, Nicholas Piggin, Qian Cai, Thomas Gleixner,
	linuxppc-dev, Andrew Morton, Allison Randal

From: Alastair D'Silva <alastair@d-silva.org>

When calling flush_icache_range with a size >4GB, we were masking
off the upper 32 bits, so we would incorrectly flush a range smaller
than intended.

This patch replaces the 32 bit shifts with 64 bit ones, so that
the full size is accounted for.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Cc: stable@vger.kernel.org
---
 arch/powerpc/kernel/misc_64.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
index b55a7b4cb543..9bc0aa9aeb65 100644
--- a/arch/powerpc/kernel/misc_64.S
+++ b/arch/powerpc/kernel/misc_64.S
@@ -82,7 +82,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
 	subf	r8,r6,r4		/* compute length */
 	add	r8,r8,r5		/* ensure we get enough */
 	lwz	r9,DCACHEL1LOGBLOCKSIZE(r10)	/* Get log-2 of cache block size */
-	srw.	r8,r8,r9		/* compute line count */
+	srd.	r8,r8,r9		/* compute line count */
 	beqlr				/* nothing to do? */
 	mtctr	r8
 1:	dcbst	0,r6
@@ -98,7 +98,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
 	subf	r8,r6,r4		/* compute length */
 	add	r8,r8,r5
 	lwz	r9,ICACHEL1LOGBLOCKSIZE(r10)	/* Get log-2 of Icache block size */
-	srw.	r8,r8,r9		/* compute line count */
+	srd.	r8,r8,r9		/* compute line count */
 	beqlr				/* nothing to do? */
 	mtctr	r8
 2:	icbi	0,r6
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v5 2/6] powerpc: Allow 64bit VDSO __kernel_sync_dicache to work across ranges >4GB
  2019-11-04  2:32 ` Alastair D'Silva
@ 2019-11-04  2:32   ` Alastair D'Silva
  -1 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: stable, Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Christophe Leroy, Thomas Gleixner, Allison Randal, Qian Cai,
	Nicholas Piggin, Greg Kroah-Hartman, Andrew Morton,
	David Hildenbrand, linuxppc-dev, linux-kernel

From: Alastair D'Silva <alastair@d-silva.org>

When calling __kernel_sync_dicache with a size >4GB, we were masking
off the upper 32 bits, so we would incorrectly flush a range smaller
than intended.

This patch replaces the 32 bit shifts with 64 bit ones, so that
the full size is accounted for.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Cc: stable@vger.kernel.org
---
 arch/powerpc/kernel/vdso64/cacheflush.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/vdso64/cacheflush.S b/arch/powerpc/kernel/vdso64/cacheflush.S
index 3f92561a64c4..526f5ba2593e 100644
--- a/arch/powerpc/kernel/vdso64/cacheflush.S
+++ b/arch/powerpc/kernel/vdso64/cacheflush.S
@@ -35,7 +35,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
 	subf	r8,r6,r4		/* compute length */
 	add	r8,r8,r5		/* ensure we get enough */
 	lwz	r9,CFG_DCACHE_LOGBLOCKSZ(r10)
-	srw.	r8,r8,r9		/* compute line count */
+	srd.	r8,r8,r9		/* compute line count */
 	crclr	cr0*4+so
 	beqlr				/* nothing to do? */
 	mtctr	r8
@@ -52,7 +52,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
 	subf	r8,r6,r4		/* compute length */
 	add	r8,r8,r5
 	lwz	r9,CFG_ICACHE_LOGBLOCKSZ(r10)
-	srw.	r8,r8,r9		/* compute line count */
+	srd.	r8,r8,r9		/* compute line count */
 	crclr	cr0*4+so
 	beqlr				/* nothing to do? */
 	mtctr	r8
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v5 2/6] powerpc: Allow 64bit VDSO __kernel_sync_dicache to work across ranges >4GB
@ 2019-11-04  2:32   ` Alastair D'Silva
  0 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: Greg Kroah-Hartman, David Hildenbrand, linux-kernel, stable,
	Paul Mackerras, Nicholas Piggin, Qian Cai, Thomas Gleixner,
	linuxppc-dev, Andrew Morton, Allison Randal

From: Alastair D'Silva <alastair@d-silva.org>

When calling __kernel_sync_dicache with a size >4GB, we were masking
off the upper 32 bits, so we would incorrectly flush a range smaller
than intended.

This patch replaces the 32 bit shifts with 64 bit ones, so that
the full size is accounted for.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Cc: stable@vger.kernel.org
---
 arch/powerpc/kernel/vdso64/cacheflush.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/vdso64/cacheflush.S b/arch/powerpc/kernel/vdso64/cacheflush.S
index 3f92561a64c4..526f5ba2593e 100644
--- a/arch/powerpc/kernel/vdso64/cacheflush.S
+++ b/arch/powerpc/kernel/vdso64/cacheflush.S
@@ -35,7 +35,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
 	subf	r8,r6,r4		/* compute length */
 	add	r8,r8,r5		/* ensure we get enough */
 	lwz	r9,CFG_DCACHE_LOGBLOCKSZ(r10)
-	srw.	r8,r8,r9		/* compute line count */
+	srd.	r8,r8,r9		/* compute line count */
 	crclr	cr0*4+so
 	beqlr				/* nothing to do? */
 	mtctr	r8
@@ -52,7 +52,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
 	subf	r8,r6,r4		/* compute length */
 	add	r8,r8,r5
 	lwz	r9,CFG_ICACHE_LOGBLOCKSZ(r10)
-	srw.	r8,r8,r9		/* compute line count */
+	srd.	r8,r8,r9		/* compute line count */
 	crclr	cr0*4+so
 	beqlr				/* nothing to do? */
 	mtctr	r8
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v5 3/6] powerpc: define helpers to get L1 icache sizes
  2019-11-04  2:32 ` Alastair D'Silva
@ 2019-11-04  2:32   ` Alastair D'Silva
  -1 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Christophe Leroy, Qian Cai, Thomas Gleixner, Nicholas Piggin,
	Greg Kroah-Hartman, Allison Randal, Andrew Morton,
	David Hildenbrand, linuxppc-dev, linux-kernel

From: Alastair D'Silva <alastair@d-silva.org>

This patch adds helpers to retrieve icache sizes, and renames the existing
helpers to make it clear that they are for dcache.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/include/asm/cache.h      | 29 +++++++++++++++++++++++----
 arch/powerpc/include/asm/cacheflush.h | 12 +++++------
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/include/asm/cache.h b/arch/powerpc/include/asm/cache.h
index 45e3137ccd71..afb88754e0e0 100644
--- a/arch/powerpc/include/asm/cache.h
+++ b/arch/powerpc/include/asm/cache.h
@@ -55,25 +55,46 @@ struct ppc64_caches {
 
 extern struct ppc64_caches ppc64_caches;
 
-static inline u32 l1_cache_shift(void)
+static inline u32 l1_dcache_shift(void)
 {
 	return ppc64_caches.l1d.log_block_size;
 }
 
-static inline u32 l1_cache_bytes(void)
+static inline u32 l1_dcache_bytes(void)
 {
 	return ppc64_caches.l1d.block_size;
 }
+
+static inline u32 l1_icache_shift(void)
+{
+	return ppc64_caches.l1i.log_block_size;
+}
+
+static inline u32 l1_icache_bytes(void)
+{
+	return ppc64_caches.l1i.block_size;
+}
 #else
-static inline u32 l1_cache_shift(void)
+static inline u32 l1_dcache_shift(void)
 {
 	return L1_CACHE_SHIFT;
 }
 
-static inline u32 l1_cache_bytes(void)
+static inline u32 l1_dcache_bytes(void)
 {
 	return L1_CACHE_BYTES;
 }
+
+static inline u32 l1_icache_shift(void)
+{
+	return L1_CACHE_SHIFT;
+}
+
+static inline u32 l1_icache_bytes(void)
+{
+	return L1_CACHE_BYTES;
+}
+
 #endif
 #endif /* ! __ASSEMBLY__ */
 
diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
index eef388f2659f..ed57843ef452 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -63,8 +63,8 @@ static inline void __flush_dcache_icache_phys(unsigned long physaddr)
  */
 static inline void flush_dcache_range(unsigned long start, unsigned long stop)
 {
-	unsigned long shift = l1_cache_shift();
-	unsigned long bytes = l1_cache_bytes();
+	unsigned long shift = l1_dcache_shift();
+	unsigned long bytes = l1_dcache_bytes();
 	void *addr = (void *)(start & ~(bytes - 1));
 	unsigned long size = stop - (unsigned long)addr + (bytes - 1);
 	unsigned long i;
@@ -89,8 +89,8 @@ static inline void flush_dcache_range(unsigned long start, unsigned long stop)
  */
 static inline void clean_dcache_range(unsigned long start, unsigned long stop)
 {
-	unsigned long shift = l1_cache_shift();
-	unsigned long bytes = l1_cache_bytes();
+	unsigned long shift = l1_dcache_shift();
+	unsigned long bytes = l1_dcache_bytes();
 	void *addr = (void *)(start & ~(bytes - 1));
 	unsigned long size = stop - (unsigned long)addr + (bytes - 1);
 	unsigned long i;
@@ -108,8 +108,8 @@ static inline void clean_dcache_range(unsigned long start, unsigned long stop)
 static inline void invalidate_dcache_range(unsigned long start,
 					   unsigned long stop)
 {
-	unsigned long shift = l1_cache_shift();
-	unsigned long bytes = l1_cache_bytes();
+	unsigned long shift = l1_dcache_shift();
+	unsigned long bytes = l1_dcache_bytes();
 	void *addr = (void *)(start & ~(bytes - 1));
 	unsigned long size = stop - (unsigned long)addr + (bytes - 1);
 	unsigned long i;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v5 3/6] powerpc: define helpers to get L1 icache sizes
@ 2019-11-04  2:32   ` Alastair D'Silva
  0 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: Greg Kroah-Hartman, David Hildenbrand, linux-kernel,
	Nicholas Piggin, Qian Cai, Paul Mackerras, Thomas Gleixner,
	linuxppc-dev, Andrew Morton, Allison Randal

From: Alastair D'Silva <alastair@d-silva.org>

This patch adds helpers to retrieve icache sizes, and renames the existing
helpers to make it clear that they are for dcache.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/include/asm/cache.h      | 29 +++++++++++++++++++++++----
 arch/powerpc/include/asm/cacheflush.h | 12 +++++------
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/include/asm/cache.h b/arch/powerpc/include/asm/cache.h
index 45e3137ccd71..afb88754e0e0 100644
--- a/arch/powerpc/include/asm/cache.h
+++ b/arch/powerpc/include/asm/cache.h
@@ -55,25 +55,46 @@ struct ppc64_caches {
 
 extern struct ppc64_caches ppc64_caches;
 
-static inline u32 l1_cache_shift(void)
+static inline u32 l1_dcache_shift(void)
 {
 	return ppc64_caches.l1d.log_block_size;
 }
 
-static inline u32 l1_cache_bytes(void)
+static inline u32 l1_dcache_bytes(void)
 {
 	return ppc64_caches.l1d.block_size;
 }
+
+static inline u32 l1_icache_shift(void)
+{
+	return ppc64_caches.l1i.log_block_size;
+}
+
+static inline u32 l1_icache_bytes(void)
+{
+	return ppc64_caches.l1i.block_size;
+}
 #else
-static inline u32 l1_cache_shift(void)
+static inline u32 l1_dcache_shift(void)
 {
 	return L1_CACHE_SHIFT;
 }
 
-static inline u32 l1_cache_bytes(void)
+static inline u32 l1_dcache_bytes(void)
 {
 	return L1_CACHE_BYTES;
 }
+
+static inline u32 l1_icache_shift(void)
+{
+	return L1_CACHE_SHIFT;
+}
+
+static inline u32 l1_icache_bytes(void)
+{
+	return L1_CACHE_BYTES;
+}
+
 #endif
 #endif /* ! __ASSEMBLY__ */
 
diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
index eef388f2659f..ed57843ef452 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -63,8 +63,8 @@ static inline void __flush_dcache_icache_phys(unsigned long physaddr)
  */
 static inline void flush_dcache_range(unsigned long start, unsigned long stop)
 {
-	unsigned long shift = l1_cache_shift();
-	unsigned long bytes = l1_cache_bytes();
+	unsigned long shift = l1_dcache_shift();
+	unsigned long bytes = l1_dcache_bytes();
 	void *addr = (void *)(start & ~(bytes - 1));
 	unsigned long size = stop - (unsigned long)addr + (bytes - 1);
 	unsigned long i;
@@ -89,8 +89,8 @@ static inline void flush_dcache_range(unsigned long start, unsigned long stop)
  */
 static inline void clean_dcache_range(unsigned long start, unsigned long stop)
 {
-	unsigned long shift = l1_cache_shift();
-	unsigned long bytes = l1_cache_bytes();
+	unsigned long shift = l1_dcache_shift();
+	unsigned long bytes = l1_dcache_bytes();
 	void *addr = (void *)(start & ~(bytes - 1));
 	unsigned long size = stop - (unsigned long)addr + (bytes - 1);
 	unsigned long i;
@@ -108,8 +108,8 @@ static inline void clean_dcache_range(unsigned long start, unsigned long stop)
 static inline void invalidate_dcache_range(unsigned long start,
 					   unsigned long stop)
 {
-	unsigned long shift = l1_cache_shift();
-	unsigned long bytes = l1_cache_bytes();
+	unsigned long shift = l1_dcache_shift();
+	unsigned long bytes = l1_dcache_bytes();
 	void *addr = (void *)(start & ~(bytes - 1));
 	unsigned long size = stop - (unsigned long)addr + (bytes - 1);
 	unsigned long i;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v5 4/6] powerpc: Convert flush_icache_range & friends to C
  2019-11-04  2:32 ` Alastair D'Silva
@ 2019-11-04  2:32   ` Alastair D'Silva
  -1 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Allison Randal, Qian Cai, Thomas Gleixner,
	Nicholas Piggin, Greg Kroah-Hartman, Andrew Morton,
	David Hildenbrand, linuxppc-dev, linux-kernel

From: Alastair D'Silva <alastair@d-silva.org>

Similar to commit 22e9c88d486a
("powerpc/64: reuse PPC32 static inline flush_dcache_range()")
this patch converts the following ASM symbols to C:
    flush_icache_range()
    __flush_dcache_icache()
    __flush_dcache_icache_phys()

This was done as we discovered a long-standing bug where the length of the
range was truncated due to using a 32 bit shift instead of a 64 bit one.

By converting these functions to C, it becomes easier to maintain.

flush_dcache_icache_phys() retains a critical assembler section as we must
ensure there are no memory accesses while the data MMU is disabled
(authored by Christophe Leroy). Since this has no external callers, it has
also been made static, allowing the compiler to inline it within
flush_dcache_icache_page().

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/include/asm/cache.h      |  26 ++---
 arch/powerpc/include/asm/cacheflush.h |  24 ++--
 arch/powerpc/kernel/misc_32.S         | 120 --------------------
 arch/powerpc/kernel/misc_64.S         | 102 -----------------
 arch/powerpc/mm/mem.c                 | 151 +++++++++++++++++++++++++-
 5 files changed, 172 insertions(+), 251 deletions(-)

diff --git a/arch/powerpc/include/asm/cache.h b/arch/powerpc/include/asm/cache.h
index afb88754e0e0..b809095cb1ba 100644
--- a/arch/powerpc/include/asm/cache.h
+++ b/arch/powerpc/include/asm/cache.h
@@ -98,20 +98,7 @@ static inline u32 l1_icache_bytes(void)
 #endif
 #endif /* ! __ASSEMBLY__ */
 
-#if defined(__ASSEMBLY__)
-/*
- * For a snooping icache, we still need a dummy icbi to purge all the
- * prefetched instructions from the ifetch buffers. We also need a sync
- * before the icbi to order the the actual stores to memory that might
- * have modified instructions with the icbi.
- */
-#define PURGE_PREFETCHED_INS	\
-	sync;			\
-	icbi	0,r3;		\
-	sync;			\
-	isync
-
-#else
+#if !defined(__ASSEMBLY__)
 #define __read_mostly __attribute__((__section__(".data..read_mostly")))
 
 #ifdef CONFIG_PPC_BOOK3S_32
@@ -145,6 +132,17 @@ static inline void dcbst(void *addr)
 {
 	__asm__ __volatile__ ("dcbst 0, %0" : : "r"(addr) : "memory");
 }
+
+static inline void icbi(void *addr)
+{
+	asm volatile ("icbi 0, %0" : : "r"(addr) : "memory");
+}
+
+static inline void iccci(void *addr)
+{
+	asm volatile ("iccci 0, %0" : : "r"(addr) : "memory");
+}
+
 #endif /* !__ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_CACHE_H */
diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
index ed57843ef452..4a1c9f0200e1 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -42,24 +42,20 @@ extern void flush_dcache_page(struct page *page);
 #define flush_dcache_mmap_lock(mapping)		do { } while (0)
 #define flush_dcache_mmap_unlock(mapping)	do { } while (0)
 
-extern void flush_icache_range(unsigned long, unsigned long);
+void flush_icache_range(unsigned long start, unsigned long stop);
 extern void flush_icache_user_range(struct vm_area_struct *vma,
 				    struct page *page, unsigned long addr,
 				    int len);
-extern void __flush_dcache_icache(void *page_va);
 extern void flush_dcache_icache_page(struct page *page);
-#if defined(CONFIG_PPC32) && !defined(CONFIG_BOOKE)
-extern void __flush_dcache_icache_phys(unsigned long physaddr);
-#else
-static inline void __flush_dcache_icache_phys(unsigned long physaddr)
-{
-	BUG();
-}
-#endif
-
-/*
- * Write any modified data cache blocks out to memory and invalidate them.
- * Does not invalidate the corresponding instruction cache blocks.
+void __flush_dcache_icache(void *page);
+
+/**
+ * flush_dcache_range(): Write any modified data cache blocks out to memory and
+ * invalidate them. Does not invalidate the corresponding instruction cache
+ * blocks.
+ *
+ * @start: the start address
+ * @stop: the stop address (exclusive)
  */
 static inline void flush_dcache_range(unsigned long start, unsigned long stop)
 {
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index 82df4b09e79f..f4e4a1926a7a 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -316,126 +316,6 @@ _GLOBAL(flush_instruction_cache)
 EXPORT_SYMBOL(flush_instruction_cache)
 #endif /* CONFIG_PPC_8xx */
 
-/*
- * Write any modified data cache blocks out to memory
- * and invalidate the corresponding instruction cache blocks.
- * This is a no-op on the 601.
- *
- * flush_icache_range(unsigned long start, unsigned long stop)
- */
-_GLOBAL(flush_icache_range)
-#if defined(CONFIG_PPC_BOOK3S_601) || defined(CONFIG_E200)
-	PURGE_PREFETCHED_INS
-	blr				/* for 601 and e200, do nothing */
-#else
-	rlwinm	r3,r3,0,0,31 - L1_CACHE_SHIFT
-	subf	r4,r3,r4
-	addi	r4,r4,L1_CACHE_BYTES - 1
-	srwi.	r4,r4,L1_CACHE_SHIFT
-	beqlr
-	mtctr	r4
-	mr	r6,r3
-1:	dcbst	0,r3
-	addi	r3,r3,L1_CACHE_BYTES
-	bdnz	1b
-	sync				/* wait for dcbst's to get to ram */
-#ifndef CONFIG_44x
-	mtctr	r4
-2:	icbi	0,r6
-	addi	r6,r6,L1_CACHE_BYTES
-	bdnz	2b
-#else
-	/* Flash invalidate on 44x because we are passed kmapped addresses and
-	   this doesn't work for userspace pages due to the virtually tagged
-	   icache.  Sigh. */
-	iccci	0, r0
-#endif
-	sync				/* additional sync needed on g4 */
-	isync
-	blr
-#endif
-_ASM_NOKPROBE_SYMBOL(flush_icache_range)
-EXPORT_SYMBOL(flush_icache_range)
-
-/*
- * Flush a particular page from the data cache to RAM.
- * Note: this is necessary because the instruction cache does *not*
- * snoop from the data cache.
- * This is a no-op on the 601 and e200 which have a unified cache.
- *
- *	void __flush_dcache_icache(void *page)
- */
-_GLOBAL(__flush_dcache_icache)
-#if defined(CONFIG_PPC_BOOK3S_601) || defined(CONFIG_E200)
-	PURGE_PREFETCHED_INS
-	blr
-#else
-	rlwinm	r3,r3,0,0,31-PAGE_SHIFT		/* Get page base address */
-	li	r4,PAGE_SIZE/L1_CACHE_BYTES	/* Number of lines in a page */
-	mtctr	r4
-	mr	r6,r3
-0:	dcbst	0,r3				/* Write line to ram */
-	addi	r3,r3,L1_CACHE_BYTES
-	bdnz	0b
-	sync
-#ifdef CONFIG_44x
-	/* We don't flush the icache on 44x. Those have a virtual icache
-	 * and we don't have access to the virtual address here (it's
-	 * not the page vaddr but where it's mapped in user space). The
-	 * flushing of the icache on these is handled elsewhere, when
-	 * a change in the address space occurs, before returning to
-	 * user space
-	 */
-BEGIN_MMU_FTR_SECTION
-	blr
-END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_44x)
-#endif /* CONFIG_44x */
-	mtctr	r4
-1:	icbi	0,r6
-	addi	r6,r6,L1_CACHE_BYTES
-	bdnz	1b
-	sync
-	isync
-	blr
-#endif
-
-#ifndef CONFIG_BOOKE
-/*
- * Flush a particular page from the data cache to RAM, identified
- * by its physical address.  We turn off the MMU so we can just use
- * the physical address (this may be a highmem page without a kernel
- * mapping).
- *
- *	void __flush_dcache_icache_phys(unsigned long physaddr)
- */
-_GLOBAL(__flush_dcache_icache_phys)
-#if defined(CONFIG_PPC_BOOK3S_601) || defined(CONFIG_E200)
-	PURGE_PREFETCHED_INS
-	blr					/* for 601 and e200, do nothing */
-#else
-	mfmsr	r10
-	rlwinm	r0,r10,0,28,26			/* clear DR */
-	mtmsr	r0
-	isync
-	rlwinm	r3,r3,0,0,31-PAGE_SHIFT		/* Get page base address */
-	li	r4,PAGE_SIZE/L1_CACHE_BYTES	/* Number of lines in a page */
-	mtctr	r4
-	mr	r6,r3
-0:	dcbst	0,r3				/* Write line to ram */
-	addi	r3,r3,L1_CACHE_BYTES
-	bdnz	0b
-	sync
-	mtctr	r4
-1:	icbi	0,r6
-	addi	r6,r6,L1_CACHE_BYTES
-	bdnz	1b
-	sync
-	mtmsr	r10				/* restore DR */
-	isync
-	blr
-#endif
-#endif /* CONFIG_BOOKE */
-
 /*
  * Copy a whole page.  We use the dcbz instruction on the destination
  * to reduce memory traffic (it eliminates the unnecessary reads of
diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
index 9bc0aa9aeb65..ff20c253f273 100644
--- a/arch/powerpc/kernel/misc_64.S
+++ b/arch/powerpc/kernel/misc_64.S
@@ -49,108 +49,6 @@ _GLOBAL(call_do_irq)
 	mtlr	r0
 	blr
 
-	.section	".toc","aw"
-PPC64_CACHES:
-	.tc		ppc64_caches[TC],ppc64_caches
-	.section	".text"
-
-/*
- * Write any modified data cache blocks out to memory
- * and invalidate the corresponding instruction cache blocks.
- *
- * flush_icache_range(unsigned long start, unsigned long stop)
- *
- *   flush all bytes from start through stop-1 inclusive
- */
-
-_GLOBAL_TOC(flush_icache_range)
-BEGIN_FTR_SECTION
-	PURGE_PREFETCHED_INS
-	blr
-END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
-/*
- * Flush the data cache to memory 
- * 
- * Different systems have different cache line sizes
- * and in some cases i-cache and d-cache line sizes differ from
- * each other.
- */
- 	ld	r10,PPC64_CACHES@toc(r2)
-	lwz	r7,DCACHEL1BLOCKSIZE(r10)/* Get cache block size */
-	addi	r5,r7,-1
-	andc	r6,r3,r5		/* round low to line bdy */
-	subf	r8,r6,r4		/* compute length */
-	add	r8,r8,r5		/* ensure we get enough */
-	lwz	r9,DCACHEL1LOGBLOCKSIZE(r10)	/* Get log-2 of cache block size */
-	srd.	r8,r8,r9		/* compute line count */
-	beqlr				/* nothing to do? */
-	mtctr	r8
-1:	dcbst	0,r6
-	add	r6,r6,r7
-	bdnz	1b
-	sync
-
-/* Now invalidate the instruction cache */
-	
-	lwz	r7,ICACHEL1BLOCKSIZE(r10)	/* Get Icache block size */
-	addi	r5,r7,-1
-	andc	r6,r3,r5		/* round low to line bdy */
-	subf	r8,r6,r4		/* compute length */
-	add	r8,r8,r5
-	lwz	r9,ICACHEL1LOGBLOCKSIZE(r10)	/* Get log-2 of Icache block size */
-	srd.	r8,r8,r9		/* compute line count */
-	beqlr				/* nothing to do? */
-	mtctr	r8
-2:	icbi	0,r6
-	add	r6,r6,r7
-	bdnz	2b
-	isync
-	blr
-_ASM_NOKPROBE_SYMBOL(flush_icache_range)
-EXPORT_SYMBOL(flush_icache_range)
-
-/*
- * Flush a particular page from the data cache to RAM.
- * Note: this is necessary because the instruction cache does *not*
- * snoop from the data cache.
- *
- *	void __flush_dcache_icache(void *page)
- */
-_GLOBAL(__flush_dcache_icache)
-/*
- * Flush the data cache to memory 
- * 
- * Different systems have different cache line sizes
- */
-
-BEGIN_FTR_SECTION
-	PURGE_PREFETCHED_INS
-	blr
-END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
-
-/* Flush the dcache */
- 	ld	r7,PPC64_CACHES@toc(r2)
-	clrrdi	r3,r3,PAGE_SHIFT           	    /* Page align */
-	lwz	r4,DCACHEL1BLOCKSPERPAGE(r7)	/* Get # dcache blocks per page */
-	lwz	r5,DCACHEL1BLOCKSIZE(r7)	/* Get dcache block size */
-	mr	r6,r3
-	mtctr	r4
-0:	dcbst	0,r6
-	add	r6,r6,r5
-	bdnz	0b
-	sync
-
-/* Now invalidate the icache */	
-
-	lwz	r4,ICACHEL1BLOCKSPERPAGE(r7)	/* Get # icache blocks per page */
-	lwz	r5,ICACHEL1BLOCKSIZE(r7)	/* Get icache block size */
-	mtctr	r4
-1:	icbi	0,r3
-	add	r3,r3,r5
-	bdnz	1b
-	isync
-	blr
-
 _GLOBAL(__bswapdi2)
 EXPORT_SYMBOL(__bswapdi2)
 	srdi	r8,r3,32
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index be941d382c8d..54d61ba15e93 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -318,6 +318,120 @@ void free_initmem(void)
 	free_initmem_default(POISON_FREE_INITMEM);
 }
 
+/**
+ * flush_coherent_icache() - if a CPU has a coherent icache, flush it
+ * @addr: The base address to use (can be any valid address, the whole cache will be flushed)
+ * Return true if the cache was flushed, false otherwise
+ */
+static inline bool flush_coherent_icache(unsigned long addr)
+{
+	/*
+	 * For a snooping icache, we still need a dummy icbi to purge all the
+	 * prefetched instructions from the ifetch buffers. We also need a sync
+	 * before the icbi to order the the actual stores to memory that might
+	 * have modified instructions with the icbi.
+	 */
+	if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
+		mb(); /* sync */
+		icbi((void *)addr);
+		mb(); /* sync */
+		isync();
+		return true;
+	}
+
+	return false;
+}
+
+/**
+ * invalidate_icache_range() - Flush the icache by issuing icbi across an address range
+ * @start: the start address
+ * @stop: the stop address (exclusive)
+ */
+static void invalidate_icache_range(unsigned long start, unsigned long stop)
+{
+	unsigned long shift = l1_icache_shift();
+	unsigned long bytes = l1_icache_bytes();
+	char *addr = (char *)(start & ~(bytes - 1));
+	unsigned long size = stop - (unsigned long)addr + (bytes - 1);
+	unsigned long i;
+
+	for (i = 0; i < size >> shift; i++, addr += bytes)
+		icbi(addr);
+
+	mb(); /* sync */
+	isync();
+}
+
+/**
+ * flush_icache_range: Write any modified data cache blocks out to memory
+ * and invalidate the corresponding blocks in the instruction cache
+ *
+ * Generic code will call this after writing memory, before executing from it.
+ *
+ * @start: the start address
+ * @stop: the stop address (exclusive)
+ */
+void flush_icache_range(unsigned long start, unsigned long stop)
+{
+	if (flush_coherent_icache(start))
+		return;
+
+	clean_dcache_range(start, stop);
+
+	if (IS_ENABLED(CONFIG_44x)) {
+		/*
+		 * Flash invalidate on 44x because we are passed kmapped
+		 * addresses and this doesn't work for userspace pages due to
+		 * the virtually tagged icache.
+		 */
+		iccci((void *)start);
+		mb(); /* sync */
+		isync();
+	} else
+		invalidate_icache_range(start, stop);
+}
+EXPORT_SYMBOL(flush_icache_range);
+
+#if !defined(CONFIG_PPC_8xx) && !defined(CONFIG_PPC64)
+/**
+ * flush_dcache_icache_phys() - Flush a page by it's physical address
+ * @physaddr: the physical address of the page
+ */
+static void flush_dcache_icache_phys(unsigned long physaddr)
+{
+	unsigned long bytes = l1_dcache_bytes();
+	unsigned long nb = PAGE_SIZE / bytes;
+	unsigned long addr = physaddr & PAGE_MASK;
+	unsigned long msr, msr0;
+	unsigned long loop1 = addr, loop2 = addr;
+
+	msr0 = mfmsr();
+	msr = msr0 & ~MSR_DR;
+	/*
+	 * This must remain as ASM to prevent potential memory accesses
+	 * while the data MMU is disabled
+	 */
+	asm volatile(
+		"   mtctr %2;\n"
+		"   mtmsr %3;\n"
+		"   isync;\n"
+		"0: dcbst   0, %0;\n"
+		"   addi    %0, %0, %4;\n"
+		"   bdnz    0b;\n"
+		"   sync;\n"
+		"   mtctr %2;\n"
+		"1: icbi    0, %1;\n"
+		"   addi    %1, %1, %4;\n"
+		"   bdnz    1b;\n"
+		"   sync;\n"
+		"   mtmsr %5;\n"
+		"   isync;\n"
+		: "+&r" (loop1), "+&r" (loop2)
+		: "r" (nb), "r" (msr), "i" (bytes), "r" (msr0)
+		: "ctr", "memory");
+}
+#endif // !defined(CONFIG_PPC_8xx) && !defined(CONFIG_PPC64)
+
 /*
  * This is called when a page has been modified by the kernel.
  * It just marks the page as not i-cache clean.  We do the i-cache
@@ -350,12 +464,47 @@ void flush_dcache_icache_page(struct page *page)
 		__flush_dcache_icache(start);
 		kunmap_atomic(start);
 	} else {
-		__flush_dcache_icache_phys(page_to_pfn(page) << PAGE_SHIFT);
+		unsigned long addr = page_to_pfn(page) << PAGE_SHIFT;
+
+		if (flush_coherent_icache((void *)addr))
+			return;
+		flush_dcache_icache_phys(addr);
 	}
 #endif
 }
 EXPORT_SYMBOL(flush_dcache_icache_page);
 
+/**
+ * __flush_dcache_icache(): Flush a particular page from the data cache to RAM.
+ * Note: this is necessary because the instruction cache does *not*
+ * snoop from the data cache.
+ *
+ * @page: the address of the page to flush
+ */
+void __flush_dcache_icache(void *page)
+{
+	u64 addr = (u64)page;
+
+	if (flush_coherent_icache(addr))
+		return;
+
+	clean_dcache_range(addr, addr + PAGE_SIZE);
+
+	/*
+	 * We don't flush the icache on 44x. Those have a virtual icache and we
+	 * don't have access to the virtual address here (it's not the page
+	 * vaddr but where it's mapped in user space). The flushing of the
+	 * icache on these is handled elsewhere, when a change in the address
+	 * space occurs, before returning to user space.
+	 */
+
+	if (cpu_has_feature(MMU_FTR_TYPE_44x))
+		return;
+
+	invalidate_icache_range(addr, addr + PAGE_SIZE);
+}
+EXPORT_SYMBOL(__flush_dcache_icache);
+
 void clear_user_page(void *page, unsigned long vaddr, struct page *pg)
 {
 	clear_page(page);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v5 4/6] powerpc: Convert flush_icache_range & friends to C
@ 2019-11-04  2:32   ` Alastair D'Silva
  0 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: Greg Kroah-Hartman, David Hildenbrand, linux-kernel,
	Nicholas Piggin, Qian Cai, Paul Mackerras, Thomas Gleixner,
	linuxppc-dev, Andrew Morton, Allison Randal

From: Alastair D'Silva <alastair@d-silva.org>

Similar to commit 22e9c88d486a
("powerpc/64: reuse PPC32 static inline flush_dcache_range()")
this patch converts the following ASM symbols to C:
    flush_icache_range()
    __flush_dcache_icache()
    __flush_dcache_icache_phys()

This was done as we discovered a long-standing bug where the length of the
range was truncated due to using a 32 bit shift instead of a 64 bit one.

By converting these functions to C, it becomes easier to maintain.

flush_dcache_icache_phys() retains a critical assembler section as we must
ensure there are no memory accesses while the data MMU is disabled
(authored by Christophe Leroy). Since this has no external callers, it has
also been made static, allowing the compiler to inline it within
flush_dcache_icache_page().

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/include/asm/cache.h      |  26 ++---
 arch/powerpc/include/asm/cacheflush.h |  24 ++--
 arch/powerpc/kernel/misc_32.S         | 120 --------------------
 arch/powerpc/kernel/misc_64.S         | 102 -----------------
 arch/powerpc/mm/mem.c                 | 151 +++++++++++++++++++++++++-
 5 files changed, 172 insertions(+), 251 deletions(-)

diff --git a/arch/powerpc/include/asm/cache.h b/arch/powerpc/include/asm/cache.h
index afb88754e0e0..b809095cb1ba 100644
--- a/arch/powerpc/include/asm/cache.h
+++ b/arch/powerpc/include/asm/cache.h
@@ -98,20 +98,7 @@ static inline u32 l1_icache_bytes(void)
 #endif
 #endif /* ! __ASSEMBLY__ */
 
-#if defined(__ASSEMBLY__)
-/*
- * For a snooping icache, we still need a dummy icbi to purge all the
- * prefetched instructions from the ifetch buffers. We also need a sync
- * before the icbi to order the the actual stores to memory that might
- * have modified instructions with the icbi.
- */
-#define PURGE_PREFETCHED_INS	\
-	sync;			\
-	icbi	0,r3;		\
-	sync;			\
-	isync
-
-#else
+#if !defined(__ASSEMBLY__)
 #define __read_mostly __attribute__((__section__(".data..read_mostly")))
 
 #ifdef CONFIG_PPC_BOOK3S_32
@@ -145,6 +132,17 @@ static inline void dcbst(void *addr)
 {
 	__asm__ __volatile__ ("dcbst 0, %0" : : "r"(addr) : "memory");
 }
+
+static inline void icbi(void *addr)
+{
+	asm volatile ("icbi 0, %0" : : "r"(addr) : "memory");
+}
+
+static inline void iccci(void *addr)
+{
+	asm volatile ("iccci 0, %0" : : "r"(addr) : "memory");
+}
+
 #endif /* !__ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_CACHE_H */
diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
index ed57843ef452..4a1c9f0200e1 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -42,24 +42,20 @@ extern void flush_dcache_page(struct page *page);
 #define flush_dcache_mmap_lock(mapping)		do { } while (0)
 #define flush_dcache_mmap_unlock(mapping)	do { } while (0)
 
-extern void flush_icache_range(unsigned long, unsigned long);
+void flush_icache_range(unsigned long start, unsigned long stop);
 extern void flush_icache_user_range(struct vm_area_struct *vma,
 				    struct page *page, unsigned long addr,
 				    int len);
-extern void __flush_dcache_icache(void *page_va);
 extern void flush_dcache_icache_page(struct page *page);
-#if defined(CONFIG_PPC32) && !defined(CONFIG_BOOKE)
-extern void __flush_dcache_icache_phys(unsigned long physaddr);
-#else
-static inline void __flush_dcache_icache_phys(unsigned long physaddr)
-{
-	BUG();
-}
-#endif
-
-/*
- * Write any modified data cache blocks out to memory and invalidate them.
- * Does not invalidate the corresponding instruction cache blocks.
+void __flush_dcache_icache(void *page);
+
+/**
+ * flush_dcache_range(): Write any modified data cache blocks out to memory and
+ * invalidate them. Does not invalidate the corresponding instruction cache
+ * blocks.
+ *
+ * @start: the start address
+ * @stop: the stop address (exclusive)
  */
 static inline void flush_dcache_range(unsigned long start, unsigned long stop)
 {
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index 82df4b09e79f..f4e4a1926a7a 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -316,126 +316,6 @@ _GLOBAL(flush_instruction_cache)
 EXPORT_SYMBOL(flush_instruction_cache)
 #endif /* CONFIG_PPC_8xx */
 
-/*
- * Write any modified data cache blocks out to memory
- * and invalidate the corresponding instruction cache blocks.
- * This is a no-op on the 601.
- *
- * flush_icache_range(unsigned long start, unsigned long stop)
- */
-_GLOBAL(flush_icache_range)
-#if defined(CONFIG_PPC_BOOK3S_601) || defined(CONFIG_E200)
-	PURGE_PREFETCHED_INS
-	blr				/* for 601 and e200, do nothing */
-#else
-	rlwinm	r3,r3,0,0,31 - L1_CACHE_SHIFT
-	subf	r4,r3,r4
-	addi	r4,r4,L1_CACHE_BYTES - 1
-	srwi.	r4,r4,L1_CACHE_SHIFT
-	beqlr
-	mtctr	r4
-	mr	r6,r3
-1:	dcbst	0,r3
-	addi	r3,r3,L1_CACHE_BYTES
-	bdnz	1b
-	sync				/* wait for dcbst's to get to ram */
-#ifndef CONFIG_44x
-	mtctr	r4
-2:	icbi	0,r6
-	addi	r6,r6,L1_CACHE_BYTES
-	bdnz	2b
-#else
-	/* Flash invalidate on 44x because we are passed kmapped addresses and
-	   this doesn't work for userspace pages due to the virtually tagged
-	   icache.  Sigh. */
-	iccci	0, r0
-#endif
-	sync				/* additional sync needed on g4 */
-	isync
-	blr
-#endif
-_ASM_NOKPROBE_SYMBOL(flush_icache_range)
-EXPORT_SYMBOL(flush_icache_range)
-
-/*
- * Flush a particular page from the data cache to RAM.
- * Note: this is necessary because the instruction cache does *not*
- * snoop from the data cache.
- * This is a no-op on the 601 and e200 which have a unified cache.
- *
- *	void __flush_dcache_icache(void *page)
- */
-_GLOBAL(__flush_dcache_icache)
-#if defined(CONFIG_PPC_BOOK3S_601) || defined(CONFIG_E200)
-	PURGE_PREFETCHED_INS
-	blr
-#else
-	rlwinm	r3,r3,0,0,31-PAGE_SHIFT		/* Get page base address */
-	li	r4,PAGE_SIZE/L1_CACHE_BYTES	/* Number of lines in a page */
-	mtctr	r4
-	mr	r6,r3
-0:	dcbst	0,r3				/* Write line to ram */
-	addi	r3,r3,L1_CACHE_BYTES
-	bdnz	0b
-	sync
-#ifdef CONFIG_44x
-	/* We don't flush the icache on 44x. Those have a virtual icache
-	 * and we don't have access to the virtual address here (it's
-	 * not the page vaddr but where it's mapped in user space). The
-	 * flushing of the icache on these is handled elsewhere, when
-	 * a change in the address space occurs, before returning to
-	 * user space
-	 */
-BEGIN_MMU_FTR_SECTION
-	blr
-END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_44x)
-#endif /* CONFIG_44x */
-	mtctr	r4
-1:	icbi	0,r6
-	addi	r6,r6,L1_CACHE_BYTES
-	bdnz	1b
-	sync
-	isync
-	blr
-#endif
-
-#ifndef CONFIG_BOOKE
-/*
- * Flush a particular page from the data cache to RAM, identified
- * by its physical address.  We turn off the MMU so we can just use
- * the physical address (this may be a highmem page without a kernel
- * mapping).
- *
- *	void __flush_dcache_icache_phys(unsigned long physaddr)
- */
-_GLOBAL(__flush_dcache_icache_phys)
-#if defined(CONFIG_PPC_BOOK3S_601) || defined(CONFIG_E200)
-	PURGE_PREFETCHED_INS
-	blr					/* for 601 and e200, do nothing */
-#else
-	mfmsr	r10
-	rlwinm	r0,r10,0,28,26			/* clear DR */
-	mtmsr	r0
-	isync
-	rlwinm	r3,r3,0,0,31-PAGE_SHIFT		/* Get page base address */
-	li	r4,PAGE_SIZE/L1_CACHE_BYTES	/* Number of lines in a page */
-	mtctr	r4
-	mr	r6,r3
-0:	dcbst	0,r3				/* Write line to ram */
-	addi	r3,r3,L1_CACHE_BYTES
-	bdnz	0b
-	sync
-	mtctr	r4
-1:	icbi	0,r6
-	addi	r6,r6,L1_CACHE_BYTES
-	bdnz	1b
-	sync
-	mtmsr	r10				/* restore DR */
-	isync
-	blr
-#endif
-#endif /* CONFIG_BOOKE */
-
 /*
  * Copy a whole page.  We use the dcbz instruction on the destination
  * to reduce memory traffic (it eliminates the unnecessary reads of
diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
index 9bc0aa9aeb65..ff20c253f273 100644
--- a/arch/powerpc/kernel/misc_64.S
+++ b/arch/powerpc/kernel/misc_64.S
@@ -49,108 +49,6 @@ _GLOBAL(call_do_irq)
 	mtlr	r0
 	blr
 
-	.section	".toc","aw"
-PPC64_CACHES:
-	.tc		ppc64_caches[TC],ppc64_caches
-	.section	".text"
-
-/*
- * Write any modified data cache blocks out to memory
- * and invalidate the corresponding instruction cache blocks.
- *
- * flush_icache_range(unsigned long start, unsigned long stop)
- *
- *   flush all bytes from start through stop-1 inclusive
- */
-
-_GLOBAL_TOC(flush_icache_range)
-BEGIN_FTR_SECTION
-	PURGE_PREFETCHED_INS
-	blr
-END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
-/*
- * Flush the data cache to memory 
- * 
- * Different systems have different cache line sizes
- * and in some cases i-cache and d-cache line sizes differ from
- * each other.
- */
- 	ld	r10,PPC64_CACHES@toc(r2)
-	lwz	r7,DCACHEL1BLOCKSIZE(r10)/* Get cache block size */
-	addi	r5,r7,-1
-	andc	r6,r3,r5		/* round low to line bdy */
-	subf	r8,r6,r4		/* compute length */
-	add	r8,r8,r5		/* ensure we get enough */
-	lwz	r9,DCACHEL1LOGBLOCKSIZE(r10)	/* Get log-2 of cache block size */
-	srd.	r8,r8,r9		/* compute line count */
-	beqlr				/* nothing to do? */
-	mtctr	r8
-1:	dcbst	0,r6
-	add	r6,r6,r7
-	bdnz	1b
-	sync
-
-/* Now invalidate the instruction cache */
-	
-	lwz	r7,ICACHEL1BLOCKSIZE(r10)	/* Get Icache block size */
-	addi	r5,r7,-1
-	andc	r6,r3,r5		/* round low to line bdy */
-	subf	r8,r6,r4		/* compute length */
-	add	r8,r8,r5
-	lwz	r9,ICACHEL1LOGBLOCKSIZE(r10)	/* Get log-2 of Icache block size */
-	srd.	r8,r8,r9		/* compute line count */
-	beqlr				/* nothing to do? */
-	mtctr	r8
-2:	icbi	0,r6
-	add	r6,r6,r7
-	bdnz	2b
-	isync
-	blr
-_ASM_NOKPROBE_SYMBOL(flush_icache_range)
-EXPORT_SYMBOL(flush_icache_range)
-
-/*
- * Flush a particular page from the data cache to RAM.
- * Note: this is necessary because the instruction cache does *not*
- * snoop from the data cache.
- *
- *	void __flush_dcache_icache(void *page)
- */
-_GLOBAL(__flush_dcache_icache)
-/*
- * Flush the data cache to memory 
- * 
- * Different systems have different cache line sizes
- */
-
-BEGIN_FTR_SECTION
-	PURGE_PREFETCHED_INS
-	blr
-END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
-
-/* Flush the dcache */
- 	ld	r7,PPC64_CACHES@toc(r2)
-	clrrdi	r3,r3,PAGE_SHIFT           	    /* Page align */
-	lwz	r4,DCACHEL1BLOCKSPERPAGE(r7)	/* Get # dcache blocks per page */
-	lwz	r5,DCACHEL1BLOCKSIZE(r7)	/* Get dcache block size */
-	mr	r6,r3
-	mtctr	r4
-0:	dcbst	0,r6
-	add	r6,r6,r5
-	bdnz	0b
-	sync
-
-/* Now invalidate the icache */	
-
-	lwz	r4,ICACHEL1BLOCKSPERPAGE(r7)	/* Get # icache blocks per page */
-	lwz	r5,ICACHEL1BLOCKSIZE(r7)	/* Get icache block size */
-	mtctr	r4
-1:	icbi	0,r3
-	add	r3,r3,r5
-	bdnz	1b
-	isync
-	blr
-
 _GLOBAL(__bswapdi2)
 EXPORT_SYMBOL(__bswapdi2)
 	srdi	r8,r3,32
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index be941d382c8d..54d61ba15e93 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -318,6 +318,120 @@ void free_initmem(void)
 	free_initmem_default(POISON_FREE_INITMEM);
 }
 
+/**
+ * flush_coherent_icache() - if a CPU has a coherent icache, flush it
+ * @addr: The base address to use (can be any valid address, the whole cache will be flushed)
+ * Return true if the cache was flushed, false otherwise
+ */
+static inline bool flush_coherent_icache(unsigned long addr)
+{
+	/*
+	 * For a snooping icache, we still need a dummy icbi to purge all the
+	 * prefetched instructions from the ifetch buffers. We also need a sync
+	 * before the icbi to order the the actual stores to memory that might
+	 * have modified instructions with the icbi.
+	 */
+	if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
+		mb(); /* sync */
+		icbi((void *)addr);
+		mb(); /* sync */
+		isync();
+		return true;
+	}
+
+	return false;
+}
+
+/**
+ * invalidate_icache_range() - Flush the icache by issuing icbi across an address range
+ * @start: the start address
+ * @stop: the stop address (exclusive)
+ */
+static void invalidate_icache_range(unsigned long start, unsigned long stop)
+{
+	unsigned long shift = l1_icache_shift();
+	unsigned long bytes = l1_icache_bytes();
+	char *addr = (char *)(start & ~(bytes - 1));
+	unsigned long size = stop - (unsigned long)addr + (bytes - 1);
+	unsigned long i;
+
+	for (i = 0; i < size >> shift; i++, addr += bytes)
+		icbi(addr);
+
+	mb(); /* sync */
+	isync();
+}
+
+/**
+ * flush_icache_range: Write any modified data cache blocks out to memory
+ * and invalidate the corresponding blocks in the instruction cache
+ *
+ * Generic code will call this after writing memory, before executing from it.
+ *
+ * @start: the start address
+ * @stop: the stop address (exclusive)
+ */
+void flush_icache_range(unsigned long start, unsigned long stop)
+{
+	if (flush_coherent_icache(start))
+		return;
+
+	clean_dcache_range(start, stop);
+
+	if (IS_ENABLED(CONFIG_44x)) {
+		/*
+		 * Flash invalidate on 44x because we are passed kmapped
+		 * addresses and this doesn't work for userspace pages due to
+		 * the virtually tagged icache.
+		 */
+		iccci((void *)start);
+		mb(); /* sync */
+		isync();
+	} else
+		invalidate_icache_range(start, stop);
+}
+EXPORT_SYMBOL(flush_icache_range);
+
+#if !defined(CONFIG_PPC_8xx) && !defined(CONFIG_PPC64)
+/**
+ * flush_dcache_icache_phys() - Flush a page by it's physical address
+ * @physaddr: the physical address of the page
+ */
+static void flush_dcache_icache_phys(unsigned long physaddr)
+{
+	unsigned long bytes = l1_dcache_bytes();
+	unsigned long nb = PAGE_SIZE / bytes;
+	unsigned long addr = physaddr & PAGE_MASK;
+	unsigned long msr, msr0;
+	unsigned long loop1 = addr, loop2 = addr;
+
+	msr0 = mfmsr();
+	msr = msr0 & ~MSR_DR;
+	/*
+	 * This must remain as ASM to prevent potential memory accesses
+	 * while the data MMU is disabled
+	 */
+	asm volatile(
+		"   mtctr %2;\n"
+		"   mtmsr %3;\n"
+		"   isync;\n"
+		"0: dcbst   0, %0;\n"
+		"   addi    %0, %0, %4;\n"
+		"   bdnz    0b;\n"
+		"   sync;\n"
+		"   mtctr %2;\n"
+		"1: icbi    0, %1;\n"
+		"   addi    %1, %1, %4;\n"
+		"   bdnz    1b;\n"
+		"   sync;\n"
+		"   mtmsr %5;\n"
+		"   isync;\n"
+		: "+&r" (loop1), "+&r" (loop2)
+		: "r" (nb), "r" (msr), "i" (bytes), "r" (msr0)
+		: "ctr", "memory");
+}
+#endif // !defined(CONFIG_PPC_8xx) && !defined(CONFIG_PPC64)
+
 /*
  * This is called when a page has been modified by the kernel.
  * It just marks the page as not i-cache clean.  We do the i-cache
@@ -350,12 +464,47 @@ void flush_dcache_icache_page(struct page *page)
 		__flush_dcache_icache(start);
 		kunmap_atomic(start);
 	} else {
-		__flush_dcache_icache_phys(page_to_pfn(page) << PAGE_SHIFT);
+		unsigned long addr = page_to_pfn(page) << PAGE_SHIFT;
+
+		if (flush_coherent_icache((void *)addr))
+			return;
+		flush_dcache_icache_phys(addr);
 	}
 #endif
 }
 EXPORT_SYMBOL(flush_dcache_icache_page);
 
+/**
+ * __flush_dcache_icache(): Flush a particular page from the data cache to RAM.
+ * Note: this is necessary because the instruction cache does *not*
+ * snoop from the data cache.
+ *
+ * @page: the address of the page to flush
+ */
+void __flush_dcache_icache(void *page)
+{
+	u64 addr = (u64)page;
+
+	if (flush_coherent_icache(addr))
+		return;
+
+	clean_dcache_range(addr, addr + PAGE_SIZE);
+
+	/*
+	 * We don't flush the icache on 44x. Those have a virtual icache and we
+	 * don't have access to the virtual address here (it's not the page
+	 * vaddr but where it's mapped in user space). The flushing of the
+	 * icache on these is handled elsewhere, when a change in the address
+	 * space occurs, before returning to user space.
+	 */
+
+	if (cpu_has_feature(MMU_FTR_TYPE_44x))
+		return;
+
+	invalidate_icache_range(addr, addr + PAGE_SIZE);
+}
+EXPORT_SYMBOL(__flush_dcache_icache);
+
 void clear_user_page(void *page, unsigned long vaddr, struct page *pg)
 {
 	clear_page(page);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v5 5/6] powerpc: Chunk calls to flush_dcache_range in arch_*_memory
  2019-11-04  2:32 ` Alastair D'Silva
@ 2019-11-04  2:32   ` Alastair D'Silva
  -1 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Christophe Leroy, Greg Kroah-Hartman, Thomas Gleixner, Qian Cai,
	Nicholas Piggin, Allison Randal, Andrew Morton,
	David Hildenbrand, linuxppc-dev, linux-kernel

From: Alastair D'Silva <alastair@d-silva.org>

When presented with large amounts of memory being hotplugged
(in my test case, ~890GB), the call to flush_dcache_range takes
a while (~50 seconds), triggering RCU stalls.

This patch breaks up the call into 1GB chunks, calling
cond_resched() inbetween to allow the scheduler to run.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/mm/mem.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 54d61ba15e93..a7b662fc02c8 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -104,6 +104,27 @@ int __weak remove_section_mapping(unsigned long start, unsigned long end)
 	return -ENODEV;
 }
 
+#define FLUSH_CHUNK_SIZE SZ_1G
+/**
+ * flush_dcache_range_chunked(): Write any modified data cache blocks out to
+ * memory and invalidate them, in chunks of up to FLUSH_CHUNK_SIZE
+ * Does not invalidate the corresponding instruction cache blocks.
+ *
+ * @start: the start address
+ * @stop: the stop address (exclusive)
+ * @chunk: the max size of the chunks
+ */
+static void flush_dcache_range_chunked(unsigned long start, unsigned long stop,
+				       unsigned long chunk)
+{
+	unsigned long i;
+
+	for (i = start; i < stop; i += chunk) {
+		flush_dcache_range(i, min(stop, start + chunk));
+		cond_resched();
+	}
+}
+
 int __ref arch_add_memory(int nid, u64 start, u64 size,
 			struct mhp_restrictions *restrictions)
 {
@@ -120,7 +141,8 @@ int __ref arch_add_memory(int nid, u64 start, u64 size,
 			start, start + size, rc);
 		return -EFAULT;
 	}
-	flush_dcache_range(start, start + size);
+
+	flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
 
 	return __add_pages(nid, start_pfn, nr_pages, restrictions);
 }
@@ -137,7 +159,8 @@ void __ref arch_remove_memory(int nid, u64 start, u64 size,
 
 	/* Remove htab bolted mappings for this section of memory */
 	start = (unsigned long)__va(start);
-	flush_dcache_range(start, start + size);
+	flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
+
 	ret = remove_section_mapping(start, start + size);
 	WARN_ON_ONCE(ret);
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v5 5/6] powerpc: Chunk calls to flush_dcache_range in arch_*_memory
@ 2019-11-04  2:32   ` Alastair D'Silva
  0 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: David Hildenbrand, linux-kernel, Nicholas Piggin, Paul Mackerras,
	Greg Kroah-Hartman, Qian Cai, Thomas Gleixner, linuxppc-dev,
	Andrew Morton, Allison Randal

From: Alastair D'Silva <alastair@d-silva.org>

When presented with large amounts of memory being hotplugged
(in my test case, ~890GB), the call to flush_dcache_range takes
a while (~50 seconds), triggering RCU stalls.

This patch breaks up the call into 1GB chunks, calling
cond_resched() inbetween to allow the scheduler to run.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/mm/mem.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 54d61ba15e93..a7b662fc02c8 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -104,6 +104,27 @@ int __weak remove_section_mapping(unsigned long start, unsigned long end)
 	return -ENODEV;
 }
 
+#define FLUSH_CHUNK_SIZE SZ_1G
+/**
+ * flush_dcache_range_chunked(): Write any modified data cache blocks out to
+ * memory and invalidate them, in chunks of up to FLUSH_CHUNK_SIZE
+ * Does not invalidate the corresponding instruction cache blocks.
+ *
+ * @start: the start address
+ * @stop: the stop address (exclusive)
+ * @chunk: the max size of the chunks
+ */
+static void flush_dcache_range_chunked(unsigned long start, unsigned long stop,
+				       unsigned long chunk)
+{
+	unsigned long i;
+
+	for (i = start; i < stop; i += chunk) {
+		flush_dcache_range(i, min(stop, start + chunk));
+		cond_resched();
+	}
+}
+
 int __ref arch_add_memory(int nid, u64 start, u64 size,
 			struct mhp_restrictions *restrictions)
 {
@@ -120,7 +141,8 @@ int __ref arch_add_memory(int nid, u64 start, u64 size,
 			start, start + size, rc);
 		return -EFAULT;
 	}
-	flush_dcache_range(start, start + size);
+
+	flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
 
 	return __add_pages(nid, start_pfn, nr_pages, restrictions);
 }
@@ -137,7 +159,8 @@ void __ref arch_remove_memory(int nid, u64 start, u64 size,
 
 	/* Remove htab bolted mappings for this section of memory */
 	start = (unsigned long)__va(start);
-	flush_dcache_range(start, start + size);
+	flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
+
 	ret = remove_section_mapping(start, start + size);
 	WARN_ON_ONCE(ret);
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v5 6/6] powerpc: Don't flush caches when adding memory
  2019-11-04  2:32 ` Alastair D'Silva
@ 2019-11-04  2:32   ` Alastair D'Silva
  -1 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Christophe Leroy, Thomas Gleixner, Allison Randal, Qian Cai,
	Nicholas Piggin, Greg Kroah-Hartman, Andrew Morton,
	David Hildenbrand, linuxppc-dev, linux-kernel

From: Alastair D'Silva <alastair@d-silva.org>

This operation takes a significant amount of time when hotplugging
large amounts of memory (~50 seconds with 890GB of persistent memory).

This was orignally in commit fb5924fddf9e
("powerpc/mm: Flush cache on memory hot(un)plug") to support memtrace,
but the flush on add is not needed as it is flushed on remove.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/mm/mem.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index a7b662fc02c8..4a424b514772 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -142,8 +142,6 @@ int __ref arch_add_memory(int nid, u64 start, u64 size,
 		return -EFAULT;
 	}
 
-	flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
-
 	return __add_pages(nid, start_pfn, nr_pages, restrictions);
 }
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v5 6/6] powerpc: Don't flush caches when adding memory
@ 2019-11-04  2:32   ` Alastair D'Silva
  0 siblings, 0 replies; 28+ messages in thread
From: Alastair D'Silva @ 2019-11-04  2:32 UTC (permalink / raw)
  To: alastair
  Cc: Greg Kroah-Hartman, David Hildenbrand, linux-kernel,
	Nicholas Piggin, Paul Mackerras, Qian Cai, Thomas Gleixner,
	linuxppc-dev, Andrew Morton, Allison Randal

From: Alastair D'Silva <alastair@d-silva.org>

This operation takes a significant amount of time when hotplugging
large amounts of memory (~50 seconds with 890GB of persistent memory).

This was orignally in commit fb5924fddf9e
("powerpc/mm: Flush cache on memory hot(un)plug") to support memtrace,
but the flush on add is not needed as it is flushed on remove.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 arch/powerpc/mm/mem.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index a7b662fc02c8..4a424b514772 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -142,8 +142,6 @@ int __ref arch_add_memory(int nid, u64 start, u64 size,
 		return -EFAULT;
 	}
 
-	flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
-
 	return __add_pages(nid, start_pfn, nr_pages, restrictions);
 }
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 4/6] powerpc: Convert flush_icache_range & friends to C
  2019-11-04  2:32   ` Alastair D'Silva
  (?)
@ 2019-11-04  4:54     ` kbuild test robot
  -1 siblings, 0 replies; 28+ messages in thread
From: kbuild test robot @ 2019-11-04  4:54 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: kbuild-all, alastair, Christophe Leroy, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Allison Randal, Qian Cai,
	Thomas Gleixner, Nicholas Piggin, Greg Kroah-Hartman,
	Andrew Morton, David Hildenbrand, linuxppc-dev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4063 bytes --]

Hi Alastair,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.4-rc6 next-20191031]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Alastair-D-Silva/powerpc-convert-cache-asm-to-C/20191104-103528
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/powerpc/mm/mem.c: In function 'flush_dcache_icache_page':
>> arch/powerpc/mm/mem.c:469:29: error: passing argument 1 of 'flush_coherent_icache' makes integer from pointer without a cast [-Werror=int-conversion]
      if (flush_coherent_icache((void *)addr))
                                ^
   arch/powerpc/mm/mem.c:326:20: note: expected 'long unsigned int' but argument is of type 'void *'
    static inline bool flush_coherent_icache(unsigned long addr)
                       ^~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/mm/mem.c: In function '__flush_dcache_icache':
>> arch/powerpc/mm/mem.c:486:13: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
     u64 addr = (u64)page;
                ^
   cc1: all warnings being treated as errors

vim +/flush_coherent_icache +469 arch/powerpc/mm/mem.c

   449	
   450	void flush_dcache_icache_page(struct page *page)
   451	{
   452	#ifdef CONFIG_HUGETLB_PAGE
   453		if (PageCompound(page)) {
   454			flush_dcache_icache_hugepage(page);
   455			return;
   456		}
   457	#endif
   458	#if defined(CONFIG_PPC_8xx) || defined(CONFIG_PPC64)
   459		/* On 8xx there is no need to kmap since highmem is not supported */
   460		__flush_dcache_icache(page_address(page));
   461	#else
   462		if (IS_ENABLED(CONFIG_BOOKE) || sizeof(phys_addr_t) > sizeof(void *)) {
   463			void *start = kmap_atomic(page);
   464			__flush_dcache_icache(start);
   465			kunmap_atomic(start);
   466		} else {
   467			unsigned long addr = page_to_pfn(page) << PAGE_SHIFT;
   468	
 > 469			if (flush_coherent_icache((void *)addr))
   470				return;
   471			flush_dcache_icache_phys(addr);
   472		}
   473	#endif
   474	}
   475	EXPORT_SYMBOL(flush_dcache_icache_page);
   476	
   477	/**
   478	 * __flush_dcache_icache(): Flush a particular page from the data cache to RAM.
   479	 * Note: this is necessary because the instruction cache does *not*
   480	 * snoop from the data cache.
   481	 *
   482	 * @page: the address of the page to flush
   483	 */
   484	void __flush_dcache_icache(void *page)
   485	{
 > 486		u64 addr = (u64)page;
   487	
   488		if (flush_coherent_icache(addr))
   489			return;
   490	
   491		clean_dcache_range(addr, addr + PAGE_SIZE);
   492	
   493		/*
   494		 * We don't flush the icache on 44x. Those have a virtual icache and we
   495		 * don't have access to the virtual address here (it's not the page
   496		 * vaddr but where it's mapped in user space). The flushing of the
   497		 * icache on these is handled elsewhere, when a change in the address
   498		 * space occurs, before returning to user space.
   499		 */
   500	
   501		if (cpu_has_feature(MMU_FTR_TYPE_44x))
   502			return;
   503	
   504		invalidate_icache_range(addr, addr + PAGE_SIZE);
   505	}
   506	EXPORT_SYMBOL(__flush_dcache_icache);
   507	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6375 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 4/6] powerpc: Convert flush_icache_range & friends to C
@ 2019-11-04  4:54     ` kbuild test robot
  0 siblings, 0 replies; 28+ messages in thread
From: kbuild test robot @ 2019-11-04  4:54 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: kbuild-all, Greg Kroah-Hartman, alastair, David Hildenbrand,
	linux-kernel, Nicholas Piggin, Paul Mackerras, Qian Cai,
	Thomas Gleixner, linuxppc-dev, Andrew Morton, Allison Randal

[-- Attachment #1: Type: text/plain, Size: 4063 bytes --]

Hi Alastair,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.4-rc6 next-20191031]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Alastair-D-Silva/powerpc-convert-cache-asm-to-C/20191104-103528
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/powerpc/mm/mem.c: In function 'flush_dcache_icache_page':
>> arch/powerpc/mm/mem.c:469:29: error: passing argument 1 of 'flush_coherent_icache' makes integer from pointer without a cast [-Werror=int-conversion]
      if (flush_coherent_icache((void *)addr))
                                ^
   arch/powerpc/mm/mem.c:326:20: note: expected 'long unsigned int' but argument is of type 'void *'
    static inline bool flush_coherent_icache(unsigned long addr)
                       ^~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/mm/mem.c: In function '__flush_dcache_icache':
>> arch/powerpc/mm/mem.c:486:13: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
     u64 addr = (u64)page;
                ^
   cc1: all warnings being treated as errors

vim +/flush_coherent_icache +469 arch/powerpc/mm/mem.c

   449	
   450	void flush_dcache_icache_page(struct page *page)
   451	{
   452	#ifdef CONFIG_HUGETLB_PAGE
   453		if (PageCompound(page)) {
   454			flush_dcache_icache_hugepage(page);
   455			return;
   456		}
   457	#endif
   458	#if defined(CONFIG_PPC_8xx) || defined(CONFIG_PPC64)
   459		/* On 8xx there is no need to kmap since highmem is not supported */
   460		__flush_dcache_icache(page_address(page));
   461	#else
   462		if (IS_ENABLED(CONFIG_BOOKE) || sizeof(phys_addr_t) > sizeof(void *)) {
   463			void *start = kmap_atomic(page);
   464			__flush_dcache_icache(start);
   465			kunmap_atomic(start);
   466		} else {
   467			unsigned long addr = page_to_pfn(page) << PAGE_SHIFT;
   468	
 > 469			if (flush_coherent_icache((void *)addr))
   470				return;
   471			flush_dcache_icache_phys(addr);
   472		}
   473	#endif
   474	}
   475	EXPORT_SYMBOL(flush_dcache_icache_page);
   476	
   477	/**
   478	 * __flush_dcache_icache(): Flush a particular page from the data cache to RAM.
   479	 * Note: this is necessary because the instruction cache does *not*
   480	 * snoop from the data cache.
   481	 *
   482	 * @page: the address of the page to flush
   483	 */
   484	void __flush_dcache_icache(void *page)
   485	{
 > 486		u64 addr = (u64)page;
   487	
   488		if (flush_coherent_icache(addr))
   489			return;
   490	
   491		clean_dcache_range(addr, addr + PAGE_SIZE);
   492	
   493		/*
   494		 * We don't flush the icache on 44x. Those have a virtual icache and we
   495		 * don't have access to the virtual address here (it's not the page
   496		 * vaddr but where it's mapped in user space). The flushing of the
   497		 * icache on these is handled elsewhere, when a change in the address
   498		 * space occurs, before returning to user space.
   499		 */
   500	
   501		if (cpu_has_feature(MMU_FTR_TYPE_44x))
   502			return;
   503	
   504		invalidate_icache_range(addr, addr + PAGE_SIZE);
   505	}
   506	EXPORT_SYMBOL(__flush_dcache_icache);
   507	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6375 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 4/6] powerpc: Convert flush_icache_range & friends to C
@ 2019-11-04  4:54     ` kbuild test robot
  0 siblings, 0 replies; 28+ messages in thread
From: kbuild test robot @ 2019-11-04  4:54 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4166 bytes --]

Hi Alastair,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.4-rc6 next-20191031]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Alastair-D-Silva/powerpc-convert-cache-asm-to-C/20191104-103528
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/powerpc/mm/mem.c: In function 'flush_dcache_icache_page':
>> arch/powerpc/mm/mem.c:469:29: error: passing argument 1 of 'flush_coherent_icache' makes integer from pointer without a cast [-Werror=int-conversion]
      if (flush_coherent_icache((void *)addr))
                                ^
   arch/powerpc/mm/mem.c:326:20: note: expected 'long unsigned int' but argument is of type 'void *'
    static inline bool flush_coherent_icache(unsigned long addr)
                       ^~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/mm/mem.c: In function '__flush_dcache_icache':
>> arch/powerpc/mm/mem.c:486:13: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
     u64 addr = (u64)page;
                ^
   cc1: all warnings being treated as errors

vim +/flush_coherent_icache +469 arch/powerpc/mm/mem.c

   449	
   450	void flush_dcache_icache_page(struct page *page)
   451	{
   452	#ifdef CONFIG_HUGETLB_PAGE
   453		if (PageCompound(page)) {
   454			flush_dcache_icache_hugepage(page);
   455			return;
   456		}
   457	#endif
   458	#if defined(CONFIG_PPC_8xx) || defined(CONFIG_PPC64)
   459		/* On 8xx there is no need to kmap since highmem is not supported */
   460		__flush_dcache_icache(page_address(page));
   461	#else
   462		if (IS_ENABLED(CONFIG_BOOKE) || sizeof(phys_addr_t) > sizeof(void *)) {
   463			void *start = kmap_atomic(page);
   464			__flush_dcache_icache(start);
   465			kunmap_atomic(start);
   466		} else {
   467			unsigned long addr = page_to_pfn(page) << PAGE_SHIFT;
   468	
 > 469			if (flush_coherent_icache((void *)addr))
   470				return;
   471			flush_dcache_icache_phys(addr);
   472		}
   473	#endif
   474	}
   475	EXPORT_SYMBOL(flush_dcache_icache_page);
   476	
   477	/**
   478	 * __flush_dcache_icache(): Flush a particular page from the data cache to RAM.
   479	 * Note: this is necessary because the instruction cache does *not*
   480	 * snoop from the data cache.
   481	 *
   482	 * @page: the address of the page to flush
   483	 */
   484	void __flush_dcache_icache(void *page)
   485	{
 > 486		u64 addr = (u64)page;
   487	
   488		if (flush_coherent_icache(addr))
   489			return;
   490	
   491		clean_dcache_range(addr, addr + PAGE_SIZE);
   492	
   493		/*
   494		 * We don't flush the icache on 44x. Those have a virtual icache and we
   495		 * don't have access to the virtual address here (it's not the page
   496		 * vaddr but where it's mapped in user space). The flushing of the
   497		 * icache on these is handled elsewhere, when a change in the address
   498		 * space occurs, before returning to user space.
   499		 */
   500	
   501		if (cpu_has_feature(MMU_FTR_TYPE_44x))
   502			return;
   503	
   504		invalidate_icache_range(addr, addr + PAGE_SIZE);
   505	}
   506	EXPORT_SYMBOL(__flush_dcache_icache);
   507	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 6375 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 1/6] powerpc: Allow flush_icache_range to work across ranges >4GB
  2019-11-04  2:32   ` Alastair D'Silva
@ 2019-11-04 19:43     ` Segher Boessenkool
  -1 siblings, 0 replies; 28+ messages in thread
From: Segher Boessenkool @ 2019-11-04 19:43 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: alastair, Greg Kroah-Hartman, David Hildenbrand, linux-kernel,
	stable, Paul Mackerras, Nicholas Piggin, Qian Cai,
	Thomas Gleixner, linuxppc-dev, Andrew Morton, Allison Randal

On Mon, Nov 04, 2019 at 01:32:53PM +1100, Alastair D'Silva wrote:
> When calling flush_icache_range with a size >4GB, we were masking
> off the upper 32 bits, so we would incorrectly flush a range smaller
> than intended.
> 
> This patch replaces the 32 bit shifts with 64 bit ones, so that
> the full size is accounted for.

Please send this separately, to be committed right now?  It is a bug fix,
independent of the rest of the series.


Segher

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 1/6] powerpc: Allow flush_icache_range to work across ranges >4GB
@ 2019-11-04 19:43     ` Segher Boessenkool
  0 siblings, 0 replies; 28+ messages in thread
From: Segher Boessenkool @ 2019-11-04 19:43 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: David Hildenbrand, Greg Kroah-Hartman, linux-kernel, stable,
	Paul Mackerras, Nicholas Piggin, alastair, Qian Cai,
	Thomas Gleixner, linuxppc-dev, Andrew Morton, Allison Randal

On Mon, Nov 04, 2019 at 01:32:53PM +1100, Alastair D'Silva wrote:
> When calling flush_icache_range with a size >4GB, we were masking
> off the upper 32 bits, so we would incorrectly flush a range smaller
> than intended.
> 
> This patch replaces the 32 bit shifts with 64 bit ones, so that
> the full size is accounted for.

Please send this separately, to be committed right now?  It is a bug fix,
independent of the rest of the series.


Segher

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 1/6] powerpc: Allow flush_icache_range to work across ranges >4GB
  2019-11-04 19:43     ` Segher Boessenkool
@ 2019-11-05  6:04       ` Christophe Leroy
  -1 siblings, 0 replies; 28+ messages in thread
From: Christophe Leroy @ 2019-11-05  6:04 UTC (permalink / raw)
  To: Segher Boessenkool, Alastair D'Silva
  Cc: David Hildenbrand, Greg Kroah-Hartman, linux-kernel, stable,
	Paul Mackerras, Nicholas Piggin, alastair, Qian Cai,
	Thomas Gleixner, linuxppc-dev, Andrew Morton, Allison Randal



Le 04/11/2019 à 20:43, Segher Boessenkool a écrit :
> On Mon, Nov 04, 2019 at 01:32:53PM +1100, Alastair D'Silva wrote:
>> When calling flush_icache_range with a size >4GB, we were masking
>> off the upper 32 bits, so we would incorrectly flush a range smaller
>> than intended.
>>
>> This patch replaces the 32 bit shifts with 64 bit ones, so that
>> the full size is accounted for.
> 
> Please send this separately, to be committed right now?  It is a bug fix,
> independent of the rest of the series.
> 

Patch 4/6 needs it, as it drops the function.

Or do you mean that the series should drop the assembly at once, and 
this patch should only go into stable ?

But I guess mpe can take this patch alone if he wants to ?

By the way, Patch 2/6 is also a bugfix.

Christophe

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 1/6] powerpc: Allow flush_icache_range to work across ranges >4GB
@ 2019-11-05  6:04       ` Christophe Leroy
  0 siblings, 0 replies; 28+ messages in thread
From: Christophe Leroy @ 2019-11-05  6:04 UTC (permalink / raw)
  To: Segher Boessenkool, Alastair D'Silva
  Cc: David Hildenbrand, Greg Kroah-Hartman, linux-kernel,
	Nicholas Piggin, Paul Mackerras, stable, alastair, Qian Cai,
	Thomas Gleixner, linuxppc-dev, Andrew Morton, Allison Randal



Le 04/11/2019 à 20:43, Segher Boessenkool a écrit :
> On Mon, Nov 04, 2019 at 01:32:53PM +1100, Alastair D'Silva wrote:
>> When calling flush_icache_range with a size >4GB, we were masking
>> off the upper 32 bits, so we would incorrectly flush a range smaller
>> than intended.
>>
>> This patch replaces the 32 bit shifts with 64 bit ones, so that
>> the full size is accounted for.
> 
> Please send this separately, to be committed right now?  It is a bug fix,
> independent of the rest of the series.
> 

Patch 4/6 needs it, as it drops the function.

Or do you mean that the series should drop the assembly at once, and 
this patch should only go into stable ?

But I guess mpe can take this patch alone if he wants to ?

By the way, Patch 2/6 is also a bugfix.

Christophe

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 1/6] powerpc: Allow flush_icache_range to work across ranges >4GB
  2019-11-05  6:04       ` Christophe Leroy
  (?)
@ 2019-11-06 17:49       ` Segher Boessenkool
  -1 siblings, 0 replies; 28+ messages in thread
From: Segher Boessenkool @ 2019-11-06 17:49 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Alastair D'Silva, David Hildenbrand, Greg Kroah-Hartman,
	linux-kernel, stable, Paul Mackerras, Nicholas Piggin, alastair,
	Qian Cai, Thomas Gleixner, linuxppc-dev, Andrew Morton,
	Allison Randal

On Tue, Nov 05, 2019 at 07:04:04AM +0100, Christophe Leroy wrote:
> Le 04/11/2019 à 20:43, Segher Boessenkool a écrit :
> >Please send this separately, to be committed right now?  It is a bug fix,
> >independent of the rest of the series.
> 
> Patch 4/6 needs it, as it drops the function.
> 
> Or do you mean that the series should drop the assembly at once, and 
> this patch should only go into stable ?

I meant that you can say these patches (yes, 2/ as well) are bug fixes,
independent of the rest, and they can be picked up immediately, there
is no need to wait for v18 of this series.

> But I guess mpe can take this patch alone if he wants to ?

Yeah, but you can help him do that ;-)


Segher

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 5/6] powerpc: Chunk calls to flush_dcache_range in arch_*_memory
  2019-11-04  2:32   ` Alastair D'Silva
@ 2019-11-07 11:54     ` Michael Ellerman
  -1 siblings, 0 replies; 28+ messages in thread
From: Michael Ellerman @ 2019-11-07 11:54 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Christophe Leroy,
	Greg Kroah-Hartman, Thomas Gleixner, Qian Cai, Nicholas Piggin,
	Allison Randal, Andrew Morton, David Hildenbrand, linuxppc-dev,
	linux-kernel

"Alastair D'Silva" <alastair@au1.ibm.com> writes:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> When presented with large amounts of memory being hotplugged
> (in my test case, ~890GB), the call to flush_dcache_range takes
> a while (~50 seconds), triggering RCU stalls.
>
> This patch breaks up the call into 1GB chunks, calling
> cond_resched() inbetween to allow the scheduler to run.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>

I'm going to mark this as:
  Fixes: fb5924fddf9e ("powerpc/mm: Flush cache on memory hot(un)plug")

Because anyone doing large memory hotplugs on older kernels is going to
want to backport this to at least that point, otherwise they will see
the softlockups/RCU stalls.

cheers

> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 54d61ba15e93..a7b662fc02c8 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -104,6 +104,27 @@ int __weak remove_section_mapping(unsigned long start, unsigned long end)
>  	return -ENODEV;
>  }
>  
> +#define FLUSH_CHUNK_SIZE SZ_1G
> +/**
> + * flush_dcache_range_chunked(): Write any modified data cache blocks out to
> + * memory and invalidate them, in chunks of up to FLUSH_CHUNK_SIZE
> + * Does not invalidate the corresponding instruction cache blocks.
> + *
> + * @start: the start address
> + * @stop: the stop address (exclusive)
> + * @chunk: the max size of the chunks
> + */
> +static void flush_dcache_range_chunked(unsigned long start, unsigned long stop,
> +				       unsigned long chunk)
> +{
> +	unsigned long i;
> +
> +	for (i = start; i < stop; i += chunk) {
> +		flush_dcache_range(i, min(stop, start + chunk));
> +		cond_resched();
> +	}
> +}
> +
>  int __ref arch_add_memory(int nid, u64 start, u64 size,
>  			struct mhp_restrictions *restrictions)
>  {
> @@ -120,7 +141,8 @@ int __ref arch_add_memory(int nid, u64 start, u64 size,
>  			start, start + size, rc);
>  		return -EFAULT;
>  	}
> -	flush_dcache_range(start, start + size);
> +
> +	flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
>  
>  	return __add_pages(nid, start_pfn, nr_pages, restrictions);
>  }
> @@ -137,7 +159,8 @@ void __ref arch_remove_memory(int nid, u64 start, u64 size,
>  
>  	/* Remove htab bolted mappings for this section of memory */
>  	start = (unsigned long)__va(start);
> -	flush_dcache_range(start, start + size);
> +	flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
> +
>  	ret = remove_section_mapping(start, start + size);
>  	WARN_ON_ONCE(ret);
>  
> -- 
> 2.21.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 5/6] powerpc: Chunk calls to flush_dcache_range in arch_*_memory
@ 2019-11-07 11:54     ` Michael Ellerman
  0 siblings, 0 replies; 28+ messages in thread
From: Michael Ellerman @ 2019-11-07 11:54 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: David Hildenbrand, linux-kernel, Nicholas Piggin, Qian Cai,
	Greg Kroah-Hartman, Paul Mackerras, Thomas Gleixner,
	linuxppc-dev, Andrew Morton, Allison Randal

"Alastair D'Silva" <alastair@au1.ibm.com> writes:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> When presented with large amounts of memory being hotplugged
> (in my test case, ~890GB), the call to flush_dcache_range takes
> a while (~50 seconds), triggering RCU stalls.
>
> This patch breaks up the call into 1GB chunks, calling
> cond_resched() inbetween to allow the scheduler to run.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>

I'm going to mark this as:
  Fixes: fb5924fddf9e ("powerpc/mm: Flush cache on memory hot(un)plug")

Because anyone doing large memory hotplugs on older kernels is going to
want to backport this to at least that point, otherwise they will see
the softlockups/RCU stalls.

cheers

> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 54d61ba15e93..a7b662fc02c8 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -104,6 +104,27 @@ int __weak remove_section_mapping(unsigned long start, unsigned long end)
>  	return -ENODEV;
>  }
>  
> +#define FLUSH_CHUNK_SIZE SZ_1G
> +/**
> + * flush_dcache_range_chunked(): Write any modified data cache blocks out to
> + * memory and invalidate them, in chunks of up to FLUSH_CHUNK_SIZE
> + * Does not invalidate the corresponding instruction cache blocks.
> + *
> + * @start: the start address
> + * @stop: the stop address (exclusive)
> + * @chunk: the max size of the chunks
> + */
> +static void flush_dcache_range_chunked(unsigned long start, unsigned long stop,
> +				       unsigned long chunk)
> +{
> +	unsigned long i;
> +
> +	for (i = start; i < stop; i += chunk) {
> +		flush_dcache_range(i, min(stop, start + chunk));
> +		cond_resched();
> +	}
> +}
> +
>  int __ref arch_add_memory(int nid, u64 start, u64 size,
>  			struct mhp_restrictions *restrictions)
>  {
> @@ -120,7 +141,8 @@ int __ref arch_add_memory(int nid, u64 start, u64 size,
>  			start, start + size, rc);
>  		return -EFAULT;
>  	}
> -	flush_dcache_range(start, start + size);
> +
> +	flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
>  
>  	return __add_pages(nid, start_pfn, nr_pages, restrictions);
>  }
> @@ -137,7 +159,8 @@ void __ref arch_remove_memory(int nid, u64 start, u64 size,
>  
>  	/* Remove htab bolted mappings for this section of memory */
>  	start = (unsigned long)__va(start);
> -	flush_dcache_range(start, start + size);
> +	flush_dcache_range_chunked(start, start + size, FLUSH_CHUNK_SIZE);
> +
>  	ret = remove_section_mapping(start, start + size);
>  	WARN_ON_ONCE(ret);
>  
> -- 
> 2.21.0

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 4/6] powerpc: Convert flush_icache_range & friends to C
  2019-11-04  2:32   ` Alastair D'Silva
@ 2019-11-12 10:49     ` Michael Ellerman
  -1 siblings, 0 replies; 28+ messages in thread
From: Michael Ellerman @ 2019-11-12 10:49 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Allison Randal, Qian Cai, Thomas Gleixner, Nicholas Piggin,
	Greg Kroah-Hartman, Andrew Morton, David Hildenbrand,
	linuxppc-dev, linux-kernel

"Alastair D'Silva" <alastair@au1.ibm.com> writes:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Similar to commit 22e9c88d486a
> ("powerpc/64: reuse PPC32 static inline flush_dcache_range()")
> this patch converts the following ASM symbols to C:
>     flush_icache_range()
>     __flush_dcache_icache()
>     __flush_dcache_icache_phys()
>
> This was done as we discovered a long-standing bug where the length of the
> range was truncated due to using a 32 bit shift instead of a 64 bit one.
>
> By converting these functions to C, it becomes easier to maintain.
>
> flush_dcache_icache_phys() retains a critical assembler section as we must
> ensure there are no memory accesses while the data MMU is disabled
> (authored by Christophe Leroy). Since this has no external callers, it has
> also been made static, allowing the compiler to inline it within
> flush_dcache_icache_page().
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  arch/powerpc/include/asm/cache.h      |  26 ++---
>  arch/powerpc/include/asm/cacheflush.h |  24 ++--
>  arch/powerpc/kernel/misc_32.S         | 120 --------------------
>  arch/powerpc/kernel/misc_64.S         | 102 -----------------
>  arch/powerpc/mm/mem.c                 | 151 +++++++++++++++++++++++++-
>  5 files changed, 172 insertions(+), 251 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cache.h b/arch/powerpc/include/asm/cache.h
> index afb88754e0e0..b809095cb1ba 100644
> --- a/arch/powerpc/include/asm/cache.h
> +++ b/arch/powerpc/include/asm/cache.h
> @@ -98,20 +98,7 @@ static inline u32 l1_icache_bytes(void)
>  #endif
>  #endif /* ! __ASSEMBLY__ */
  
Because of the above ...

> -#if defined(__ASSEMBLY__)
> -/*
> - * For a snooping icache, we still need a dummy icbi to purge all the
> - * prefetched instructions from the ifetch buffers. We also need a sync
> - * before the icbi to order the the actual stores to memory that might
> - * have modified instructions with the icbi.
> - */
> -#define PURGE_PREFETCHED_INS	\
> -	sync;			\
> -	icbi	0,r3;		\
> -	sync;			\
> -	isync
> -
> -#else
> +#if !defined(__ASSEMBLY__)

And this, we now end up with two adjacent !defined(__ASSEMBLY__) blocks.
So I merged them.

>  #define __read_mostly __attribute__((__section__(".data..read_mostly")))
>  
>  #ifdef CONFIG_PPC_BOOK3S_32
> @@ -145,6 +132,17 @@ static inline void dcbst(void *addr)
>  {
>  	__asm__ __volatile__ ("dcbst 0, %0" : : "r"(addr) : "memory");
>  }
> +
> +static inline void icbi(void *addr)
> +{
> +	asm volatile ("icbi 0, %0" : : "r"(addr) : "memory");
> +}
> +
> +static inline void iccci(void *addr)
> +{
> +	asm volatile ("iccci 0, %0" : : "r"(addr) : "memory");
> +}
> +
>  #endif /* !__ASSEMBLY__ */
>  #endif /* __KERNEL__ */
>  #endif /* _ASM_POWERPC_CACHE_H */
> diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
> index ed57843ef452..4a1c9f0200e1 100644
> --- a/arch/powerpc/include/asm/cacheflush.h
> +++ b/arch/powerpc/include/asm/cacheflush.h
> @@ -42,24 +42,20 @@ extern void flush_dcache_page(struct page *page);
>  #define flush_dcache_mmap_lock(mapping)		do { } while (0)
>  #define flush_dcache_mmap_unlock(mapping)	do { } while (0)
>  
> -extern void flush_icache_range(unsigned long, unsigned long);
> +void flush_icache_range(unsigned long start, unsigned long stop);
>  extern void flush_icache_user_range(struct vm_area_struct *vma,
>  				    struct page *page, unsigned long addr,
>  				    int len);
> -extern void __flush_dcache_icache(void *page_va);
>  extern void flush_dcache_icache_page(struct page *page);
> -#if defined(CONFIG_PPC32) && !defined(CONFIG_BOOKE)
> -extern void __flush_dcache_icache_phys(unsigned long physaddr);
> -#else
> -static inline void __flush_dcache_icache_phys(unsigned long physaddr)
> -{
> -	BUG();
> -}
> -#endif
> -
> -/*
> - * Write any modified data cache blocks out to memory and invalidate them.
> - * Does not invalidate the corresponding instruction cache blocks.
> +void __flush_dcache_icache(void *page);
> +
> +/**
> + * flush_dcache_range(): Write any modified data cache blocks out to memory and
> + * invalidate them. Does not invalidate the corresponding instruction cache
> + * blocks.
> + *
> + * @start: the start address
> + * @stop: the stop address (exclusive)
>   */
>  static inline void flush_dcache_range(unsigned long start, unsigned long stop)
>  {
> diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
> index 82df4b09e79f..f4e4a1926a7a 100644
> --- a/arch/powerpc/kernel/misc_32.S
> +++ b/arch/powerpc/kernel/misc_32.S
> @@ -316,126 +316,6 @@ _GLOBAL(flush_instruction_cache)
>  EXPORT_SYMBOL(flush_instruction_cache)
>  #endif /* CONFIG_PPC_8xx */
>  
> -/*
> - * Write any modified data cache blocks out to memory
> - * and invalidate the corresponding instruction cache blocks.
> - * This is a no-op on the 601.
> - *
> - * flush_icache_range(unsigned long start, unsigned long stop)
> - */
> -_GLOBAL(flush_icache_range)
> -#if defined(CONFIG_PPC_BOOK3S_601) || defined(CONFIG_E200)
> -	PURGE_PREFETCHED_INS
> -	blr				/* for 601 and e200, do nothing */
> -#else
> -	rlwinm	r3,r3,0,0,31 - L1_CACHE_SHIFT
> -	subf	r4,r3,r4
> -	addi	r4,r4,L1_CACHE_BYTES - 1
> -	srwi.	r4,r4,L1_CACHE_SHIFT
> -	beqlr
> -	mtctr	r4
> -	mr	r6,r3
> -1:	dcbst	0,r3
> -	addi	r3,r3,L1_CACHE_BYTES
> -	bdnz	1b
> -	sync				/* wait for dcbst's to get to ram */
> -#ifndef CONFIG_44x
> -	mtctr	r4
> -2:	icbi	0,r6
> -	addi	r6,r6,L1_CACHE_BYTES
> -	bdnz	2b
> -#else
> -	/* Flash invalidate on 44x because we are passed kmapped addresses and
> -	   this doesn't work for userspace pages due to the virtually tagged
> -	   icache.  Sigh. */
> -	iccci	0, r0
> -#endif
> -	sync				/* additional sync needed on g4 */
> -	isync
> -	blr
> -#endif
> -_ASM_NOKPROBE_SYMBOL(flush_icache_range)
> -EXPORT_SYMBOL(flush_icache_range)
> -
> -/*
> - * Flush a particular page from the data cache to RAM.
> - * Note: this is necessary because the instruction cache does *not*
> - * snoop from the data cache.
> - * This is a no-op on the 601 and e200 which have a unified cache.
> - *
> - *	void __flush_dcache_icache(void *page)
> - */
> -_GLOBAL(__flush_dcache_icache)
> -#if defined(CONFIG_PPC_BOOK3S_601) || defined(CONFIG_E200)
> -	PURGE_PREFETCHED_INS
> -	blr
> -#else
> -	rlwinm	r3,r3,0,0,31-PAGE_SHIFT		/* Get page base address */
> -	li	r4,PAGE_SIZE/L1_CACHE_BYTES	/* Number of lines in a page */
> -	mtctr	r4
> -	mr	r6,r3
> -0:	dcbst	0,r3				/* Write line to ram */
> -	addi	r3,r3,L1_CACHE_BYTES
> -	bdnz	0b
> -	sync
> -#ifdef CONFIG_44x
> -	/* We don't flush the icache on 44x. Those have a virtual icache
> -	 * and we don't have access to the virtual address here (it's
> -	 * not the page vaddr but where it's mapped in user space). The
> -	 * flushing of the icache on these is handled elsewhere, when
> -	 * a change in the address space occurs, before returning to
> -	 * user space
> -	 */
> -BEGIN_MMU_FTR_SECTION
> -	blr
> -END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_44x)
> -#endif /* CONFIG_44x */
> -	mtctr	r4
> -1:	icbi	0,r6
> -	addi	r6,r6,L1_CACHE_BYTES
> -	bdnz	1b
> -	sync
> -	isync
> -	blr
> -#endif
> -
> -#ifndef CONFIG_BOOKE
> -/*
> - * Flush a particular page from the data cache to RAM, identified
> - * by its physical address.  We turn off the MMU so we can just use
> - * the physical address (this may be a highmem page without a kernel
> - * mapping).
> - *
> - *	void __flush_dcache_icache_phys(unsigned long physaddr)
> - */
> -_GLOBAL(__flush_dcache_icache_phys)
> -#if defined(CONFIG_PPC_BOOK3S_601) || defined(CONFIG_E200)
> -	PURGE_PREFETCHED_INS
> -	blr					/* for 601 and e200, do nothing */
> -#else
> -	mfmsr	r10
> -	rlwinm	r0,r10,0,28,26			/* clear DR */
> -	mtmsr	r0
> -	isync
> -	rlwinm	r3,r3,0,0,31-PAGE_SHIFT		/* Get page base address */
> -	li	r4,PAGE_SIZE/L1_CACHE_BYTES	/* Number of lines in a page */
> -	mtctr	r4
> -	mr	r6,r3
> -0:	dcbst	0,r3				/* Write line to ram */
> -	addi	r3,r3,L1_CACHE_BYTES
> -	bdnz	0b
> -	sync
> -	mtctr	r4
> -1:	icbi	0,r6
> -	addi	r6,r6,L1_CACHE_BYTES
> -	bdnz	1b
> -	sync
> -	mtmsr	r10				/* restore DR */
> -	isync
> -	blr
> -#endif
> -#endif /* CONFIG_BOOKE */
> -
>  /*
>   * Copy a whole page.  We use the dcbz instruction on the destination
>   * to reduce memory traffic (it eliminates the unnecessary reads of
> diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
> index 9bc0aa9aeb65..ff20c253f273 100644
> --- a/arch/powerpc/kernel/misc_64.S
> +++ b/arch/powerpc/kernel/misc_64.S
> @@ -49,108 +49,6 @@ _GLOBAL(call_do_irq)
>  	mtlr	r0
>  	blr
>  
> -	.section	".toc","aw"
> -PPC64_CACHES:
> -	.tc		ppc64_caches[TC],ppc64_caches
> -	.section	".text"
> -
> -/*
> - * Write any modified data cache blocks out to memory
> - * and invalidate the corresponding instruction cache blocks.
> - *
> - * flush_icache_range(unsigned long start, unsigned long stop)
> - *
> - *   flush all bytes from start through stop-1 inclusive
> - */
> -
> -_GLOBAL_TOC(flush_icache_range)
> -BEGIN_FTR_SECTION
> -	PURGE_PREFETCHED_INS
> -	blr
> -END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> -/*
> - * Flush the data cache to memory 
> - * 
> - * Different systems have different cache line sizes
> - * and in some cases i-cache and d-cache line sizes differ from
> - * each other.
> - */
> - 	ld	r10,PPC64_CACHES@toc(r2)
> -	lwz	r7,DCACHEL1BLOCKSIZE(r10)/* Get cache block size */
> -	addi	r5,r7,-1
> -	andc	r6,r3,r5		/* round low to line bdy */
> -	subf	r8,r6,r4		/* compute length */
> -	add	r8,r8,r5		/* ensure we get enough */
> -	lwz	r9,DCACHEL1LOGBLOCKSIZE(r10)	/* Get log-2 of cache block size */
> -	srd.	r8,r8,r9		/* compute line count */
> -	beqlr				/* nothing to do? */
> -	mtctr	r8
> -1:	dcbst	0,r6
> -	add	r6,r6,r7
> -	bdnz	1b
> -	sync
> -
> -/* Now invalidate the instruction cache */
> -	
> -	lwz	r7,ICACHEL1BLOCKSIZE(r10)	/* Get Icache block size */
> -	addi	r5,r7,-1
> -	andc	r6,r3,r5		/* round low to line bdy */
> -	subf	r8,r6,r4		/* compute length */
> -	add	r8,r8,r5
> -	lwz	r9,ICACHEL1LOGBLOCKSIZE(r10)	/* Get log-2 of Icache block size */
> -	srd.	r8,r8,r9		/* compute line count */
> -	beqlr				/* nothing to do? */
> -	mtctr	r8
> -2:	icbi	0,r6
> -	add	r6,r6,r7
> -	bdnz	2b
> -	isync
> -	blr
> -_ASM_NOKPROBE_SYMBOL(flush_icache_range)
> -EXPORT_SYMBOL(flush_icache_range)
> -
> -/*
> - * Flush a particular page from the data cache to RAM.
> - * Note: this is necessary because the instruction cache does *not*
> - * snoop from the data cache.
> - *
> - *	void __flush_dcache_icache(void *page)
> - */
> -_GLOBAL(__flush_dcache_icache)
> -/*
> - * Flush the data cache to memory 
> - * 
> - * Different systems have different cache line sizes
> - */
> -
> -BEGIN_FTR_SECTION
> -	PURGE_PREFETCHED_INS
> -	blr
> -END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> -
> -/* Flush the dcache */
> - 	ld	r7,PPC64_CACHES@toc(r2)
> -	clrrdi	r3,r3,PAGE_SHIFT           	    /* Page align */
> -	lwz	r4,DCACHEL1BLOCKSPERPAGE(r7)	/* Get # dcache blocks per page */
> -	lwz	r5,DCACHEL1BLOCKSIZE(r7)	/* Get dcache block size */
> -	mr	r6,r3
> -	mtctr	r4
> -0:	dcbst	0,r6
> -	add	r6,r6,r5
> -	bdnz	0b
> -	sync
> -
> -/* Now invalidate the icache */	
> -
> -	lwz	r4,ICACHEL1BLOCKSPERPAGE(r7)	/* Get # icache blocks per page */
> -	lwz	r5,ICACHEL1BLOCKSIZE(r7)	/* Get icache block size */
> -	mtctr	r4
> -1:	icbi	0,r3
> -	add	r3,r3,r5
> -	bdnz	1b
> -	isync
> -	blr
> -
>  _GLOBAL(__bswapdi2)
>  EXPORT_SYMBOL(__bswapdi2)
>  	srdi	r8,r3,32
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index be941d382c8d..54d61ba15e93 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -318,6 +318,120 @@ void free_initmem(void)
>  	free_initmem_default(POISON_FREE_INITMEM);
>  }
>  
> +/**
> + * flush_coherent_icache() - if a CPU has a coherent icache, flush it
> + * @addr: The base address to use (can be any valid address, the whole cache will be flushed)
> + * Return true if the cache was flushed, false otherwise
> + */
> +static inline bool flush_coherent_icache(unsigned long addr)
> +{
> +	/*
> +	 * For a snooping icache, we still need a dummy icbi to purge all the
> +	 * prefetched instructions from the ifetch buffers. We also need a sync
> +	 * before the icbi to order the the actual stores to memory that might
> +	 * have modified instructions with the icbi.
> +	 */
> +	if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
> +		mb(); /* sync */
> +		icbi((void *)addr);
> +		mb(); /* sync */
> +		isync();
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +/**
> + * invalidate_icache_range() - Flush the icache by issuing icbi across an address range
> + * @start: the start address
> + * @stop: the stop address (exclusive)
> + */
> +static void invalidate_icache_range(unsigned long start, unsigned long stop)
> +{
> +	unsigned long shift = l1_icache_shift();
> +	unsigned long bytes = l1_icache_bytes();
> +	char *addr = (char *)(start & ~(bytes - 1));
> +	unsigned long size = stop - (unsigned long)addr + (bytes - 1);
> +	unsigned long i;
> +
> +	for (i = 0; i < size >> shift; i++, addr += bytes)
> +		icbi(addr);
> +
> +	mb(); /* sync */
> +	isync();
> +}
> +
> +/**
> + * flush_icache_range: Write any modified data cache blocks out to memory
> + * and invalidate the corresponding blocks in the instruction cache
> + *
> + * Generic code will call this after writing memory, before executing from it.
> + *
> + * @start: the start address
> + * @stop: the stop address (exclusive)
> + */
> +void flush_icache_range(unsigned long start, unsigned long stop)
> +{
> +	if (flush_coherent_icache(start))
> +		return;
> +
> +	clean_dcache_range(start, stop);
> +
> +	if (IS_ENABLED(CONFIG_44x)) {
> +		/*
> +		 * Flash invalidate on 44x because we are passed kmapped
> +		 * addresses and this doesn't work for userspace pages due to
> +		 * the virtually tagged icache.
> +		 */
> +		iccci((void *)start);
> +		mb(); /* sync */
> +		isync();
> +	} else
> +		invalidate_icache_range(start, stop);
> +}
> +EXPORT_SYMBOL(flush_icache_range);
> +
> +#if !defined(CONFIG_PPC_8xx) && !defined(CONFIG_PPC64)
> +/**
> + * flush_dcache_icache_phys() - Flush a page by it's physical address
> + * @physaddr: the physical address of the page
> + */
> +static void flush_dcache_icache_phys(unsigned long physaddr)
> +{
> +	unsigned long bytes = l1_dcache_bytes();
> +	unsigned long nb = PAGE_SIZE / bytes;
> +	unsigned long addr = physaddr & PAGE_MASK;
> +	unsigned long msr, msr0;
> +	unsigned long loop1 = addr, loop2 = addr;
> +
> +	msr0 = mfmsr();
> +	msr = msr0 & ~MSR_DR;
> +	/*
> +	 * This must remain as ASM to prevent potential memory accesses
> +	 * while the data MMU is disabled
> +	 */
> +	asm volatile(
> +		"   mtctr %2;\n"
> +		"   mtmsr %3;\n"
> +		"   isync;\n"
> +		"0: dcbst   0, %0;\n"
> +		"   addi    %0, %0, %4;\n"
> +		"   bdnz    0b;\n"
> +		"   sync;\n"
> +		"   mtctr %2;\n"
> +		"1: icbi    0, %1;\n"
> +		"   addi    %1, %1, %4;\n"
> +		"   bdnz    1b;\n"
> +		"   sync;\n"
> +		"   mtmsr %5;\n"
> +		"   isync;\n"
> +		: "+&r" (loop1), "+&r" (loop2)
> +		: "r" (nb), "r" (msr), "i" (bytes), "r" (msr0)
> +		: "ctr", "memory");
> +}
> +#endif // !defined(CONFIG_PPC_8xx) && !defined(CONFIG_PPC64)
> +
>  /*
>   * This is called when a page has been modified by the kernel.
>   * It just marks the page as not i-cache clean.  We do the i-cache
> @@ -350,12 +464,47 @@ void flush_dcache_icache_page(struct page *page)
>  		__flush_dcache_icache(start);
>  		kunmap_atomic(start);
>  	} else {
> -		__flush_dcache_icache_phys(page_to_pfn(page) << PAGE_SHIFT);
> +		unsigned long addr = page_to_pfn(page) << PAGE_SHIFT;
> +
> +		if (flush_coherent_icache((void *)addr))

That's wrong:

arch/powerpc/mm/mem.c:469:29: error: passing argument 1 of 'flush_coherent_icache' makes integer from pointer without a cast [-Werror=int-conversion]
      if (flush_coherent_icache((void *)addr))

I fixed it.

> +			return;
> +		flush_dcache_icache_phys(addr);
>  	}
>  #endif
>  }
>  EXPORT_SYMBOL(flush_dcache_icache_page);
>  
> +/**
> + * __flush_dcache_icache(): Flush a particular page from the data cache to RAM.
> + * Note: this is necessary because the instruction cache does *not*
> + * snoop from the data cache.
> + *
> + * @page: the address of the page to flush
> + */
> +void __flush_dcache_icache(void *page)

I know it used to be called page, but I'd prefer we reserve that for
struct page * pointers.

So I'll rename it p.

> +{
> +	u64 addr = (u64)page;

This is wrong on 32-bit:

arch/powerpc/mm/mem.c:486:13: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
     u64 addr = (u64)page;

I fixed it to be unsigned long.

> +
> +	if (flush_coherent_icache(addr))
> +		return;
> +
> +	clean_dcache_range(addr, addr + PAGE_SIZE);
> +
> +	/*
> +	 * We don't flush the icache on 44x. Those have a virtual icache and we
> +	 * don't have access to the virtual address here (it's not the page
> +	 * vaddr but where it's mapped in user space). The flushing of the
> +	 * icache on these is handled elsewhere, when a change in the address
> +	 * space occurs, before returning to user space.
> +	 */
> +
> +	if (cpu_has_feature(MMU_FTR_TYPE_44x))
> +		return;
> +
> +	invalidate_icache_range(addr, addr + PAGE_SIZE);
> +}
> +EXPORT_SYMBOL(__flush_dcache_icache);

This was not previously exported, and doesn't need to be AFAICS, so I
dropped the export.

cheers

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 4/6] powerpc: Convert flush_icache_range & friends to C
@ 2019-11-12 10:49     ` Michael Ellerman
  0 siblings, 0 replies; 28+ messages in thread
From: Michael Ellerman @ 2019-11-12 10:49 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: David Hildenbrand, linux-kernel, Nicholas Piggin, Qian Cai,
	Greg Kroah-Hartman, Paul Mackerras, Thomas Gleixner,
	linuxppc-dev, Andrew Morton, Allison Randal

"Alastair D'Silva" <alastair@au1.ibm.com> writes:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Similar to commit 22e9c88d486a
> ("powerpc/64: reuse PPC32 static inline flush_dcache_range()")
> this patch converts the following ASM symbols to C:
>     flush_icache_range()
>     __flush_dcache_icache()
>     __flush_dcache_icache_phys()
>
> This was done as we discovered a long-standing bug where the length of the
> range was truncated due to using a 32 bit shift instead of a 64 bit one.
>
> By converting these functions to C, it becomes easier to maintain.
>
> flush_dcache_icache_phys() retains a critical assembler section as we must
> ensure there are no memory accesses while the data MMU is disabled
> (authored by Christophe Leroy). Since this has no external callers, it has
> also been made static, allowing the compiler to inline it within
> flush_dcache_icache_page().
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  arch/powerpc/include/asm/cache.h      |  26 ++---
>  arch/powerpc/include/asm/cacheflush.h |  24 ++--
>  arch/powerpc/kernel/misc_32.S         | 120 --------------------
>  arch/powerpc/kernel/misc_64.S         | 102 -----------------
>  arch/powerpc/mm/mem.c                 | 151 +++++++++++++++++++++++++-
>  5 files changed, 172 insertions(+), 251 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cache.h b/arch/powerpc/include/asm/cache.h
> index afb88754e0e0..b809095cb1ba 100644
> --- a/arch/powerpc/include/asm/cache.h
> +++ b/arch/powerpc/include/asm/cache.h
> @@ -98,20 +98,7 @@ static inline u32 l1_icache_bytes(void)
>  #endif
>  #endif /* ! __ASSEMBLY__ */
  
Because of the above ...

> -#if defined(__ASSEMBLY__)
> -/*
> - * For a snooping icache, we still need a dummy icbi to purge all the
> - * prefetched instructions from the ifetch buffers. We also need a sync
> - * before the icbi to order the the actual stores to memory that might
> - * have modified instructions with the icbi.
> - */
> -#define PURGE_PREFETCHED_INS	\
> -	sync;			\
> -	icbi	0,r3;		\
> -	sync;			\
> -	isync
> -
> -#else
> +#if !defined(__ASSEMBLY__)

And this, we now end up with two adjacent !defined(__ASSEMBLY__) blocks.
So I merged them.

>  #define __read_mostly __attribute__((__section__(".data..read_mostly")))
>  
>  #ifdef CONFIG_PPC_BOOK3S_32
> @@ -145,6 +132,17 @@ static inline void dcbst(void *addr)
>  {
>  	__asm__ __volatile__ ("dcbst 0, %0" : : "r"(addr) : "memory");
>  }
> +
> +static inline void icbi(void *addr)
> +{
> +	asm volatile ("icbi 0, %0" : : "r"(addr) : "memory");
> +}
> +
> +static inline void iccci(void *addr)
> +{
> +	asm volatile ("iccci 0, %0" : : "r"(addr) : "memory");
> +}
> +
>  #endif /* !__ASSEMBLY__ */
>  #endif /* __KERNEL__ */
>  #endif /* _ASM_POWERPC_CACHE_H */
> diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
> index ed57843ef452..4a1c9f0200e1 100644
> --- a/arch/powerpc/include/asm/cacheflush.h
> +++ b/arch/powerpc/include/asm/cacheflush.h
> @@ -42,24 +42,20 @@ extern void flush_dcache_page(struct page *page);
>  #define flush_dcache_mmap_lock(mapping)		do { } while (0)
>  #define flush_dcache_mmap_unlock(mapping)	do { } while (0)
>  
> -extern void flush_icache_range(unsigned long, unsigned long);
> +void flush_icache_range(unsigned long start, unsigned long stop);
>  extern void flush_icache_user_range(struct vm_area_struct *vma,
>  				    struct page *page, unsigned long addr,
>  				    int len);
> -extern void __flush_dcache_icache(void *page_va);
>  extern void flush_dcache_icache_page(struct page *page);
> -#if defined(CONFIG_PPC32) && !defined(CONFIG_BOOKE)
> -extern void __flush_dcache_icache_phys(unsigned long physaddr);
> -#else
> -static inline void __flush_dcache_icache_phys(unsigned long physaddr)
> -{
> -	BUG();
> -}
> -#endif
> -
> -/*
> - * Write any modified data cache blocks out to memory and invalidate them.
> - * Does not invalidate the corresponding instruction cache blocks.
> +void __flush_dcache_icache(void *page);
> +
> +/**
> + * flush_dcache_range(): Write any modified data cache blocks out to memory and
> + * invalidate them. Does not invalidate the corresponding instruction cache
> + * blocks.
> + *
> + * @start: the start address
> + * @stop: the stop address (exclusive)
>   */
>  static inline void flush_dcache_range(unsigned long start, unsigned long stop)
>  {
> diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
> index 82df4b09e79f..f4e4a1926a7a 100644
> --- a/arch/powerpc/kernel/misc_32.S
> +++ b/arch/powerpc/kernel/misc_32.S
> @@ -316,126 +316,6 @@ _GLOBAL(flush_instruction_cache)
>  EXPORT_SYMBOL(flush_instruction_cache)
>  #endif /* CONFIG_PPC_8xx */
>  
> -/*
> - * Write any modified data cache blocks out to memory
> - * and invalidate the corresponding instruction cache blocks.
> - * This is a no-op on the 601.
> - *
> - * flush_icache_range(unsigned long start, unsigned long stop)
> - */
> -_GLOBAL(flush_icache_range)
> -#if defined(CONFIG_PPC_BOOK3S_601) || defined(CONFIG_E200)
> -	PURGE_PREFETCHED_INS
> -	blr				/* for 601 and e200, do nothing */
> -#else
> -	rlwinm	r3,r3,0,0,31 - L1_CACHE_SHIFT
> -	subf	r4,r3,r4
> -	addi	r4,r4,L1_CACHE_BYTES - 1
> -	srwi.	r4,r4,L1_CACHE_SHIFT
> -	beqlr
> -	mtctr	r4
> -	mr	r6,r3
> -1:	dcbst	0,r3
> -	addi	r3,r3,L1_CACHE_BYTES
> -	bdnz	1b
> -	sync				/* wait for dcbst's to get to ram */
> -#ifndef CONFIG_44x
> -	mtctr	r4
> -2:	icbi	0,r6
> -	addi	r6,r6,L1_CACHE_BYTES
> -	bdnz	2b
> -#else
> -	/* Flash invalidate on 44x because we are passed kmapped addresses and
> -	   this doesn't work for userspace pages due to the virtually tagged
> -	   icache.  Sigh. */
> -	iccci	0, r0
> -#endif
> -	sync				/* additional sync needed on g4 */
> -	isync
> -	blr
> -#endif
> -_ASM_NOKPROBE_SYMBOL(flush_icache_range)
> -EXPORT_SYMBOL(flush_icache_range)
> -
> -/*
> - * Flush a particular page from the data cache to RAM.
> - * Note: this is necessary because the instruction cache does *not*
> - * snoop from the data cache.
> - * This is a no-op on the 601 and e200 which have a unified cache.
> - *
> - *	void __flush_dcache_icache(void *page)
> - */
> -_GLOBAL(__flush_dcache_icache)
> -#if defined(CONFIG_PPC_BOOK3S_601) || defined(CONFIG_E200)
> -	PURGE_PREFETCHED_INS
> -	blr
> -#else
> -	rlwinm	r3,r3,0,0,31-PAGE_SHIFT		/* Get page base address */
> -	li	r4,PAGE_SIZE/L1_CACHE_BYTES	/* Number of lines in a page */
> -	mtctr	r4
> -	mr	r6,r3
> -0:	dcbst	0,r3				/* Write line to ram */
> -	addi	r3,r3,L1_CACHE_BYTES
> -	bdnz	0b
> -	sync
> -#ifdef CONFIG_44x
> -	/* We don't flush the icache on 44x. Those have a virtual icache
> -	 * and we don't have access to the virtual address here (it's
> -	 * not the page vaddr but where it's mapped in user space). The
> -	 * flushing of the icache on these is handled elsewhere, when
> -	 * a change in the address space occurs, before returning to
> -	 * user space
> -	 */
> -BEGIN_MMU_FTR_SECTION
> -	blr
> -END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_44x)
> -#endif /* CONFIG_44x */
> -	mtctr	r4
> -1:	icbi	0,r6
> -	addi	r6,r6,L1_CACHE_BYTES
> -	bdnz	1b
> -	sync
> -	isync
> -	blr
> -#endif
> -
> -#ifndef CONFIG_BOOKE
> -/*
> - * Flush a particular page from the data cache to RAM, identified
> - * by its physical address.  We turn off the MMU so we can just use
> - * the physical address (this may be a highmem page without a kernel
> - * mapping).
> - *
> - *	void __flush_dcache_icache_phys(unsigned long physaddr)
> - */
> -_GLOBAL(__flush_dcache_icache_phys)
> -#if defined(CONFIG_PPC_BOOK3S_601) || defined(CONFIG_E200)
> -	PURGE_PREFETCHED_INS
> -	blr					/* for 601 and e200, do nothing */
> -#else
> -	mfmsr	r10
> -	rlwinm	r0,r10,0,28,26			/* clear DR */
> -	mtmsr	r0
> -	isync
> -	rlwinm	r3,r3,0,0,31-PAGE_SHIFT		/* Get page base address */
> -	li	r4,PAGE_SIZE/L1_CACHE_BYTES	/* Number of lines in a page */
> -	mtctr	r4
> -	mr	r6,r3
> -0:	dcbst	0,r3				/* Write line to ram */
> -	addi	r3,r3,L1_CACHE_BYTES
> -	bdnz	0b
> -	sync
> -	mtctr	r4
> -1:	icbi	0,r6
> -	addi	r6,r6,L1_CACHE_BYTES
> -	bdnz	1b
> -	sync
> -	mtmsr	r10				/* restore DR */
> -	isync
> -	blr
> -#endif
> -#endif /* CONFIG_BOOKE */
> -
>  /*
>   * Copy a whole page.  We use the dcbz instruction on the destination
>   * to reduce memory traffic (it eliminates the unnecessary reads of
> diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
> index 9bc0aa9aeb65..ff20c253f273 100644
> --- a/arch/powerpc/kernel/misc_64.S
> +++ b/arch/powerpc/kernel/misc_64.S
> @@ -49,108 +49,6 @@ _GLOBAL(call_do_irq)
>  	mtlr	r0
>  	blr
>  
> -	.section	".toc","aw"
> -PPC64_CACHES:
> -	.tc		ppc64_caches[TC],ppc64_caches
> -	.section	".text"
> -
> -/*
> - * Write any modified data cache blocks out to memory
> - * and invalidate the corresponding instruction cache blocks.
> - *
> - * flush_icache_range(unsigned long start, unsigned long stop)
> - *
> - *   flush all bytes from start through stop-1 inclusive
> - */
> -
> -_GLOBAL_TOC(flush_icache_range)
> -BEGIN_FTR_SECTION
> -	PURGE_PREFETCHED_INS
> -	blr
> -END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> -/*
> - * Flush the data cache to memory 
> - * 
> - * Different systems have different cache line sizes
> - * and in some cases i-cache and d-cache line sizes differ from
> - * each other.
> - */
> - 	ld	r10,PPC64_CACHES@toc(r2)
> -	lwz	r7,DCACHEL1BLOCKSIZE(r10)/* Get cache block size */
> -	addi	r5,r7,-1
> -	andc	r6,r3,r5		/* round low to line bdy */
> -	subf	r8,r6,r4		/* compute length */
> -	add	r8,r8,r5		/* ensure we get enough */
> -	lwz	r9,DCACHEL1LOGBLOCKSIZE(r10)	/* Get log-2 of cache block size */
> -	srd.	r8,r8,r9		/* compute line count */
> -	beqlr				/* nothing to do? */
> -	mtctr	r8
> -1:	dcbst	0,r6
> -	add	r6,r6,r7
> -	bdnz	1b
> -	sync
> -
> -/* Now invalidate the instruction cache */
> -	
> -	lwz	r7,ICACHEL1BLOCKSIZE(r10)	/* Get Icache block size */
> -	addi	r5,r7,-1
> -	andc	r6,r3,r5		/* round low to line bdy */
> -	subf	r8,r6,r4		/* compute length */
> -	add	r8,r8,r5
> -	lwz	r9,ICACHEL1LOGBLOCKSIZE(r10)	/* Get log-2 of Icache block size */
> -	srd.	r8,r8,r9		/* compute line count */
> -	beqlr				/* nothing to do? */
> -	mtctr	r8
> -2:	icbi	0,r6
> -	add	r6,r6,r7
> -	bdnz	2b
> -	isync
> -	blr
> -_ASM_NOKPROBE_SYMBOL(flush_icache_range)
> -EXPORT_SYMBOL(flush_icache_range)
> -
> -/*
> - * Flush a particular page from the data cache to RAM.
> - * Note: this is necessary because the instruction cache does *not*
> - * snoop from the data cache.
> - *
> - *	void __flush_dcache_icache(void *page)
> - */
> -_GLOBAL(__flush_dcache_icache)
> -/*
> - * Flush the data cache to memory 
> - * 
> - * Different systems have different cache line sizes
> - */
> -
> -BEGIN_FTR_SECTION
> -	PURGE_PREFETCHED_INS
> -	blr
> -END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)
> -
> -/* Flush the dcache */
> - 	ld	r7,PPC64_CACHES@toc(r2)
> -	clrrdi	r3,r3,PAGE_SHIFT           	    /* Page align */
> -	lwz	r4,DCACHEL1BLOCKSPERPAGE(r7)	/* Get # dcache blocks per page */
> -	lwz	r5,DCACHEL1BLOCKSIZE(r7)	/* Get dcache block size */
> -	mr	r6,r3
> -	mtctr	r4
> -0:	dcbst	0,r6
> -	add	r6,r6,r5
> -	bdnz	0b
> -	sync
> -
> -/* Now invalidate the icache */	
> -
> -	lwz	r4,ICACHEL1BLOCKSPERPAGE(r7)	/* Get # icache blocks per page */
> -	lwz	r5,ICACHEL1BLOCKSIZE(r7)	/* Get icache block size */
> -	mtctr	r4
> -1:	icbi	0,r3
> -	add	r3,r3,r5
> -	bdnz	1b
> -	isync
> -	blr
> -
>  _GLOBAL(__bswapdi2)
>  EXPORT_SYMBOL(__bswapdi2)
>  	srdi	r8,r3,32
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index be941d382c8d..54d61ba15e93 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -318,6 +318,120 @@ void free_initmem(void)
>  	free_initmem_default(POISON_FREE_INITMEM);
>  }
>  
> +/**
> + * flush_coherent_icache() - if a CPU has a coherent icache, flush it
> + * @addr: The base address to use (can be any valid address, the whole cache will be flushed)
> + * Return true if the cache was flushed, false otherwise
> + */
> +static inline bool flush_coherent_icache(unsigned long addr)
> +{
> +	/*
> +	 * For a snooping icache, we still need a dummy icbi to purge all the
> +	 * prefetched instructions from the ifetch buffers. We also need a sync
> +	 * before the icbi to order the the actual stores to memory that might
> +	 * have modified instructions with the icbi.
> +	 */
> +	if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
> +		mb(); /* sync */
> +		icbi((void *)addr);
> +		mb(); /* sync */
> +		isync();
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +/**
> + * invalidate_icache_range() - Flush the icache by issuing icbi across an address range
> + * @start: the start address
> + * @stop: the stop address (exclusive)
> + */
> +static void invalidate_icache_range(unsigned long start, unsigned long stop)
> +{
> +	unsigned long shift = l1_icache_shift();
> +	unsigned long bytes = l1_icache_bytes();
> +	char *addr = (char *)(start & ~(bytes - 1));
> +	unsigned long size = stop - (unsigned long)addr + (bytes - 1);
> +	unsigned long i;
> +
> +	for (i = 0; i < size >> shift; i++, addr += bytes)
> +		icbi(addr);
> +
> +	mb(); /* sync */
> +	isync();
> +}
> +
> +/**
> + * flush_icache_range: Write any modified data cache blocks out to memory
> + * and invalidate the corresponding blocks in the instruction cache
> + *
> + * Generic code will call this after writing memory, before executing from it.
> + *
> + * @start: the start address
> + * @stop: the stop address (exclusive)
> + */
> +void flush_icache_range(unsigned long start, unsigned long stop)
> +{
> +	if (flush_coherent_icache(start))
> +		return;
> +
> +	clean_dcache_range(start, stop);
> +
> +	if (IS_ENABLED(CONFIG_44x)) {
> +		/*
> +		 * Flash invalidate on 44x because we are passed kmapped
> +		 * addresses and this doesn't work for userspace pages due to
> +		 * the virtually tagged icache.
> +		 */
> +		iccci((void *)start);
> +		mb(); /* sync */
> +		isync();
> +	} else
> +		invalidate_icache_range(start, stop);
> +}
> +EXPORT_SYMBOL(flush_icache_range);
> +
> +#if !defined(CONFIG_PPC_8xx) && !defined(CONFIG_PPC64)
> +/**
> + * flush_dcache_icache_phys() - Flush a page by it's physical address
> + * @physaddr: the physical address of the page
> + */
> +static void flush_dcache_icache_phys(unsigned long physaddr)
> +{
> +	unsigned long bytes = l1_dcache_bytes();
> +	unsigned long nb = PAGE_SIZE / bytes;
> +	unsigned long addr = physaddr & PAGE_MASK;
> +	unsigned long msr, msr0;
> +	unsigned long loop1 = addr, loop2 = addr;
> +
> +	msr0 = mfmsr();
> +	msr = msr0 & ~MSR_DR;
> +	/*
> +	 * This must remain as ASM to prevent potential memory accesses
> +	 * while the data MMU is disabled
> +	 */
> +	asm volatile(
> +		"   mtctr %2;\n"
> +		"   mtmsr %3;\n"
> +		"   isync;\n"
> +		"0: dcbst   0, %0;\n"
> +		"   addi    %0, %0, %4;\n"
> +		"   bdnz    0b;\n"
> +		"   sync;\n"
> +		"   mtctr %2;\n"
> +		"1: icbi    0, %1;\n"
> +		"   addi    %1, %1, %4;\n"
> +		"   bdnz    1b;\n"
> +		"   sync;\n"
> +		"   mtmsr %5;\n"
> +		"   isync;\n"
> +		: "+&r" (loop1), "+&r" (loop2)
> +		: "r" (nb), "r" (msr), "i" (bytes), "r" (msr0)
> +		: "ctr", "memory");
> +}
> +#endif // !defined(CONFIG_PPC_8xx) && !defined(CONFIG_PPC64)
> +
>  /*
>   * This is called when a page has been modified by the kernel.
>   * It just marks the page as not i-cache clean.  We do the i-cache
> @@ -350,12 +464,47 @@ void flush_dcache_icache_page(struct page *page)
>  		__flush_dcache_icache(start);
>  		kunmap_atomic(start);
>  	} else {
> -		__flush_dcache_icache_phys(page_to_pfn(page) << PAGE_SHIFT);
> +		unsigned long addr = page_to_pfn(page) << PAGE_SHIFT;
> +
> +		if (flush_coherent_icache((void *)addr))

That's wrong:

arch/powerpc/mm/mem.c:469:29: error: passing argument 1 of 'flush_coherent_icache' makes integer from pointer without a cast [-Werror=int-conversion]
      if (flush_coherent_icache((void *)addr))

I fixed it.

> +			return;
> +		flush_dcache_icache_phys(addr);
>  	}
>  #endif
>  }
>  EXPORT_SYMBOL(flush_dcache_icache_page);
>  
> +/**
> + * __flush_dcache_icache(): Flush a particular page from the data cache to RAM.
> + * Note: this is necessary because the instruction cache does *not*
> + * snoop from the data cache.
> + *
> + * @page: the address of the page to flush
> + */
> +void __flush_dcache_icache(void *page)

I know it used to be called page, but I'd prefer we reserve that for
struct page * pointers.

So I'll rename it p.

> +{
> +	u64 addr = (u64)page;

This is wrong on 32-bit:

arch/powerpc/mm/mem.c:486:13: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
     u64 addr = (u64)page;

I fixed it to be unsigned long.

> +
> +	if (flush_coherent_icache(addr))
> +		return;
> +
> +	clean_dcache_range(addr, addr + PAGE_SIZE);
> +
> +	/*
> +	 * We don't flush the icache on 44x. Those have a virtual icache and we
> +	 * don't have access to the virtual address here (it's not the page
> +	 * vaddr but where it's mapped in user space). The flushing of the
> +	 * icache on these is handled elsewhere, when a change in the address
> +	 * space occurs, before returning to user space.
> +	 */
> +
> +	if (cpu_has_feature(MMU_FTR_TYPE_44x))
> +		return;
> +
> +	invalidate_icache_range(addr, addr + PAGE_SIZE);
> +}
> +EXPORT_SYMBOL(__flush_dcache_icache);

This was not previously exported, and doesn't need to be AFAICS, so I
dropped the export.

cheers

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 1/6] powerpc: Allow flush_icache_range to work across ranges >4GB
  2019-11-04  2:32   ` Alastair D'Silva
@ 2019-11-14  9:08     ` Michael Ellerman
  -1 siblings, 0 replies; 28+ messages in thread
From: Michael Ellerman @ 2019-11-14  9:08 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: Greg Kroah-Hartman, David Hildenbrand, linux-kernel, stable,
	Paul Mackerras, Nicholas Piggin, Qian Cai, Thomas Gleixner,
	linuxppc-dev, Andrew Morton, Allison Randal

On Mon, 2019-11-04 at 02:32:53 UTC, "Alastair D'Silva" wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> When calling flush_icache_range with a size >4GB, we were masking
> off the upper 32 bits, so we would incorrectly flush a range smaller
> than intended.
> 
> This patch replaces the 32 bit shifts with 64 bit ones, so that
> the full size is accounted for.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Cc: stable@vger.kernel.org

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/29430fae82073d39b1b881a3cd507416a56a363f

cheers

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5 1/6] powerpc: Allow flush_icache_range to work across ranges >4GB
@ 2019-11-14  9:08     ` Michael Ellerman
  0 siblings, 0 replies; 28+ messages in thread
From: Michael Ellerman @ 2019-11-14  9:08 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: David Hildenbrand, Greg Kroah-Hartman, linux-kernel,
	Nicholas Piggin, Paul Mackerras, stable, Qian Cai,
	Thomas Gleixner, linuxppc-dev, Andrew Morton, Allison Randal

On Mon, 2019-11-04 at 02:32:53 UTC, "Alastair D'Silva" wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> When calling flush_icache_range with a size >4GB, we were masking
> off the upper 32 bits, so we would incorrectly flush a range smaller
> than intended.
> 
> This patch replaces the 32 bit shifts with 64 bit ones, so that
> the full size is accounted for.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Cc: stable@vger.kernel.org

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/29430fae82073d39b1b881a3cd507416a56a363f

cheers

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2019-11-14 10:19 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-04  2:32 [PATCH v5 0/6] powerpc: convert cache asm to C Alastair D'Silva
2019-11-04  2:32 ` Alastair D'Silva
2019-11-04  2:32 ` [PATCH v5 1/6] powerpc: Allow flush_icache_range to work across ranges >4GB Alastair D'Silva
2019-11-04  2:32   ` Alastair D'Silva
2019-11-04 19:43   ` Segher Boessenkool
2019-11-04 19:43     ` Segher Boessenkool
2019-11-05  6:04     ` Christophe Leroy
2019-11-05  6:04       ` Christophe Leroy
2019-11-06 17:49       ` Segher Boessenkool
2019-11-14  9:08   ` Michael Ellerman
2019-11-14  9:08     ` Michael Ellerman
2019-11-04  2:32 ` [PATCH v5 2/6] powerpc: Allow 64bit VDSO __kernel_sync_dicache " Alastair D'Silva
2019-11-04  2:32   ` Alastair D'Silva
2019-11-04  2:32 ` [PATCH v5 3/6] powerpc: define helpers to get L1 icache sizes Alastair D'Silva
2019-11-04  2:32   ` Alastair D'Silva
2019-11-04  2:32 ` [PATCH v5 4/6] powerpc: Convert flush_icache_range & friends to C Alastair D'Silva
2019-11-04  2:32   ` Alastair D'Silva
2019-11-04  4:54   ` kbuild test robot
2019-11-04  4:54     ` kbuild test robot
2019-11-04  4:54     ` kbuild test robot
2019-11-12 10:49   ` Michael Ellerman
2019-11-12 10:49     ` Michael Ellerman
2019-11-04  2:32 ` [PATCH v5 5/6] powerpc: Chunk calls to flush_dcache_range in arch_*_memory Alastair D'Silva
2019-11-04  2:32   ` Alastair D'Silva
2019-11-07 11:54   ` Michael Ellerman
2019-11-07 11:54     ` Michael Ellerman
2019-11-04  2:32 ` [PATCH v5 6/6] powerpc: Don't flush caches when adding memory Alastair D'Silva
2019-11-04  2:32   ` Alastair D'Silva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.