On Thu, 2019-08-15 at 09:29 +0200, christophe leroy wrote:

Le 15/08/2019 à 06:10, Alastair D'Silva a écrit :

quoted

From: Alastair D'Silva <redacted>

Similar to commit 22e9c88d486a
("powerpc/64: reuse PPC32 static inline flush_dcache_range()")
this patch converts flush_icache_range() to C, and reimplements the
following functions as wrappers around it:
__flush_dcache_icache
__flush_dcache_icache_phys

Not sure you can do that for __flush_dcache_icache_phys(), see
detailed 
comments below

quoted

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.

Signed-off-by: Alastair D'Silva <redacted>
---
  arch/powerpc/include/asm/cache.h      |  26 +++---
  arch/powerpc/include/asm/cacheflush.h |  32 ++++---
  arch/powerpc/kernel/misc_32.S         | 117 -------------------
-------
  arch/powerpc/kernel/misc_64.S         |  97 ---------------------
  arch/powerpc/mm/mem.c                 |  71 +++++++++++++++-
  5 files changed, 102 insertions(+), 241 deletions(-)

diff --git a/arch/powerpc/include/asm/cache.h

b/arch/powerpc/include/asm/cache.h
index f852d5cd746c..728f154204db 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

Is this still used anywhere now ?

No, this patch removes all users of it.

quoted

-
-#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 %y0" : : "Z"(*(u8 *)addr) :

"memory");
  }
+
+static inline void icbi(void *addr)
+{
+	__asm__ __volatile__ ("icbi 0, %0" : : "r"(addr) : "memory");
+}
+
+static inline void iccci(void)
+{
+	__asm__ __volatile__ ("iccci 0, r0");

I think you need the "memory" clobber too here.

Thanks, I'll add that.

quoted

+}
+
  #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..4c3377aff8ed 100644

--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h

@@ -42,24 +42,18 @@ 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.
+/**
+ * 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)
  {

@@ -82,6 +76,20 @@ static inline void flush_dcache_range(unsigned

long start, unsigned long stop)
  		isync();
  }
  
+/**
+ * __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
+ */
+static inline void __flush_dcache_icache(void *page)
+{
+	unsigned long page_addr = (unsigned long)page;

The function is small enough to call this addr instead of page_addr 
without ambiguity.

Ok

quoted

+
+	flush_icache_range(page_addr, page_addr + PAGE_SIZE);

Finally, I think that's not so simple. For the 44x, if
MMU_FTR_TYPE_44x 
is set, that's just a clean_dcache_range().
If that feature is not set, it looks like a standard 
flush_icache_range() but without the special CONFIG_4xx handling
with 
iccci we have in flush_icache_range()

Hmm, I'll have to hack at this a bit more.

<snip>
-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819