[RFC 00/11] Cache handling updates

[RFC 00/11] Cache handling updates

[Russell King]

For RFC, this series of patches cleans up and updates various aspects
of the kernel's cache handling.  The 'fix errata 411920' patch is one
which I'm intending pushing to mainline once the current set has been
pulled.

A few things of note from this:
1. flush flush_dcache_page, on VIPT aliasing caches, we ended up
   flushing the I-cache multiple times.  This has been fixed.
2. we now avoid flush_dcache_page operating on the ZERO_PAGE (for bss)
3. ZERO_PAGE can now be allocated from highmem
4. reinforced I-cache handling for VIVT ASID-tagged caches (to be more
   like VIVT caches were)
5. fix the RX->RW->RX issue by flushing the I-cache in flush_cache_range()
6. it seems pointless flushing the I-cache of VIPT caches in flush_cache_mm

Russell King (11):
  ARM: Reduce __flush_dcache_page() visibility
  ARM: Fix errata 411920 workarounds
  ARM: move __flush_icache_all() out of flush_pfn_alias()
  ARM: Remove __flush_icache_all() from __flush_dcache_page()
  ARM: Avoid flush_dcache_page() for zero page
  ARM: Avoid duplicated implementation for VIVT cache flushing
  ARM: Allocate ZERO_PAGE from highmem
  ARM: Avoid evaluating page_address() multiple times
  ARM: Add I-cache invalidation for VIVT ASID tagged caches
  ARM: arrange for flush_cache_range() to always flush the I-cache
  ARM: avoid flushing I-cache in flush_cache_mm()

 arch/arm/include/asm/cacheflush.h |   27 ++++++++++---
 arch/arm/mm/context.c             |    4 +-
 arch/arm/mm/fault-armv.c          |    9 ++++
 arch/arm/mm/flush.c               |   74 ++++++++++++++++---------------------
 arch/arm/mm/init.c                |    9 ++++
 arch/arm/mm/mm.h                  |    2 +
 arch/arm/mm/mmu.c                 |   10 -----
 7 files changed, 74 insertions(+), 61 deletions(-)

[RFC 01/11] ARM: Reduce __flush_dcache_page() visibility

[Russell King]

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/include/asm/cacheflush.h |    2 --
 arch/arm/mm/fault-armv.c          |    2 ++
 arch/arm/mm/mm.h                  |    2 ++
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
index fd03fb6..3147f75 100644
--- a/arch/arm/include/asm/cacheflush.h
+++ b/arch/arm/include/asm/cacheflush.h
@@ -410,8 +410,6 @@ extern void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
  */
 extern void flush_dcache_page(struct page *);
 
-extern void __flush_dcache_page(struct address_space *mapping, struct page *page);
-
 static inline void __flush_icache_all(void)
 {
 	asm("mcr	p15, 0, %0, c7, c5, 0	@ invalidate I-cache\n"
diff --git a/arch/arm/mm/fault-armv.c b/arch/arm/mm/fault-armv.c
index d0d17b6..830cf17 100644
--- a/arch/arm/mm/fault-armv.c
+++ b/arch/arm/mm/fault-armv.c
@@ -23,6 +23,8 @@
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
 
+#include "mm.h"
+
 static unsigned long shared_pte_mask = L_PTE_MT_BUFFERABLE;
 
 /*
diff --git a/arch/arm/mm/mm.h b/arch/arm/mm/mm.h
index c4f6f05..a888363 100644
--- a/arch/arm/mm/mm.h
+++ b/arch/arm/mm/mm.h
@@ -24,6 +24,8 @@ struct mem_type {
 
 const struct mem_type *get_mem_type(unsigned int type);
 
+extern void __flush_dcache_page(struct address_space *mapping, struct page *page);
+
 #endif
 
 struct map_desc;
-- 
1.6.2.5

[RFC 02/11] ARM: Fix errata 411920 workarounds

[Russell King]

Errata 411920 indicates that any "invalidate entire instruction cache"
operation can fail if the right conditions are present.  This is not
limited just to those operations in flush.c, but elsewhere.  Place the
workaround in the already existing __flush_icache_all() function
instead.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/include/asm/cacheflush.h |    5 +++++
 arch/arm/mm/context.c             |    4 ++--
 arch/arm/mm/flush.c               |   31 ++++++-------------------------
 3 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
index 3147f75..af0624c 100644
--- a/arch/arm/include/asm/cacheflush.h
+++ b/arch/arm/include/asm/cacheflush.h
@@ -412,9 +412,14 @@ extern void flush_dcache_page(struct page *);
 
 static inline void __flush_icache_all(void)
 {
+#ifdef CONFIG_ARM_ERRATA_411920
+	extern void v6_icache_inval_all(void);
+	v6_icache_inval_all();
+#else
 	asm("mcr	p15, 0, %0, c7, c5, 0	@ invalidate I-cache\n"
 	    :
 	    : "r" (0));
+#endif
 }
 
 #define ARCH_HAS_FLUSH_ANON_PAGE
diff --git a/arch/arm/mm/context.c b/arch/arm/mm/context.c
index 6bda76a..d4f8275 100644
--- a/arch/arm/mm/context.c
+++ b/arch/arm/mm/context.c
@@ -50,8 +50,8 @@ void __new_context(struct mm_struct *mm)
 		isb();
 		flush_tlb_all();
 		if (icache_is_vivt_asid_tagged()) {
-			asm("mcr	p15, 0, %0, c7, c5, 0	@ invalidate I-cache\n"
-			    "mcr	p15, 0, %0, c7, c5, 6	@ flush BTAC/BTB\n"
+			__flush_icache_all();
+			asm("mcr	p15, 0, %0, c7, c5, 6	@ flush BTAC/BTB"
 			    :
 			    : "r" (0));
 			dsb();
diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index b279429..7f294f3 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -18,10 +18,6 @@
 
 #include "mm.h"
 
-#ifdef CONFIG_ARM_ERRATA_411920
-extern void v6_icache_inval_all(void);
-#endif
-
 #ifdef CONFIG_CPU_CACHE_VIPT
 
 #define ALIAS_FLUSH_START	0xffff4000
@@ -35,16 +31,11 @@ static void flush_pfn_alias(unsigned long pfn, unsigned long vaddr)
 	flush_tlb_kernel_page(to);
 
 	asm(	"mcrr	p15, 0, %1, %0, c14\n"
-	"	mcr	p15, 0, %2, c7, c10, 4\n"
-#ifndef CONFIG_ARM_ERRATA_411920
-	"	mcr	p15, 0, %2, c7, c5, 0\n"
-#endif
+	"	mcr	p15, 0, %2, c7, c10, 4"
 	    :
 	    : "r" (to), "r" (to + PAGE_SIZE - L1_CACHE_BYTES), "r" (zero)
 	    : "cc");
-#ifdef CONFIG_ARM_ERRATA_411920
-	v6_icache_inval_all();
-#endif
+	__flush_icache_all();
 }
 
 void flush_cache_mm(struct mm_struct *mm)
@@ -57,16 +48,11 @@ void flush_cache_mm(struct mm_struct *mm)
 
 	if (cache_is_vipt_aliasing()) {
 		asm(	"mcr	p15, 0, %0, c7, c14, 0\n"
-		"	mcr	p15, 0, %0, c7, c10, 4\n"
-#ifndef CONFIG_ARM_ERRATA_411920
-		"	mcr	p15, 0, %0, c7, c5, 0\n"
-#endif
+		"	mcr	p15, 0, %0, c7, c10, 4"
 		    :
 		    : "r" (0)
 		    : "cc");
-#ifdef CONFIG_ARM_ERRATA_411920
-		v6_icache_inval_all();
-#endif
+		__flush_icache_all();
 	}
 }
 
@@ -81,16 +67,11 @@ void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned
 
 	if (cache_is_vipt_aliasing()) {
 		asm(	"mcr	p15, 0, %0, c7, c14, 0\n"
-		"	mcr	p15, 0, %0, c7, c10, 4\n"
-#ifndef CONFIG_ARM_ERRATA_411920
-		"	mcr	p15, 0, %0, c7, c5, 0\n"
-#endif
+		"	mcr	p15, 0, %0, c7, c10, 4"
 		    :
 		    : "r" (0)
 		    : "cc");
-#ifdef CONFIG_ARM_ERRATA_411920
-		v6_icache_inval_all();
-#endif
+		__flush_icache_all();
 	}
 }
 
-- 
1.6.2.5

[RFC 02/11] ARM: Fix errata 411920 workarounds

[Catalin Marinas]

On Sat, 2009-10-24 at 22:36 +0100, Russell King wrote:
> Errata 411920 indicates that any "invalidate entire instruction cache"
> operation can fail if the right conditions are present.  This is not
> limited just to those operations in flush.c, but elsewhere.  Place the
> workaround in the already existing __flush_icache_all() function
> instead.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  arch/arm/include/asm/cacheflush.h |    5 +++++
>  arch/arm/mm/context.c             |    4 ++--
>  arch/arm/mm/flush.c               |   31 ++++++-------------------------
>  3 files changed, 13 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
> index 3147f75..af0624c 100644
> --- a/arch/arm/include/asm/cacheflush.h
> +++ b/arch/arm/include/asm/cacheflush.h
> @@ -412,9 +412,14 @@ extern void flush_dcache_page(struct page *);
>  
>  static inline void __flush_icache_all(void)
>  {
> +#ifdef CONFIG_ARM_ERRATA_411920
> +	extern void v6_icache_inval_all(void);
> +	v6_icache_inval_all();
> +#else

I think some compiler complain of "extern" declarations inside a
function, not entirely sure.

>         asm("mcr        p15, 0, %0, c7, c5, 0   @ invalidate I-cache\n"
>             :
>             : "r" (0));
> +#endif
>  }

For correctness, can we add a dsb() at the end of this function so that
the I-cache operation is guaranteed to complete?

> diff --git a/arch/arm/mm/context.c b/arch/arm/mm/context.c
> index 6bda76a..d4f8275 100644
> --- a/arch/arm/mm/context.c
> +++ b/arch/arm/mm/context.c
> @@ -50,8 +50,8 @@ void __new_context(struct mm_struct *mm)
>  		isb();
>  		flush_tlb_all();
>  		if (icache_is_vivt_asid_tagged()) {
> -			asm("mcr	p15, 0, %0, c7, c5, 0	@ invalidate I-cache\n"
> -			    "mcr	p15, 0, %0, c7, c5, 6	@ flush BTAC/BTB\n"
> +			__flush_icache_all();
> +			asm("mcr	p15, 0, %0, c7, c5, 6	@ flush BTAC/BTB"

IIUC, we don't need to flush the BTAC after a full I-cache invalidation
(not sure why I added it).
 
-- 
Catalin

[RFC 02/11] ARM: Fix errata 411920 workarounds

[Russell King - ARM Linux]

On Wed, Oct 28, 2009 at 03:31:35PM +0000, Catalin Marinas wrote:
> On Sat, 2009-10-24 at 22:36 +0100, Russell King wrote:
> > Errata 411920 indicates that any "invalidate entire instruction cache"
> > operation can fail if the right conditions are present.  This is not
> > limited just to those operations in flush.c, but elsewhere.  Place the
> > workaround in the already existing __flush_icache_all() function
> > instead.
> > 
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > ---
> >  arch/arm/include/asm/cacheflush.h |    5 +++++
> >  arch/arm/mm/context.c             |    4 ++--
> >  arch/arm/mm/flush.c               |   31 ++++++-------------------------
> >  3 files changed, 13 insertions(+), 27 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
> > index 3147f75..af0624c 100644
> > --- a/arch/arm/include/asm/cacheflush.h
> > +++ b/arch/arm/include/asm/cacheflush.h
> > @@ -412,9 +412,14 @@ extern void flush_dcache_page(struct page *);
> >  
> >  static inline void __flush_icache_all(void)
> >  {
> > +#ifdef CONFIG_ARM_ERRATA_411920
> > +	extern void v6_icache_inval_all(void);
> > +	v6_icache_inval_all();
> > +#else
> 
> I think some compiler complain of "extern" declarations inside a
> function, not entirely sure.

It does not, it's an entirely valid scoping mechanism.

> >         asm("mcr        p15, 0, %0, c7, c5, 0   @ invalidate I-cache\n"
> >             :
> >             : "r" (0));
> > +#endif
> >  }
> 
> For correctness, can we add a dsb() at the end of this function so that
> the I-cache operation is guaranteed to complete?

Two reasons why not to in this patch:

1. it changes the produced code, and therefore is another functional change.
2. it means we end up with a sequence like:

	data cache maintainence
	dsb();
	instruction cache maintainence
	dsb();

The dsb() between the two operations probably isn't required.

> > diff --git a/arch/arm/mm/context.c b/arch/arm/mm/context.c
> > index 6bda76a..d4f8275 100644
> > --- a/arch/arm/mm/context.c
> > +++ b/arch/arm/mm/context.c
> > @@ -50,8 +50,8 @@ void __new_context(struct mm_struct *mm)
> >  		isb();
> >  		flush_tlb_all();
> >  		if (icache_is_vivt_asid_tagged()) {
> > -			asm("mcr	p15, 0, %0, c7, c5, 0	@ invalidate I-cache\n"
> > -			    "mcr	p15, 0, %0, c7, c5, 6	@ flush BTAC/BTB\n"
> > +			__flush_icache_all();
> > +			asm("mcr	p15, 0, %0, c7, c5, 6	@ flush BTAC/BTB"
> 
> IIUC, we don't need to flush the BTAC after a full I-cache invalidation
> (not sure why I added it).

Okay, I'll fix the patch for that.

[RFC 02/11] ARM: Fix errata 411920 workarounds

[Catalin Marinas]

On Wed, 2009-10-28 at 17:32 +0000, Russell King - ARM Linux wrote:
> On Wed, Oct 28, 2009 at 03:31:35PM +0000, Catalin Marinas wrote:
> > On Sat, 2009-10-24 at 22:36 +0100, Russell King wrote:
> > > diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
> > > index 3147f75..af0624c 100644
> > > --- a/arch/arm/include/asm/cacheflush.h
> > > +++ b/arch/arm/include/asm/cacheflush.h
> > > @@ -412,9 +412,14 @@ extern void flush_dcache_page(struct page *);
> > >  
> > >  static inline void __flush_icache_all(void)
> > >  {
> > > +#ifdef CONFIG_ARM_ERRATA_411920
> > > +	extern void v6_icache_inval_all(void);
> > > +	v6_icache_inval_all();
> > > +#else
> > >         asm("mcr        p15, 0, %0, c7, c5, 0   @ invalidate I-cache\n"
> > >             :
> > >             : "r" (0));
> > > +#endif
> > >  }
> > 
> > For correctness, can we add a dsb() at the end of this function so that
> > the I-cache operation is guaranteed to complete?
> 
> Two reasons why not to in this patch:
> 
> 1. it changes the produced code, and therefore is another functional change.
> 2. it means we end up with a sequence like:
> 
> 	data cache maintainence
> 	dsb();
> 	instruction cache maintainence
> 	dsb();
> 
> The dsb() between the two operations probably isn't required.

That's described in B2.2.7 in the ARM ARM (make sure you use the latest
errata markup - 4.0).

The first DSB is needed to ensure the the D-cache op completed before
the I-cache op (in case we get speculative prefetches into the I-cache
before the D-cache op completed).

The second DSB would be needed to ensure the completion of the I-cache
maintenance operation. An ISB may be needed as well but the return from
exception has the same effect, so user code isn't affected.

-- 
Catalin

[RFC 02/11] ARM: Fix errata 411920 workarounds

[Russell King - ARM Linux]

On Thu, Oct 29, 2009 at 06:32:33PM +0000, Catalin Marinas wrote:
> On Wed, 2009-10-28 at 17:32 +0000, Russell King - ARM Linux wrote:
> > On Wed, Oct 28, 2009 at 03:31:35PM +0000, Catalin Marinas wrote:
> > > On Sat, 2009-10-24 at 22:36 +0100, Russell King wrote:
> > > > diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
> > > > index 3147f75..af0624c 100644
> > > > --- a/arch/arm/include/asm/cacheflush.h
> > > > +++ b/arch/arm/include/asm/cacheflush.h
> > > > @@ -412,9 +412,14 @@ extern void flush_dcache_page(struct page *);
> > > >  
> > > >  static inline void __flush_icache_all(void)
> > > >  {
> > > > +#ifdef CONFIG_ARM_ERRATA_411920
> > > > +	extern void v6_icache_inval_all(void);
> > > > +	v6_icache_inval_all();
> > > > +#else
> > > >         asm("mcr        p15, 0, %0, c7, c5, 0   @ invalidate I-cache\n"
> > > >             :
> > > >             : "r" (0));
> > > > +#endif
> > > >  }
> > > 
> > > For correctness, can we add a dsb() at the end of this function so that
> > > the I-cache operation is guaranteed to complete?
> > 
> > Two reasons why not to in this patch:
> > 
> > 1. it changes the produced code, and therefore is another functional change.
> > 2. it means we end up with a sequence like:
> > 
> > 	data cache maintainence
> > 	dsb();
> > 	instruction cache maintainence
> > 	dsb();
> > 
> > The dsb() between the two operations probably isn't required.
> 
> That's described in B2.2.7 in the ARM ARM (make sure you use the latest
> errata markup - 4.0).
> 
> The first DSB is needed to ensure the the D-cache op completed before
> the I-cache op (in case we get speculative prefetches into the I-cache
> before the D-cache op completed).
> 
> The second DSB would be needed to ensure the completion of the I-cache
> maintenance operation. An ISB may be needed as well but the return from
> exception has the same effect, so user code isn't affected.

Yes, it looks that way.

However, it looks like a DSB is not required for the work-around case -
the I-cache invalidation must complete before any branch prediction
occurs, and that's the purpose of the NOPs (to space other code well
outside of the visibility of any prefetching which might interfere with
the workaround.)

[RFC 02/11] ARM: Fix errata 411920 workarounds

[Catalin Marinas]

On Thu, 2009-10-29 at 19:11 +0000, Russell King - ARM Linux wrote:
> On Thu, Oct 29, 2009 at 06:32:33PM +0000, Catalin Marinas wrote:
> > On Wed, 2009-10-28 at 17:32 +0000, Russell King - ARM Linux wrote:
> > > On Wed, Oct 28, 2009 at 03:31:35PM +0000, Catalin Marinas wrote:
> > > > On Sat, 2009-10-24 at 22:36 +0100, Russell King wrote:
> > > > > diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
> > > > > index 3147f75..af0624c 100644
> > > > > --- a/arch/arm/include/asm/cacheflush.h
> > > > > +++ b/arch/arm/include/asm/cacheflush.h
> > > > > @@ -412,9 +412,14 @@ extern void flush_dcache_page(struct page *);
> > > > >  
> > > > >  static inline void __flush_icache_all(void)
> > > > >  {
> > > > > +#ifdef CONFIG_ARM_ERRATA_411920
> > > > > +	extern void v6_icache_inval_all(void);
> > > > > +	v6_icache_inval_all();
> > > > > +#else
> > > > >         asm("mcr        p15, 0, %0, c7, c5, 0   @ invalidate I-cache\n"
> > > > >             :
> > > > >             : "r" (0));
> > > > > +#endif
> > > > >  }
> > > > 
> > > > For correctness, can we add a dsb() at the end of this function so that
> > > > the I-cache operation is guaranteed to complete?
> > > 
> > > Two reasons why not to in this patch:
> > > 
> > > 1. it changes the produced code, and therefore is another functional change.
> > > 2. it means we end up with a sequence like:
> > > 
> > > 	data cache maintainence
> > > 	dsb();
> > > 	instruction cache maintainence
> > > 	dsb();
> > > 
> > > The dsb() between the two operations probably isn't required.
> > 
> > That's described in B2.2.7 in the ARM ARM (make sure you use the latest
> > errata markup - 4.0).
> > 
> > The first DSB is needed to ensure the the D-cache op completed before
> > the I-cache op (in case we get speculative prefetches into the I-cache
> > before the D-cache op completed).
> > 
> > The second DSB would be needed to ensure the completion of the I-cache
> > maintenance operation. An ISB may be needed as well but the return from
> > exception has the same effect, so user code isn't affected.
> 
> Yes, it looks that way.
> 
> However, it looks like a DSB is not required for the work-around case -
> the I-cache invalidation must complete before any branch prediction
> occurs, and that's the purpose of the NOPs (to space other code well
> outside of the visibility of any prefetching which might interfere with
> the workaround.)

On ARM1136 where this erratum applies there isn't much prefetching, so
it should be fine without the first DSB.

But I was more thinking about placing only the second DSB in the
flush_icache_all() function while the first one is in the corresponding
D-cache flushing code.

-- 
Catalin

[RFC 03/11] ARM: move __flush_icache_all() out of flush_pfn_alias()

[Russell King]

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mm/flush.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index 7f294f3..4e2a4e7 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -35,7 +35,6 @@ static void flush_pfn_alias(unsigned long pfn, unsigned long vaddr)
 	    :
 	    : "r" (to), "r" (to + PAGE_SIZE - L1_CACHE_BYTES), "r" (zero)
 	    : "cc");
-	__flush_icache_all();
 }
 
 void flush_cache_mm(struct mm_struct *mm)
@@ -85,8 +84,10 @@ void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig
 		return;
 	}
 
-	if (cache_is_vipt_aliasing())
+	if (cache_is_vipt_aliasing()) {
 		flush_pfn_alias(pfn, user_addr);
+		__flush_icache_all();
+	}
 }
 
 void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
@@ -103,6 +104,7 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 
 	if (cache_is_vipt_aliasing()) {
 		flush_pfn_alias(page_to_pfn(page), uaddr);
+		__flush_icache_all();
 		return;
 	}
 
@@ -139,9 +141,11 @@ void __flush_dcache_page(struct address_space *mapping, struct page *page)
 	 * we only need to do one flush - which would be at the relevant
 	 * userspace colour, which is congruent with page->index.
 	 */
-	if (mapping && cache_is_vipt_aliasing())
+	if (mapping && cache_is_vipt_aliasing()) {
 		flush_pfn_alias(page_to_pfn(page),
 				page->index << PAGE_CACHE_SHIFT);
+		__flush_icache_all();
+	}
 }
 
 static void __flush_dcache_aliases(struct address_space *mapping, struct page *page)
@@ -242,6 +246,7 @@ void __flush_anon_page(struct vm_area_struct *vma, struct page *page, unsigned l
 		 * userspace address only.
 		 */
 		flush_pfn_alias(pfn, vmaddr);
+		__flush_icache_all();
 	}
 
 	/*
-- 
1.6.2.5

[RFC 04/11] ARM: Remove __flush_icache_all() from __flush_dcache_page()

[Russell King]

Both call sites for __flush_dcache_page() end up calling
__flush_icache_all() themselves, so having __flush_dcache_page() do
this as well is wasteful.  Remove the duplicated icache flushing.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mm/flush.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index 4e2a4e7..19c53dd 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -141,11 +141,9 @@ void __flush_dcache_page(struct address_space *mapping, struct page *page)
 	 * we only need to do one flush - which would be at the relevant
 	 * userspace colour, which is congruent with page->index.
 	 */
-	if (mapping && cache_is_vipt_aliasing()) {
+	if (mapping && cache_is_vipt_aliasing())
 		flush_pfn_alias(page_to_pfn(page),
 				page->index << PAGE_CACHE_SHIFT);
-		__flush_icache_all();
-	}
 }
 
 static void __flush_dcache_aliases(struct address_space *mapping, struct page *page)
-- 
1.6.2.5

[RFC 05/11] ARM: Avoid flush_dcache_page() for zero page

[Russell King]

The zero page is read-only, and has its cache state cleared during
boot.  No further maintanence for this page is required.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mm/fault-armv.c |    7 +++++++
 arch/arm/mm/flush.c      |   11 ++++++++++-
 arch/arm/mm/mmu.c        |    2 +-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/fault-armv.c b/arch/arm/mm/fault-armv.c
index 830cf17..7296022 100644
--- a/arch/arm/mm/fault-armv.c
+++ b/arch/arm/mm/fault-armv.c
@@ -153,7 +153,14 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long addr, pte_t pte)
 	if (!pfn_valid(pfn))
 		return;
 
+	/*
+	 * The zero page is never written to, so never has any dirty
+	 * cache lines, and therefore never needs to be flushed.
+	 */
 	page = pfn_to_page(pfn);
+	if (page == ZERO_PAGE(0))
+		return;
+
 	mapping = page_mapping(page);
 #ifndef CONFIG_SMP
 	if (test_and_clear_bit(PG_dcache_dirty, &page->flags))
diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index 19c53dd..82dd2c5 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -198,7 +198,16 @@ static void __flush_dcache_aliases(struct address_space *mapping, struct page *p
  */
 void flush_dcache_page(struct page *page)
 {
-	struct address_space *mapping = page_mapping(page);
+	struct address_space *mapping;
+
+	/*
+	 * The zero page is never written to, so never has any dirty
+	 * cache lines, and therefore never needs to be flushed.
+	 */
+	if (page == ZERO_PAGE(0))
+		return;
+
+	mapping = page_mapping(page);
 
 #ifndef CONFIG_SMP
 	if (!PageHighMem(page) && mapping && !mapping_mapped(mapping))
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 02243ee..2a7cc61 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1029,7 +1029,7 @@ void __init paging_init(struct machine_desc *mdesc)
 	 */
 	zero_page = alloc_bootmem_low_pages(PAGE_SIZE);
 	empty_zero_page = virt_to_page(zero_page);
-	flush_dcache_page(empty_zero_page);
+	__flush_dcache_page(NULL, empty_zero_page);
 }
 
 /*
-- 
1.6.2.5

[RFC 06/11] ARM: Avoid duplicated implementation for VIVT cache flushing

[Russell King]

We had two copies of the wrapper code for VIVT cache flushing - one in
asm/cacheflush.h and one in arch/arm/mm/flush.c.  Reduce this down to
one common copy.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/include/asm/cacheflush.h |   20 +++++++++++++++-----
 arch/arm/mm/flush.c               |   17 ++++-------------
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
index af0624c..9fd6d3a 100644
--- a/arch/arm/include/asm/cacheflush.h
+++ b/arch/arm/include/asm/cacheflush.h
@@ -331,15 +331,15 @@ static inline void outer_flush_range(unsigned long start, unsigned long end)
  * Convert calls to our calling convention.
  */
 #define flush_cache_all()		__cpuc_flush_kern_all()
-#ifndef CONFIG_CPU_CACHE_VIPT
-static inline void flush_cache_mm(struct mm_struct *mm)
+
+static inline void vivt_flush_cache_mm(struct mm_struct *mm)
 {
 	if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(mm)))
 		__cpuc_flush_user_all();
 }
 
 static inline void
-flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned long end)
+vivt_flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned long end)
 {
 	if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(vma->vm_mm)))
 		__cpuc_flush_user_range(start & PAGE_MASK, PAGE_ALIGN(end),
@@ -347,7 +347,7 @@ flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned long
 }
 
 static inline void
-flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsigned long pfn)
+vivt_flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsigned long pfn)
 {
 	if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(vma->vm_mm))) {
 		unsigned long addr = user_addr & PAGE_MASK;
@@ -356,7 +356,7 @@ flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsigned l
 }
 
 static inline void
-flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
+vivt_flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 			 unsigned long uaddr, void *kaddr,
 			 unsigned long len, int write)
 {
@@ -365,6 +365,16 @@ flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 		__cpuc_coherent_kern_range(addr, addr + len);
 	}
 }
+
+#ifndef CONFIG_CPU_CACHE_VIPT
+#define flush_cache_mm(mm) \
+		vivt_flush_cache_mm(mm)
+#define flush_cache_range(vma,start,end) \
+		vivt_flush_cache_range(vma,start,end)
+#define flush_cache_page(vma,addr,pfn) \
+		vivt_flush_cache_page(vma,addr,pfn)
+#define flush_ptrace_access(vma,page,ua,ka,len,write) \
+		vivt_flush_ptrace_access(vma,page,ua,ka,len,write)
 #else
 extern void flush_cache_mm(struct mm_struct *mm);
 extern void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned long end);
diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index 82dd2c5..4fa3d4d 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -40,8 +40,7 @@ static void flush_pfn_alias(unsigned long pfn, unsigned long vaddr)
 void flush_cache_mm(struct mm_struct *mm)
 {
 	if (cache_is_vivt()) {
-		if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(mm)))
-			__cpuc_flush_user_all();
+		vivt_flush_cache_mm(mm);
 		return;
 	}
 
@@ -58,9 +57,7 @@ void flush_cache_mm(struct mm_struct *mm)
 void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned long end)
 {
 	if (cache_is_vivt()) {
-		if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(vma->vm_mm)))
-			__cpuc_flush_user_range(start & PAGE_MASK, PAGE_ALIGN(end),
-						vma->vm_flags);
+		vivt_flush_cache_range(vma, start, end);
 		return;
 	}
 
@@ -77,10 +74,7 @@ void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned
 void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsigned long pfn)
 {
 	if (cache_is_vivt()) {
-		if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(vma->vm_mm))) {
-			unsigned long addr = user_addr & PAGE_MASK;
-			__cpuc_flush_user_range(addr, addr + PAGE_SIZE, vma->vm_flags);
-		}
+		vivt_flush_cache_page(vma, user_addr, pfn);
 		return;
 	}
 
@@ -95,10 +89,7 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 			 unsigned long len, int write)
 {
 	if (cache_is_vivt()) {
-		if (cpumask_test_cpu(smp_processor_id(), mm_cpumask(vma->vm_mm))) {
-			unsigned long addr = (unsigned long)kaddr;
-			__cpuc_coherent_kern_range(addr, addr + len);
-		}
+		vivt_flush_ptrace_access(vma, page, uaddr, kaddr, len, write);
 		return;
 	}
 
-- 
1.6.2.5

[RFC 07/11] ARM: Allocate ZERO_PAGE from highmem

[Russell King]

The zero page does not need to be in low memory, so allocate it from
high memory.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mm/init.c |    9 +++++++++
 arch/arm/mm/mmu.c  |   10 ----------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 40940d7..101e100 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -609,6 +609,15 @@ void __init mem_init(void)
 		 */
 		sysctl_overcommit_memory = OVERCOMMIT_ALWAYS;
 	}
+
+	/*
+	 * Allocate the zero page, trying to get it from highmem if
+	 * available.  Note that this must always succeed and returns
+	 * a zeroed page.
+	 */
+	empty_zero_page = alloc_pages(GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO, 0);
+	BUG_ON(!empty_zero_page);
+	__flush_dcache_page(NULL, empty_zero_page);
 }
 
 void free_initmem(void)
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 2a7cc61..2bff0c2 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1012,8 +1012,6 @@ static void __init kmap_init(void)
  */
 void __init paging_init(struct machine_desc *mdesc)
 {
-	void *zero_page;
-
 	build_mem_type_table();
 	sanity_check_meminfo();
 	prepare_page_table();
@@ -1022,14 +1020,6 @@ void __init paging_init(struct machine_desc *mdesc)
 	kmap_init();
 
 	top_pmd = pmd_off_k(0xffff0000);
-
-	/*
-	 * allocate the zero page.  Note that this always succeeds and
-	 * returns a zeroed result.
-	 */
-	zero_page = alloc_bootmem_low_pages(PAGE_SIZE);
-	empty_zero_page = virt_to_page(zero_page);
-	__flush_dcache_page(NULL, empty_zero_page);
 }
 
 /*
-- 
1.6.2.5

[RFC 07/11] ARM: Allocate ZERO_PAGE from highmem

[Russell King - ARM Linux]

On Sun, Oct 25, 2009 at 10:59:08AM +0000, Russell King wrote:
> The zero page does not need to be in low memory, so allocate it from
> high memory.

Unfortunately, this patch is about to be thrown out - mm/shmem.c does
this:

static inline void shmem_swp_balance_unmap(void)
{
	/*
	 * When passing a pointer to an i_direct entry, to code which
	 * also handles indirect entries and so will shmem_swp_unmap,
	 * we must arrange for the preempt count to remain in balance.
	 * What kmap_atomic of a lowmem page does depends on config
	 * and architecture, so pretend to kmap_atomic some lowmem page.
	 */
	(void) kmap_atomic(ZERO_PAGE(0), KM_USER1);
}

which causes:

               callsite vaddr      pteptr    *pteptr    page_to_phys(page)
kmap_atomic:   c007d108 0xfff04000 c046ac10, 0x00000000 0x7800000
kunmap_atomic: c007d88c 0xc1c2a000
kmap_atomic:   c007d108 0xfff04000 c046ac10, 0x078002cf 0x7800000
*BUG*

[RFC 08/11] ARM: Avoid evaluating page_address() multiple times

[Russell King]

page_address() is a function call rather than a macro, and so:

	if (page_address(page))
		do_something(page_address(page));

results in two calls to this function.  This is unnecessary; remove
the duplication.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mm/flush.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index 4fa3d4d..dc66f86 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -113,6 +113,8 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 
 void __flush_dcache_page(struct address_space *mapping, struct page *page)
 {
+	void *addr = page_address(page);
+
 	/*
 	 * Writeback any data associated with the kernel mapping of this
 	 * page.  This ensures that data in the physical page is mutually
@@ -123,9 +125,9 @@ void __flush_dcache_page(struct address_space *mapping, struct page *page)
 	 * kmap_atomic() doesn't set the page virtual address, and
 	 * kunmap_atomic() takes care of cache flushing already.
 	 */
-	if (page_address(page))
+	if (addr)
 #endif
-		__cpuc_flush_dcache_page(page_address(page));
+		__cpuc_flush_dcache_page(addr);
 
 	/*
 	 * If this is a page cache page, and we have an aliasing VIPT cache,
-- 
1.6.2.5

[RFC 09/11] ARM: Add I-cache invalidation for VIVT ASID tagged caches

[Russell King]

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mm/flush.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index dc66f86..9770e27 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -69,6 +69,9 @@ void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned
 		    : "cc");
 		__flush_icache_all();
 	}
+
+	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
+		__flush_icache_all();
 }
 
 void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsigned long pfn)
@@ -82,6 +85,9 @@ void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig
 		flush_pfn_alias(pfn, user_addr);
 		__flush_icache_all();
 	}
+
+	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
+		__flush_icache_all();
 }
 
 void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
-- 
1.6.2.5

[RFC 09/11] ARM: Add I-cache invalidation for VIVT ASID tagged caches

[Catalin Marinas]

On Sun, 2009-10-25 at 13:35 +0000, Russell King wrote:
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  arch/arm/mm/flush.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
> index dc66f86..9770e27 100644
> --- a/arch/arm/mm/flush.c
> +++ b/arch/arm/mm/flush.c
> @@ -69,6 +69,9 @@ void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned
>  		    : "cc");
>  		__flush_icache_all();
>  	}
> +
> +	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
> +		__flush_icache_all();
>  }
>  
>  void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsigned long pfn)
> @@ -82,6 +85,9 @@ void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig
>  		flush_pfn_alias(pfn, user_addr);
>  		__flush_icache_all();
>  	}
> +
> +	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
> +		__flush_icache_all();
>  }
>  
>  void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,

You may be right but why is this needed? Between mm's, we have the ASID.
Within the same mm, do we expect to change the corresponding physical
address of a virtual address without calling update_mmu_cache?

-- 
Catalin

[RFC 09/11] ARM: Add I-cache invalidation for VIVT ASID tagged caches

[Russell King - ARM Linux]

On Wed, Oct 28, 2009 at 03:49:01PM +0000, Catalin Marinas wrote:
> On Sun, 2009-10-25 at 13:35 +0000, Russell King wrote:
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > ---
> >  arch/arm/mm/flush.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
> > index dc66f86..9770e27 100644
> > --- a/arch/arm/mm/flush.c
> > +++ b/arch/arm/mm/flush.c
> > @@ -69,6 +69,9 @@ void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned
> >  		    : "cc");
> >  		__flush_icache_all();
> >  	}
> > +
> > +	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
> > +		__flush_icache_all();
> >  }
> >  
> >  void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsigned long pfn)
> > @@ -82,6 +85,9 @@ void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig
> >  		flush_pfn_alias(pfn, user_addr);
> >  		__flush_icache_all();
> >  	}
> > +
> > +	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
> > +		__flush_icache_all();
> >  }
> >  
> >  void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
> 
> You may be right but why is this needed? Between mm's, we have the ASID.
> Within the same mm, do we expect to change the corresponding physical
> address of a virtual address without calling update_mmu_cache?

Stop thinking that update_mmu_cache() has something to do with cache
handling.  That is not its purpose.  update_mmu_cache() is there for
systems with software-TLBs to efficiently update the software-TLB for
the page fault which has happened.

flush_cache_page() on the other hand is called whenever we change a
page mapping - but it's not called when mm's change.

The ARM ARM specifies that when we change the page tables, and we
have a VIVT I-cache, it needs to be flushed.  The above is merely
complying with that statement.

[RFC 09/11] ARM: Add I-cache invalidation for VIVT ASID tagged caches

[Catalin Marinas]

On Wed, 2009-10-28 at 17:35 +0000, Russell King - ARM Linux wrote:
> On Wed, Oct 28, 2009 at 03:49:01PM +0000, Catalin Marinas wrote:
> > On Sun, 2009-10-25 at 13:35 +0000, Russell King wrote:
> > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > > ---
> > >  arch/arm/mm/flush.c |    6 ++++++
> > >  1 files changed, 6 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
> > > index dc66f86..9770e27 100644
> > > --- a/arch/arm/mm/flush.c
> > > +++ b/arch/arm/mm/flush.c
> > > @@ -69,6 +69,9 @@ void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned
> > >  		    : "cc");
> > >  		__flush_icache_all();
> > >  	}
> > > +
> > > +	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
> > > +		__flush_icache_all();
> > >  }
> > >  
> > >  void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsigned long pfn)
> > > @@ -82,6 +85,9 @@ void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig
> > >  		flush_pfn_alias(pfn, user_addr);
> > >  		__flush_icache_all();
> > >  	}
> > > +
> > > +	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
> > > +		__flush_icache_all();
> > >  }
> > >  
> > >  void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
> > 
> > You may be right but why is this needed? Between mm's, we have the ASID.
> > Within the same mm, do we expect to change the corresponding physical
> > address of a virtual address without calling update_mmu_cache?
> 
> Stop thinking that update_mmu_cache() has something to do with cache
> handling.  That is not its purpose.  update_mmu_cache() is there for
> systems with software-TLBs to efficiently update the software-TLB for
> the page fault which has happened.
> 
> flush_cache_page() on the other hand is called whenever we change a
> page mapping - but it's not called when mm's change.
> 
> The ARM ARM specifies that when we change the page tables, and we
> have a VIVT I-cache, it needs to be flushed.  The above is merely
> complying with that statement.

In this case I'm OK with the patch (I think it's pretty much the same
behaviour with pre-ARMv6 VIVT caches).

-- 
Catalin

[RFC 10/11] ARM: arrange for flush_cache_range() to always flush the I-cache

[Russell King]

Dirk Behme reported instability on ARM11 SMP (VIPT non-aliasing cache)
caused by the dynamic linker changing protection on text pages to write
GOT entries.  The problem is due to an interaction between the write
faulting code providing new anonymous pages (which haven't had their
I-cache cleaned due to no VM_EXEC) and change_protection() not doing
any kind of I-cache maintainence.

Work around this by flushing the I-cache in flush_cache_range() for
VM_EXEC VMAs (which is what we have when the region is being made
executable again.)  This ensures that the I-cache will be up to date
with the new COW'd pages.

Note: if users are writing instructions, then they still need to use
the ARM sys_cacheflush API to ensure that the caches are correctly
synchronized.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mm/flush.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index 9770e27..ee7ce68 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -67,10 +67,9 @@ void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned
 		    :
 		    : "r" (0)
 		    : "cc");
-		__flush_icache_all();
 	}
 
-	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
+	if (vma->vm_flags & VM_EXEC)
 		__flush_icache_all();
 }
 
-- 
1.6.2.5

[RFC 10/11] ARM: arrange for flush_cache_range() to always flush the I-cache

[Catalin Marinas]

On Sun, 2009-10-25 at 14:12 +0000, Russell King wrote:
> Dirk Behme reported instability on ARM11 SMP (VIPT non-aliasing cache)
> caused by the dynamic linker changing protection on text pages to write
> GOT entries.  The problem is due to an interaction between the write
> faulting code providing new anonymous pages (which haven't had their
> I-cache cleaned due to no VM_EXEC) and change_protection() not doing
> any kind of I-cache maintainence.
> 
> Work around this by flushing the I-cache in flush_cache_range() for
> VM_EXEC VMAs (which is what we have when the region is being made
> executable again.)  This ensures that the I-cache will be up to date
> with the new COW'd pages.

Are you sure this fixes the problem? IIRC this is mainly caused by the
write-allocate D-cache and that copy_to_user_highpage() doesn't flush
the D-cache. Just an I-cache invalidation wouldn't help much.

-- 
Catalin

[RFC 10/11] ARM: arrange for flush_cache_range() to always flush the I-cache

[Russell King - ARM Linux]

On Wed, Oct 28, 2009 at 03:51:38PM +0000, Catalin Marinas wrote:
> On Sun, 2009-10-25 at 14:12 +0000, Russell King wrote:
> > Dirk Behme reported instability on ARM11 SMP (VIPT non-aliasing cache)
> > caused by the dynamic linker changing protection on text pages to write
> > GOT entries.  The problem is due to an interaction between the write
> > faulting code providing new anonymous pages (which haven't had their
> > I-cache cleaned due to no VM_EXEC) and change_protection() not doing
> > any kind of I-cache maintainence.
> > 
> > Work around this by flushing the I-cache in flush_cache_range() for
> > VM_EXEC VMAs (which is what we have when the region is being made
> > executable again.)  This ensures that the I-cache will be up to date
> > with the new COW'd pages.
> 
> Are you sure this fixes the problem? IIRC this is mainly caused by the
> write-allocate D-cache and that copy_to_user_highpage() doesn't flush
> the D-cache. Just an I-cache invalidation wouldn't help much.

Not on its own (sorry, should've made that clear).  I'm not entirely
sure what's happening with the other half of the fix (the thread seemed
to die before reaching any kind of conclusion) but this is the right
solution for the I-cache side of the problem.

[RFC 10/11] ARM: arrange for flush_cache_range() to always flush the I-cache

[Catalin Marinas]

On Wed, 2009-10-28 at 17:37 +0000, Russell King - ARM Linux wrote:
> On Wed, Oct 28, 2009 at 03:51:38PM +0000, Catalin Marinas wrote:
> > On Sun, 2009-10-25 at 14:12 +0000, Russell King wrote:
> > > Dirk Behme reported instability on ARM11 SMP (VIPT non-aliasing cache)
> > > caused by the dynamic linker changing protection on text pages to write
> > > GOT entries.  The problem is due to an interaction between the write
> > > faulting code providing new anonymous pages (which haven't had their
> > > I-cache cleaned due to no VM_EXEC) and change_protection() not doing
> > > any kind of I-cache maintainence.
> > > 
> > > Work around this by flushing the I-cache in flush_cache_range() for
> > > VM_EXEC VMAs (which is what we have when the region is being made
> > > executable again.)  This ensures that the I-cache will be up to date
> > > with the new COW'd pages.
> > 
> > Are you sure this fixes the problem? IIRC this is mainly caused by the
> > write-allocate D-cache and that copy_to_user_highpage() doesn't flush
> > the D-cache. Just an I-cache invalidation wouldn't help much.
> 
> Not on its own (sorry, should've made that clear). 

I actually got the opposite from the patch description - "I-cache will
be up to date with the new COW'd pages".

>  I'm not entirely
> sure what's happening with the other half of the fix (the thread seemed
> to die before reaching any kind of conclusion) but this is the right
> solution for the I-cache side of the problem.

I listed a few solutions there but not many replies. Let's try to sort
it out in this thread (maybe people got bored and ignored the other
thread completely :-)).

As I see it, we either fix the mprotect(RX) to do full coherency or
ensure that the COW mechanism flushes the (I-cache? and) D-cache (or a
combination of these two).

Assuming that we won't get COW for mprotect(RWX) pages (apps seem to do
mprotect(RW) follow by mprotect(RX)), your patch handles the I-cache
side.

On ARM11MPCore, cache operations aren't broadcast and there is a small
risk of thread migration between COW and mprotect(RX), so flushing the
D-cache in mprotect(RX) may not always work (Cortex-A9 is different). In
this case, the only place to do the D-cache flushing is in
copy_user_highpage().

The patch below should do it (you suggested it but never posted a
patch):


Flush the D-cache during copy_user_highpage()

From: Catalin Marinas <catalin.marinas@arm.com>

The I and D caches for copy-on-write pages on processors with
write-allocate caches become incoherent causing problems on application
relying on CoW for text pages (dynamic linker relocating symbols in a
text page). This patch flushes the D-cache for such pages.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/mm/copypage-v6.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c
index 4127a7b..f19ed4e 100644
--- a/arch/arm/mm/copypage-v6.c
+++ b/arch/arm/mm/copypage-v6.c
@@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
 	kfrom = kmap_atomic(from, KM_USER0);
 	kto = kmap_atomic(to, KM_USER1);
 	copy_page(kto, kfrom);
+	__cpuc_flush_dcache_page(kto);
 	kunmap_atomic(kto, KM_USER1);
 	kunmap_atomic(kfrom, KM_USER0);
 }


-- 
Catalin

[RFC 10/11] ARM: arrange for flush_cache_range() to always flush the I-cache

[Catalin Marinas]

Russell,

On Thu, 2009-10-29 at 17:51 +0000, Catalin Marinas wrote:
> On Wed, 2009-10-28 at 17:37 +0000, Russell King - ARM Linux wrote:
> >  I'm not entirely
> > sure what's happening with the other half of the fix (the thread seemed
> > to die before reaching any kind of conclusion) but this is the right
> > solution for the I-cache side of the problem.
> 
> I listed a few solutions there but not many replies. Let's try to sort
> it out in this thread (maybe people got bored and ignored the other
> thread completely :-)).
[...]
> Flush the D-cache during copy_user_highpage()
> 
> From: Catalin Marinas <catalin.marinas@arm.com>
> 
> The I and D caches for copy-on-write pages on processors with
> write-allocate caches become incoherent causing problems on application
> relying on CoW for text pages (dynamic linker relocating symbols in a
> text page). This patch flushes the D-cache for such pages.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  arch/arm/mm/copypage-v6.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c
> index 4127a7b..f19ed4e 100644
> --- a/arch/arm/mm/copypage-v6.c
> +++ b/arch/arm/mm/copypage-v6.c
> @@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
>  	kfrom = kmap_atomic(from, KM_USER0);
>  	kto = kmap_atomic(to, KM_USER1);
>  	copy_page(kto, kfrom);
> +	__cpuc_flush_dcache_page(kto);
>  	kunmap_atomic(kto, KM_USER1);
>  	kunmap_atomic(kfrom, KM_USER0);
>  }

Any thoughts one getting this merged (maybe in 2.6.33)? It seems that
no-one else has any views on this but the COW text pages problem is
still present.

Thanks.

-- 
Catalin

[RFC 10/11] ARM: arrange for flush_cache_range() to always flush the I-cache

[Russell King - ARM Linux]

On Mon, Nov 16, 2009 at 10:46:55AM +0000, Catalin Marinas wrote:
> > diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c
> > index 4127a7b..f19ed4e 100644
> > --- a/arch/arm/mm/copypage-v6.c
> > +++ b/arch/arm/mm/copypage-v6.c
> > @@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
> >  	kfrom = kmap_atomic(from, KM_USER0);
> >  	kto = kmap_atomic(to, KM_USER1);
> >  	copy_page(kto, kfrom);
> > +	__cpuc_flush_dcache_page(kto);
> >  	kunmap_atomic(kto, KM_USER1);
> >  	kunmap_atomic(kfrom, KM_USER0);
> >  }
> 
> Any thoughts one getting this merged (maybe in 2.6.33)? It seems that
> no-one else has any views on this but the COW text pages problem is
> still present.

Well, it would be useful to get some comment from Nicolas since
kunmap_atomic() will call this function for highmem pages.  We might want
to avoid doing the dcache flush thing multiple times for these, but it
looks to me like testing PageHighMem(to) leaves us a hole.

Maybe we need something like kunmap_atomic_flush() ?

[RFC 10/11] ARM: arrange for flush_cache_range() to always flush the I-cache

[Nicolas Pitre]

On Mon, 16 Nov 2009, Russell King - ARM Linux wrote:

> On Mon, Nov 16, 2009 at 10:46:55AM +0000, Catalin Marinas wrote:
> > > diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c
> > > index 4127a7b..f19ed4e 100644
> > > --- a/arch/arm/mm/copypage-v6.c
> > > +++ b/arch/arm/mm/copypage-v6.c
> > > @@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
> > >  	kfrom = kmap_atomic(from, KM_USER0);
> > >  	kto = kmap_atomic(to, KM_USER1);
> > >  	copy_page(kto, kfrom);
> > > +	__cpuc_flush_dcache_page(kto);
> > >  	kunmap_atomic(kto, KM_USER1);
> > >  	kunmap_atomic(kfrom, KM_USER0);
> > >  }
> > 
> > Any thoughts one getting this merged (maybe in 2.6.33)? It seems that
> > no-one else has any views on this but the COW text pages problem is
> > still present.
> 
> Well, it would be useful to get some comment from Nicolas since
> kunmap_atomic() will call this function for highmem pages.  We might want
> to avoid doing the dcache flush thing multiple times for these, but it
> looks to me like testing PageHighMem(to) leaves us a hole.

Better test page_address(to) and skip the flush if it is NULL.  That's 
what we do elsewhere already.  If page_addressto) is NULL then there is 
no long lasting mapping for that page and the cache will be flushed upon 
kunmap_atomic().  Hence:

#ifdef CONFIG_HIGHMEM
	/*
	 * kmap_atomic() doesn't set the page virtual address, and
	 * kunmap_atomic() takes care of cache flushing already.
	 */
	if (page_address(to) != NULL)
#endif
		__cpuc_flush_dcache_page(kto);


Nicolas

[RFC 10/11] ARM: arrange for flush_cache_range() to always flush the I-cache

[Russell King - ARM Linux]

On Sun, Nov 22, 2009 at 03:28:30PM -0500, Nicolas Pitre wrote:
> On Mon, 16 Nov 2009, Russell King - ARM Linux wrote:
> 
> > On Mon, Nov 16, 2009 at 10:46:55AM +0000, Catalin Marinas wrote:
> > > > diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c
> > > > index 4127a7b..f19ed4e 100644
> > > > --- a/arch/arm/mm/copypage-v6.c
> > > > +++ b/arch/arm/mm/copypage-v6.c
> > > > @@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
> > > >  	kfrom = kmap_atomic(from, KM_USER0);
> > > >  	kto = kmap_atomic(to, KM_USER1);
> > > >  	copy_page(kto, kfrom);
> > > > +	__cpuc_flush_dcache_page(kto);
> > > >  	kunmap_atomic(kto, KM_USER1);
> > > >  	kunmap_atomic(kfrom, KM_USER0);
> > > >  }
> > > 
> > > Any thoughts one getting this merged (maybe in 2.6.33)? It seems that
> > > no-one else has any views on this but the COW text pages problem is
> > > still present.
> > 
> > Well, it would be useful to get some comment from Nicolas since
> > kunmap_atomic() will call this function for highmem pages.  We might want
> > to avoid doing the dcache flush thing multiple times for these, but it
> > looks to me like testing PageHighMem(to) leaves us a hole.
> 
> Better test page_address(to) and skip the flush if it is NULL.  That's 
> what we do elsewhere already.  If page_addressto) is NULL then there is 
> no long lasting mapping for that page and the cache will be flushed upon 
> kunmap_atomic().  Hence:
> 
> #ifdef CONFIG_HIGHMEM
> 	/*
> 	 * kmap_atomic() doesn't set the page virtual address, and
> 	 * kunmap_atomic() takes care of cache flushing already.
> 	 */
> 	if (page_address(to) != NULL)
> #endif
> 		__cpuc_flush_dcache_page(kto);

Okay, can we get this combined with Catalin's patch and applied to all
applicable processors (which I assume would be just v6 and v7) ?

Catalin?

[RFC 10/11] ARM: arrange for flush_cache_range() to alwaysflush the I-cache

[Catalin Marinas]

On Mon, 2009-11-23 at 20:28 +0000, Russell King - ARM Linux wrote:
> On Sun, Nov 22, 2009 at 03:28:30PM -0500, Nicolas Pitre wrote:
> > On Mon, 16 Nov 2009, Russell King - ARM Linux wrote:
> >
> > > On Mon, Nov 16, 2009 at 10:46:55AM +0000, Catalin Marinas wrote:
> > > > > diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c
> > > > > index 4127a7b..f19ed4e 100644
> > > > > --- a/arch/arm/mm/copypage-v6.c
> > > > > +++ b/arch/arm/mm/copypage-v6.c
> > > > > @@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
> > > > >         kfrom = kmap_atomic(from, KM_USER0);
> > > > >         kto = kmap_atomic(to, KM_USER1);
> > > > >         copy_page(kto, kfrom);
> > > > > +       __cpuc_flush_dcache_page(kto);
> > > > >         kunmap_atomic(kto, KM_USER1);
> > > > >         kunmap_atomic(kfrom, KM_USER0);
> > > > >  }
> > > >
> > > > Any thoughts one getting this merged (maybe in 2.6.33)? It seems that
> > > > no-one else has any views on this but the COW text pages problem is
> > > > still present.
> > >
> > > Well, it would be useful to get some comment from Nicolas since
> > > kunmap_atomic() will call this function for highmem pages.  We might want
> > > to avoid doing the dcache flush thing multiple times for these, but it
> > > looks to me like testing PageHighMem(to) leaves us a hole.
> >
> > Better test page_address(to) and skip the flush if it is NULL.  That's
> > what we do elsewhere already.  If page_addressto) is NULL then there is
> > no long lasting mapping for that page and the cache will be flushed upon
> > kunmap_atomic().  Hence:
> >
> > #ifdef CONFIG_HIGHMEM
> >       /*
> >        * kmap_atomic() doesn't set the page virtual address, and
> >        * kunmap_atomic() takes care of cache flushing already.
> >        */
> >       if (page_address(to) != NULL)
> > #endif
> >               __cpuc_flush_dcache_page(kto);
> 
> Okay, can we get this combined with Catalin's patch and applied to all
> applicable processors (which I assume would be just v6 and v7) ?

I'm ok with this.

Thanks.

-- 
Catalin

[RFC 10/11] ARM: arrange for flush_cache_range() to alwaysflush the I-cache

[Russell King - ARM Linux]

On Tue, Nov 24, 2009 at 01:05:56PM +0000, Catalin Marinas wrote:
> On Mon, 2009-11-23 at 20:28 +0000, Russell King - ARM Linux wrote:
> > On Sun, Nov 22, 2009 at 03:28:30PM -0500, Nicolas Pitre wrote:
> > > On Mon, 16 Nov 2009, Russell King - ARM Linux wrote:
> > >
> > > > On Mon, Nov 16, 2009 at 10:46:55AM +0000, Catalin Marinas wrote:
> > > > > > diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c
> > > > > > index 4127a7b..f19ed4e 100644
> > > > > > --- a/arch/arm/mm/copypage-v6.c
> > > > > > +++ b/arch/arm/mm/copypage-v6.c
> > > > > > @@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
> > > > > >         kfrom = kmap_atomic(from, KM_USER0);
> > > > > >         kto = kmap_atomic(to, KM_USER1);
> > > > > >         copy_page(kto, kfrom);
> > > > > > +       __cpuc_flush_dcache_page(kto);
> > > > > >         kunmap_atomic(kto, KM_USER1);
> > > > > >         kunmap_atomic(kfrom, KM_USER0);
> > > > > >  }
> > > > >
> > > > > Any thoughts one getting this merged (maybe in 2.6.33)? It seems that
> > > > > no-one else has any views on this but the COW text pages problem is
> > > > > still present.
> > > >
> > > > Well, it would be useful to get some comment from Nicolas since
> > > > kunmap_atomic() will call this function for highmem pages.  We might want
> > > > to avoid doing the dcache flush thing multiple times for these, but it
> > > > looks to me like testing PageHighMem(to) leaves us a hole.
> > >
> > > Better test page_address(to) and skip the flush if it is NULL.  That's
> > > what we do elsewhere already.  If page_addressto) is NULL then there is
> > > no long lasting mapping for that page and the cache will be flushed upon
> > > kunmap_atomic().  Hence:
> > >
> > > #ifdef CONFIG_HIGHMEM
> > >       /*
> > >        * kmap_atomic() doesn't set the page virtual address, and
> > >        * kunmap_atomic() takes care of cache flushing already.
> > >        */
> > >       if (page_address(to) != NULL)
> > > #endif
> > >               __cpuc_flush_dcache_page(kto);
> > 
> > Okay, can we get this combined with Catalin's patch and applied to all
> > applicable processors (which I assume would be just v6 and v7) ?
> 
> I'm ok with this.

Sorry, I wasn't clear enough.  I was expecting you to update your patch,
and once the other copy_user_page stuff finally gets the "yes it solves
the problem" response from Steven, it can be committed and merged into
this branch.

[RFC 10/11] ARM: arrange for flush_cache_range() toalwaysflush the I-cache

[Catalin Marinas]

On Tue, 2009-11-24 at 14:46 +0000, Russell King - ARM Linux wrote:
> On Tue, Nov 24, 2009 at 01:05:56PM +0000, Catalin Marinas wrote:
> > On Mon, 2009-11-23 at 20:28 +0000, Russell King - ARM Linux wrote:
> > > On Sun, Nov 22, 2009 at 03:28:30PM -0500, Nicolas Pitre wrote:
> > > > On Mon, 16 Nov 2009, Russell King - ARM Linux wrote:
> > > >
> > > > > On Mon, Nov 16, 2009 at 10:46:55AM +0000, Catalin Marinas wrote:
> > > > > > > diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c
> > > > > > > index 4127a7b..f19ed4e 100644
> > > > > > > --- a/arch/arm/mm/copypage-v6.c
> > > > > > > +++ b/arch/arm/mm/copypage-v6.c
> > > > > > > @@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
> > > > > > >         kfrom = kmap_atomic(from, KM_USER0);
> > > > > > >         kto = kmap_atomic(to, KM_USER1);
> > > > > > >         copy_page(kto, kfrom);
> > > > > > > +       __cpuc_flush_dcache_page(kto);
> > > > > > >         kunmap_atomic(kto, KM_USER1);
> > > > > > >         kunmap_atomic(kfrom, KM_USER0);
> > > > > > >  }
> > > > > >
> > > > > > Any thoughts one getting this merged (maybe in 2.6.33)? It seems that
> > > > > > no-one else has any views on this but the COW text pages problem is
> > > > > > still present.
> > > > >
> > > > > Well, it would be useful to get some comment from Nicolas since
> > > > > kunmap_atomic() will call this function for highmem pages.  We might want
> > > > > to avoid doing the dcache flush thing multiple times for these, but it
> > > > > looks to me like testing PageHighMem(to) leaves us a hole.
> > > >
> > > > Better test page_address(to) and skip the flush if it is NULL.  That's
> > > > what we do elsewhere already.  If page_addressto) is NULL then there is
> > > > no long lasting mapping for that page and the cache will be flushed upon
> > > > kunmap_atomic().  Hence:
> > > >
> > > > #ifdef CONFIG_HIGHMEM
> > > >       /*
> > > >        * kmap_atomic() doesn't set the page virtual address, and
> > > >        * kunmap_atomic() takes care of cache flushing already.
> > > >        */
> > > >       if (page_address(to) != NULL)
> > > > #endif
> > > >               __cpuc_flush_dcache_page(kto);
> > >
> > > Okay, can we get this combined with Catalin's patch and applied to all
> > > applicable processors (which I assume would be just v6 and v7) ?
> >
> > I'm ok with this.
> 
> Sorry, I wasn't clear enough.  I was expecting you to update your patch,
> and once the other copy_user_page stuff finally gets the "yes it solves
> the problem" response from Steven, it can be committed and merged into
> this branch.

OK, no problem. I thought you have a bigger pile of patches and just
squeeze this change somewhere. I'll post a new patch.

-- 
Catalin

[RFC 10/11] ARM: arrange for flush_cache_range() toalwaysflush the I-cache

[Russell King - ARM Linux]

On Tue, Nov 24, 2009 at 04:52:54PM +0000, Catalin Marinas wrote:
> OK, no problem. I thought you have a bigger pile of patches and just
> squeeze this change somewhere. I'll post a new patch.

I do - my plan all along was to add your patch on top of my 'cup' branch
(which is intended to contain all the copy_user_page fixes) and then merge
that into the appropriate place in the 'cache' series of branches.

[RFC 10/11] ARM: arrange for flush_cache_range() to always flush the I-cache

[Dirk Behme]

Catalin Marinas wrote:
> Russell,
> 
> On Thu, 2009-10-29 at 17:51 +0000, Catalin Marinas wrote:
>> On Wed, 2009-10-28 at 17:37 +0000, Russell King - ARM Linux wrote:
>>>  I'm not entirely
>>> sure what's happening with the other half of the fix (the thread seemed
>>> to die before reaching any kind of conclusion) but this is the right
>>> solution for the I-cache side of the problem.
>> I listed a few solutions there but not many replies. Let's try to sort
>> it out in this thread (maybe people got bored and ignored the other
>> thread completely :-)).
> [...]
>> Flush the D-cache during copy_user_highpage()
>>
>> From: Catalin Marinas <catalin.marinas@arm.com>
>>
>> The I and D caches for copy-on-write pages on processors with
>> write-allocate caches become incoherent causing problems on application
>> relying on CoW for text pages (dynamic linker relocating symbols in a
>> text page). This patch flushes the D-cache for such pages.
>>
>> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
>> ---
>>  arch/arm/mm/copypage-v6.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c
>> index 4127a7b..f19ed4e 100644
>> --- a/arch/arm/mm/copypage-v6.c
>> +++ b/arch/arm/mm/copypage-v6.c
>> @@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
>>  	kfrom = kmap_atomic(from, KM_USER0);
>>  	kto = kmap_atomic(to, KM_USER1);
>>  	copy_page(kto, kfrom);
>> +	__cpuc_flush_dcache_page(kto);
>>  	kunmap_atomic(kto, KM_USER1);
>>  	kunmap_atomic(kfrom, KM_USER0);
>>  }
> 
> Any thoughts one getting this merged (maybe in 2.6.33)? It seems that
> no-one else has any views on this but the COW text pages problem is
> still present.

Yes. ARM11MPCore still suffers from this issue. Please understand my 
silence as "I can't help with the discussion about this" :( And not 
"I'm not interested in it any more".

Many thanks for all the work you spent with this,

Dirk

[RFC 11/11] ARM: avoid flushing I-cache in flush_cache_mm()

[Russell King]

flush_cache_mm() is called in two cases:
1. when a process exits, just before the page tables are torn down.
   We can allow the stale lines to evict themselves over time without
   causing any harm.

2. when a process forks, and we've allocated a new ASID.
   The instruction cache issues are dealt with as pages are brought
   into the new process address space.  Flushing the I-cache here is
   therefore unnecessary.

However, we must keep the VIPT aliasing D-cache flush to ensure that
any dirty cache lines are not written back after the pages have been
reallocated for some other use - which would result in corruption.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mm/flush.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index ee7ce68..329594e 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -50,7 +50,6 @@ void flush_cache_mm(struct mm_struct *mm)
 		    :
 		    : "r" (0)
 		    : "cc");
-		__flush_icache_all();
 	}
 }
 
-- 
1.6.2.5

[RFC 11/11] ARM: avoid flushing I-cache in flush_cache_mm()

[Gilles Chanteperdrix]

Russell King wrote:
> flush_cache_mm() is called in two cases:
> 1. when a process exits, just before the page tables are torn down.
>    We can allow the stale lines to evict themselves over time without
>    causing any harm.

Is not there any risk to see cache lines evicted at a time when there
are no longer any page table entries for them? This was said to be a
problem during the early discussions on FCSE:

http://lists.arm.linux.org.uk/lurker/message/20071005.131816.8b478a49.en.html

-- 
                                          Gilles

[RFC 11/11] ARM: avoid flushing I-cache in flush_cache_mm()

[Russell King]

On Fri, Nov 13, 2009 at 04:01:30PM +0100, Gilles Chanteperdrix wrote:
> Russell King wrote:
> > flush_cache_mm() is called in two cases:
> > 1. when a process exits, just before the page tables are torn down.
> >    We can allow the stale lines to evict themselves over time without
> >    causing any harm.
> 
> Is not there any risk to see cache lines evicted at a time when there
> are no longer any page table entries for them? This was said to be a
> problem during the early discussions on FCSE:

The discussion here is about the instruction cache - that doesn't write
back cache lines.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

[RFC 11/11] ARM: avoid flushing I-cache in flush_cache_mm()

[Gilles Chanteperdrix]

Russell King wrote:
> On Fri, Nov 13, 2009 at 04:01:30PM +0100, Gilles Chanteperdrix wrote:
>> Russell King wrote:
>>> flush_cache_mm() is called in two cases:
>>> 1. when a process exits, just before the page tables are torn down.
>>>    We can allow the stale lines to evict themselves over time without
>>>    causing any harm.
>> Is not there any risk to see cache lines evicted at a time when there
>> are no longer any page table entries for them? This was said to be a
>> problem during the early discussions on FCSE:
> 
> The discussion here is about the instruction cache - that doesn't write
> back cache lines.

Ok. I should have read the patch. Sorry for the noise.

-- 
                                          Gilles

[RFC 00/11] Cache handling updates

[Catalin Marinas]

On Tue, 2009-10-27 at 22:40 +0000, Russell King wrote:
> Russell King (11):
>   ARM: Reduce __flush_dcache_page() visibility
>   ARM: Fix errata 411920 workarounds
>   ARM: move __flush_icache_all() out of flush_pfn_alias()
>   ARM: Remove __flush_icache_all() from __flush_dcache_page()
>   ARM: Avoid flush_dcache_page() for zero page
>   ARM: Avoid duplicated implementation for VIVT cache flushing
>   ARM: Allocate ZERO_PAGE from highmem
>   ARM: Avoid evaluating page_address() multiple times
>   ARM: Add I-cache invalidation for VIVT ASID tagged caches
>   ARM: arrange for flush_cache_range() to always flush the I-cache
>   ARM: avoid flushing I-cache in flush_cache_mm()

Apart from the comments I sent to the list, the other patches look ine
to me.

Thanks.

-- 
Catalin

[RFC 00/11] Cache handling updates

[Laurent Pinchart]

Hi Russel,

for some reason the patches seem not to have made it to the list. I haven't 
received them in my mailbox, and I can't find them in the archive.

-- 
Regards,

Laurent Pinchart

[RFC 00/11] Cache handling updates

[Catalin Marinas]

On Wed, 2009-10-28 at 17:17 +0100, Laurent Pinchart wrote:
> for some reason the patches seem not to have made it to the list. I haven't 
> received them in my mailbox, and I can't find them in the archive.

They came as reply without a Re:, so they are probably held in the
moderation queue. It would be good if the list checks for [RFC or [PATCH
but is for dwmw2 to fix.

-- 
Catalin

[RFC 00/11] Cache handling updates

[Pavel Machek]

On Wed 2009-10-28 16:28:44, Catalin Marinas wrote:
> On Wed, 2009-10-28 at 17:17 +0100, Laurent Pinchart wrote:
> > for some reason the patches seem not to have made it to the list. I haven't 
> > received them in my mailbox, and I can't find them in the archive.
> 
> They came as reply without a Re:, so they are probably held in the
> moderation queue. It would be good if the list checks for [RFC or [PATCH
> but is for dwmw2 to fix.

Or just kill the bogus check.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[RFC 00/11] Cache handling updates

[Russell King - ARM Linux]

On Wed, Oct 28, 2009 at 05:17:26PM +0100, Laurent Pinchart wrote:
> Hi Russel,

Grr.

> for some reason the patches seem not to have made it to the list. I haven't 
> received them in my mailbox, and I can't find them in the archive.

I've no idea either; they got stuck in the moderation queue last night
and I assume are still pending.

I'm sure the list admins will eventually get around to it.

[RFC 00/11] Cache handling updates

[Laurent Pinchart]

On Wednesday 28 October 2009 18:41:07 Russell King - ARM Linux wrote:
> On Wed, Oct 28, 2009 at 05:17:26PM +0100, Laurent Pinchart wrote:
> > Hi Russel,
> 
> Grr.

Sorry, I'll be more careful next time.
 
> > for some reason the patches seem not to have made it to the list. I
> > haven't received them in my mailbox, and I can't find them in the
> > archive.
> 
> I've no idea either; they got stuck in the moderation queue last night
> and I assume are still pending.
> 
> I'm sure the list admins will eventually get around to it.

Thanks.

-- 
Regards,

Laurent Pinchart