Re: [PATCH 3/6] powerpc: Convert flush_icache_range & friends to C
Le 15/08/2019 à 09:29, christophe leroy a écrit :
Le 15/08/2019 à 06:10, Alastair D'Silva a écrit :
From: Alastair D'Silva
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
I just sent you an RFC patch that could be the way to convert
__flush_dcache_icache_phys() to C.
Feel free to modify it as wished and include it in your series.
Christophe
Re: [PATCH 3/6] powerpc: Convert flush_icache_range & friends to C
On Thu, 2019-08-15 at 09:29 +0200, christophe leroy wrote:
>
> Le 15/08/2019 à 06:10, Alastair D'Silva a écrit :
> > From: Alastair D'Silva
> >
> > 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
>
> > 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
> > ---
> > 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; \
> > - icbi0,r3; \
> > - sync; \
> > - isync
>
> Is this still used anywhere now ?
No, this patch removes all users of it.
> > -
> > -#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.
> > +}
> > +
> > #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
Re: [PATCH 3/6] powerpc: Convert flush_icache_range & friends to C
Le 15/08/2019 à 06:10, Alastair D'Silva a écrit :
From: Alastair D'Silva
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
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
---
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; \
- icbi0,r3; \
- sync; \
- isync
Is this still used anywhere now ?
-
-#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.
+}
+
#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.
+
+ 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()
+}
+
/*
* Write any modified data cache blocks out to memory.
* Does not invalidate the
[PATCH 3/6] powerpc: Convert flush_icache_range & friends to C
From: Alastair D'Silva
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
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
---
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; \
- icbi0,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 %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");
+}
+
#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;
+
+ flush_icache_range(page_addr, page_addr + PAGE_SIZE);
+}
+
/*
* Write any modified data cache blocks out to memory.
* Does not invalidate the corresponding cache lines (especially for
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index fe4bd321730e..12b95e6799d4 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -318,123 +318,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_UNIFIED_ID_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)
-BEGIN_FTR_SECTION
-
