[PATCH 0/3] Cache maintenance on VIPT caches

[PATCH 0/3] Cache maintenance on VIPT caches

[Catalin Marinas]

Hi,

The first and third patches have already been posted in the same form.
The second patch have been modified to handle all the VIPT caches via
__sync_icache_dcache(). The initial use case for this patch was dealing
with an SMP race condition but following suggestions from Rabin, it was
extended to cover ARMv6 onwards, both UP and SMP.

Any Tested-by's are welcome.

Thanks.


Catalin Marinas (3):
      ARM: Assume new page cache pages have dirty D-cache
      ARM: Introduce __sync_icache_dcache() for VIPT caches
      ARM: Use lazy cache flushing on ARMv7 SMP systems


 arch/arm/include/asm/cacheflush.h |    6 ++---
 arch/arm/include/asm/pgtable.h    |   26 +++++++++++++++++++--
 arch/arm/include/asm/smp_plat.h   |    4 +++
 arch/arm/include/asm/tlbflush.h   |   12 ++++++++--
 arch/arm/mm/copypage-v4mc.c       |    2 +-
 arch/arm/mm/copypage-v6.c         |    2 +-
 arch/arm/mm/copypage-xscale.c     |    2 +-
 arch/arm/mm/dma-mapping.c         |    6 +++++
 arch/arm/mm/fault-armv.c          |    8 +++---
 arch/arm/mm/flush.c               |   46 +++++++++++++++++++++++++++++--------
 10 files changed, 89 insertions(+), 25 deletions(-)

-- 
Catalin

[PATCH 1/3] ARM: Assume new page cache pages have dirty D-cache

[Catalin Marinas]

There are places in Linux where writes to newly allocated page cache
pages happen without a subsequent call to flush_dcache_page() (several
PIO drivers including USB HCD). This patch changes the meaning of
PG_arch_1 to be PG_dcache_clean and always flush the D-cache for a newly
mapped page in update_mmu_cache().

The patch also sets the PG_arch_1 bit in the DMA cache maintenance
function to avoid additional cache flushing in update_mmu_cache().

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Rabin Vincent <rabin.vincent@stericsson.com>
---
 arch/arm/include/asm/cacheflush.h |    6 +++---
 arch/arm/include/asm/tlbflush.h   |    2 +-
 arch/arm/mm/copypage-v4mc.c       |    2 +-
 arch/arm/mm/copypage-v6.c         |    2 +-
 arch/arm/mm/copypage-xscale.c     |    2 +-
 arch/arm/mm/dma-mapping.c         |    6 ++++++
 arch/arm/mm/fault-armv.c          |    4 ++--
 arch/arm/mm/flush.c               |    3 ++-
 8 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
index 4656a24..d3730f0 100644
--- a/arch/arm/include/asm/cacheflush.h
+++ b/arch/arm/include/asm/cacheflush.h
@@ -137,10 +137,10 @@
 #endif
 
 /*
- * This flag is used to indicate that the page pointed to by a pte
- * is dirty and requires cleaning before returning it to the user.
+ * This flag is used to indicate that the page pointed to by a pte is clean
+ * and does not require cleaning before returning it to the user.
  */
-#define PG_dcache_dirty PG_arch_1
+#define PG_dcache_clean PG_arch_1
 
 /*
  *	MM Cache Management
diff --git a/arch/arm/include/asm/tlbflush.h b/arch/arm/include/asm/tlbflush.h
index bd863d8..40a7092 100644
--- a/arch/arm/include/asm/tlbflush.h
+++ b/arch/arm/include/asm/tlbflush.h
@@ -552,7 +552,7 @@ extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
 #endif
 
 /*
- * if PG_dcache_dirty is set for the page, we need to ensure that any
+ * If PG_dcache_clean is not set for the page, we need to ensure that any
  * cache entries for the kernels virtual memory range are written
  * back to the page.
  */
diff --git a/arch/arm/mm/copypage-v4mc.c b/arch/arm/mm/copypage-v4mc.c
index 598c51a..b806151 100644
--- a/arch/arm/mm/copypage-v4mc.c
+++ b/arch/arm/mm/copypage-v4mc.c
@@ -73,7 +73,7 @@ void v4_mc_copy_user_highpage(struct page *to, struct page *from,
 {
 	void *kto = kmap_atomic(to, KM_USER1);
 
-	if (test_and_clear_bit(PG_dcache_dirty, &from->flags))
+	if (!test_and_set_bit(PG_dcache_clean, &from->flags))
 		__flush_dcache_page(page_mapping(from), from);
 
 	spin_lock(&minicache_lock);
diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c
index f55fa10..bdba6c6 100644
--- a/arch/arm/mm/copypage-v6.c
+++ b/arch/arm/mm/copypage-v6.c
@@ -79,7 +79,7 @@ static void v6_copy_user_highpage_aliasing(struct page *to,
 	unsigned int offset = CACHE_COLOUR(vaddr);
 	unsigned long kfrom, kto;
 
-	if (test_and_clear_bit(PG_dcache_dirty, &from->flags))
+	if (!test_and_set_bit(PG_dcache_clean, &from->flags))
 		__flush_dcache_page(page_mapping(from), from);
 
 	/* FIXME: not highmem safe */
diff --git a/arch/arm/mm/copypage-xscale.c b/arch/arm/mm/copypage-xscale.c
index 9920c0a..649bbcd 100644
--- a/arch/arm/mm/copypage-xscale.c
+++ b/arch/arm/mm/copypage-xscale.c
@@ -95,7 +95,7 @@ void xscale_mc_copy_user_highpage(struct page *to, struct page *from,
 {
 	void *kto = kmap_atomic(to, KM_USER1);
 
-	if (test_and_clear_bit(PG_dcache_dirty, &from->flags))
+	if (!test_and_set_bit(PG_dcache_clean, &from->flags))
 		__flush_dcache_page(page_mapping(from), from);
 
 	spin_lock(&minicache_lock);
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 9e7742f..97c4744 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -508,6 +508,12 @@ void ___dma_page_dev_to_cpu(struct page *page, unsigned long off,
 		outer_inv_range(paddr, paddr + size);
 
 	dma_cache_maint_page(page, off, size, dir, dmac_unmap_area);
+
+	/*
+	 * Mark the D-cache clean for this page to avoid extra flushing.
+	 */
+	if (dir != DMA_TO_DEVICE)
+		set_bit(PG_dcache_clean, &page->flags);
 }
 EXPORT_SYMBOL(___dma_page_dev_to_cpu);
 
diff --git a/arch/arm/mm/fault-armv.c b/arch/arm/mm/fault-armv.c
index 9b906de..58846cb 100644
--- a/arch/arm/mm/fault-armv.c
+++ b/arch/arm/mm/fault-armv.c
@@ -141,7 +141,7 @@ make_coherent(struct address_space *mapping, struct vm_area_struct *vma,
  * a page table, or changing an existing PTE.  Basically, there are two
  * things that we need to take care of:
  *
- *  1. If PG_dcache_dirty is set for the page, we need to ensure
+ *  1. If PG_dcache_clean is not set for the page, we need to ensure
  *     that any cache entries for the kernels virtual memory
  *     range are written back to the page.
  *  2. If we have multiple shared mappings of the same space in
@@ -169,7 +169,7 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long addr,
 
 	mapping = page_mapping(page);
 #ifndef CONFIG_SMP
-	if (test_and_clear_bit(PG_dcache_dirty, &page->flags))
+	if (!test_and_set_bit(PG_dcache_clean, &page->flags))
 		__flush_dcache_page(mapping, page);
 #endif
 	if (mapping) {
diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index c6844cb..9f9919b 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -248,7 +248,7 @@ void flush_dcache_page(struct page *page)
 
 #ifndef CONFIG_SMP
 	if (!PageHighMem(page) && mapping && !mapping_mapped(mapping))
-		set_bit(PG_dcache_dirty, &page->flags);
+		clear_bit(PG_dcache_clean, &page->flags);
 	else
 #endif
 	{
@@ -257,6 +257,7 @@ void flush_dcache_page(struct page *page)
 			__flush_dcache_aliases(mapping, page);
 		else if (mapping)
 			__flush_icache_all();
+		set_bit(PG_dcache_clean, &page->flags);
 	}
 }
 EXPORT_SYMBOL(flush_dcache_page);

[PATCH 1/3] ARM: Assume new page cache pages have dirty D-cache

[Rabin Vincent]

On Fri, Jun 25, 2010 at 01:01:32PM +0100, Catalin Marinas wrote:
> @@ -508,6 +508,12 @@ void ___dma_page_dev_to_cpu(struct page *page, unsigned long off,
>  		outer_inv_range(paddr, paddr + size);
>  
>  	dma_cache_maint_page(page, off, size, dir, dmac_unmap_area);
> +
> +	/*
> +	 * Mark the D-cache clean for this page to avoid extra flushing.
> +	 */
> +	if (dir != DMA_TO_DEVICE)
> +		set_bit(PG_dcache_clean, &page->flags);

off + size does not necessarily cover the full page.  We probably
shouldn't be setting it as clean unless it does? 

Also, off + size can cover multiple physically contiguous pages, so
perhaps they too can be set as clean if they're fully covered.

Rabin

[PATCH 1/3] ARM: Assume new page cache pages have dirty D-cache

[Catalin Marinas]

On Mon, 2010-06-28 at 15:22 +0100, Rabin Vincent wrote:
> On Fri, Jun 25, 2010 at 01:01:32PM +0100, Catalin Marinas wrote:
> > @@ -508,6 +508,12 @@ void ___dma_page_dev_to_cpu(struct page *page, unsigned long off,
> >               outer_inv_range(paddr, paddr + size);
> > 
> >       dma_cache_maint_page(page, off, size, dir, dmac_unmap_area);
> > +
> > +     /*
> > +      * Mark the D-cache clean for this page to avoid extra flushing.
> > +      */
> > +     if (dir != DMA_TO_DEVICE)
> > +             set_bit(PG_dcache_clean, &page->flags);
> 
> off + size does not necessarily cover the full page.  We probably
> shouldn't be setting it as clean unless it does?

Good point. See below for an updated patch.

> Also, off + size can cover multiple physically contiguous pages, so
> perhaps they too can be set as clean if they're fully covered.

While it may be possible, I don't think that's common. I would only care
about the "page" argument, otherwise you have to do some conversion to
find the page structure and it may not be worth.


ARM: Assume new page cache pages have dirty D-cache

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

There are places in Linux where writes to newly allocated page cache
pages happen without a subsequent call to flush_dcache_page() (several
PIO drivers including USB HCD). This patch changes the meaning of
PG_arch_1 to be PG_dcache_clean and always flush the D-cache for a newly
mapped page in update_mmu_cache().

The patch also sets the PG_arch_1 bit in the DMA cache maintenance
function to avoid additional cache flushing in update_mmu_cache().

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Rabin Vincent <rabin.vincent@stericsson.com>
---
 arch/arm/include/asm/cacheflush.h |    6 +++---
 arch/arm/include/asm/tlbflush.h   |    2 +-
 arch/arm/mm/copypage-v4mc.c       |    2 +-
 arch/arm/mm/copypage-v6.c         |    2 +-
 arch/arm/mm/copypage-xscale.c     |    2 +-
 arch/arm/mm/dma-mapping.c         |    6 ++++++
 arch/arm/mm/fault-armv.c          |    4 ++--
 arch/arm/mm/flush.c               |    3 ++-
 8 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
index 4656a24..d3730f0 100644
--- a/arch/arm/include/asm/cacheflush.h
+++ b/arch/arm/include/asm/cacheflush.h
@@ -137,10 +137,10 @@
 #endif
 
 /*
- * This flag is used to indicate that the page pointed to by a pte
- * is dirty and requires cleaning before returning it to the user.
+ * This flag is used to indicate that the page pointed to by a pte is clean
+ * and does not require cleaning before returning it to the user.
  */
-#define PG_dcache_dirty PG_arch_1
+#define PG_dcache_clean PG_arch_1
 
 /*
  *	MM Cache Management
diff --git a/arch/arm/include/asm/tlbflush.h b/arch/arm/include/asm/tlbflush.h
index bd863d8..40a7092 100644
--- a/arch/arm/include/asm/tlbflush.h
+++ b/arch/arm/include/asm/tlbflush.h
@@ -552,7 +552,7 @@ extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
 #endif
 
 /*
- * if PG_dcache_dirty is set for the page, we need to ensure that any
+ * If PG_dcache_clean is not set for the page, we need to ensure that any
  * cache entries for the kernels virtual memory range are written
  * back to the page.
  */
diff --git a/arch/arm/mm/copypage-v4mc.c b/arch/arm/mm/copypage-v4mc.c
index 598c51a..b806151 100644
--- a/arch/arm/mm/copypage-v4mc.c
+++ b/arch/arm/mm/copypage-v4mc.c
@@ -73,7 +73,7 @@ void v4_mc_copy_user_highpage(struct page *to, struct page *from,
 {
 	void *kto = kmap_atomic(to, KM_USER1);
 
-	if (test_and_clear_bit(PG_dcache_dirty, &from->flags))
+	if (!test_and_set_bit(PG_dcache_clean, &from->flags))
 		__flush_dcache_page(page_mapping(from), from);
 
 	spin_lock(&minicache_lock);
diff --git a/arch/arm/mm/copypage-v6.c b/arch/arm/mm/copypage-v6.c
index f55fa10..bdba6c6 100644
--- a/arch/arm/mm/copypage-v6.c
+++ b/arch/arm/mm/copypage-v6.c
@@ -79,7 +79,7 @@ static void v6_copy_user_highpage_aliasing(struct page *to,
 	unsigned int offset = CACHE_COLOUR(vaddr);
 	unsigned long kfrom, kto;
 
-	if (test_and_clear_bit(PG_dcache_dirty, &from->flags))
+	if (!test_and_set_bit(PG_dcache_clean, &from->flags))
 		__flush_dcache_page(page_mapping(from), from);
 
 	/* FIXME: not highmem safe */
diff --git a/arch/arm/mm/copypage-xscale.c b/arch/arm/mm/copypage-xscale.c
index 9920c0a..649bbcd 100644
--- a/arch/arm/mm/copypage-xscale.c
+++ b/arch/arm/mm/copypage-xscale.c
@@ -95,7 +95,7 @@ void xscale_mc_copy_user_highpage(struct page *to, struct page *from,
 {
 	void *kto = kmap_atomic(to, KM_USER1);
 
-	if (test_and_clear_bit(PG_dcache_dirty, &from->flags))
+	if (!test_and_set_bit(PG_dcache_clean, &from->flags))
 		__flush_dcache_page(page_mapping(from), from);
 
 	spin_lock(&minicache_lock);
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 9e7742f..fa3d07d 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -508,6 +508,12 @@ void ___dma_page_dev_to_cpu(struct page *page, unsigned long off,
 		outer_inv_range(paddr, paddr + size);
 
 	dma_cache_maint_page(page, off, size, dir, dmac_unmap_area);
+
+	/*
+	 * Mark the D-cache clean for this page to avoid extra flushing.
+	 */
+	if (dir != DMA_TO_DEVICE && off == 0 && size >= PAGE_SIZE)
+		set_bit(PG_dcache_clean, &page->flags);
 }
 EXPORT_SYMBOL(___dma_page_dev_to_cpu);
 
diff --git a/arch/arm/mm/fault-armv.c b/arch/arm/mm/fault-armv.c
index 9b906de..58846cb 100644
--- a/arch/arm/mm/fault-armv.c
+++ b/arch/arm/mm/fault-armv.c
@@ -141,7 +141,7 @@ make_coherent(struct address_space *mapping, struct vm_area_struct *vma,
  * a page table, or changing an existing PTE.  Basically, there are two
  * things that we need to take care of:
  *
- *  1. If PG_dcache_dirty is set for the page, we need to ensure
+ *  1. If PG_dcache_clean is not set for the page, we need to ensure
  *     that any cache entries for the kernels virtual memory
  *     range are written back to the page.
  *  2. If we have multiple shared mappings of the same space in
@@ -169,7 +169,7 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long addr,
 
 	mapping = page_mapping(page);
 #ifndef CONFIG_SMP
-	if (test_and_clear_bit(PG_dcache_dirty, &page->flags))
+	if (!test_and_set_bit(PG_dcache_clean, &page->flags))
 		__flush_dcache_page(mapping, page);
 #endif
 	if (mapping) {
diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index c6844cb..9f9919b 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -248,7 +248,7 @@ void flush_dcache_page(struct page *page)
 
 #ifndef CONFIG_SMP
 	if (!PageHighMem(page) && mapping && !mapping_mapped(mapping))
-		set_bit(PG_dcache_dirty, &page->flags);
+		clear_bit(PG_dcache_clean, &page->flags);
 	else
 #endif
 	{
@@ -257,6 +257,7 @@ void flush_dcache_page(struct page *page)
 			__flush_dcache_aliases(mapping, page);
 		else if (mapping)
 			__flush_icache_all();
+		set_bit(PG_dcache_clean, &page->flags);
 	}
 }
 EXPORT_SYMBOL(flush_dcache_page);

[PATCH 2/3] ARM: Introduce __sync_icache_dcache() for VIPT caches

[Catalin Marinas]

On SMP systems, there is a small chance of a PTE becoming visible to a
different CPU before the current cache maintenance operations in
update_mmu_cache(). To avoid this, cache maintenance must be handled in
set_pte_at() (similar to IA-64 and PowerPC).

This patch provides a unified VIPT cache handling mechanism and
implements the __sync_icache_dcache() function for ARMv6 onwards
architectures. It is called from set_pte_at() and replaces the
update_mmu_cache(). The latter is still used on VIVT hardware where a
vm_area_struct is required.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Rabin Vincent <rabin.vincent@stericsson.com>
---
 arch/arm/include/asm/pgtable.h  |   26 +++++++++++++++++++++++---
 arch/arm/include/asm/tlbflush.h |   10 +++++++++-
 arch/arm/mm/fault-armv.c        |    4 ++--
 arch/arm/mm/flush.c             |   30 ++++++++++++++++++++++++++++++
 4 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index ab68cf1..42e694f 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -278,9 +278,24 @@ extern struct page *empty_zero_page;
 
 #define set_pte_ext(ptep,pte,ext) cpu_set_pte_ext(ptep,pte,ext)
 
-#define set_pte_at(mm,addr,ptep,pteval) do { \
-	set_pte_ext(ptep, pteval, (addr) >= TASK_SIZE ? 0 : PTE_EXT_NG); \
- } while (0)
+#if __LINUX_ARM_ARCH__ < 6
+static inline void __sync_icache_dcache(pte_t pteval)
+{
+}
+#else
+extern void __sync_icache_dcache(pte_t pteval);
+#endif
+
+static inline void set_pte_at(struct mm_struct *mm, unsigned long addr,
+			      pte_t *ptep, pte_t pteval)
+{
+	if (addr >= TASK_SIZE)
+		set_pte_ext(ptep, pteval, 0);
+	else {
+		__sync_icache_dcache(pteval);
+		set_pte_ext(ptep, pteval, PTE_EXT_NG);
+	}
+}
 
 /*
  * The following only work if pte_present() is true.
@@ -290,8 +305,13 @@ extern struct page *empty_zero_page;
 #define pte_write(pte)		(pte_val(pte) & L_PTE_WRITE)
 #define pte_dirty(pte)		(pte_val(pte) & L_PTE_DIRTY)
 #define pte_young(pte)		(pte_val(pte) & L_PTE_YOUNG)
+#define pte_exec(pte)		(pte_val(pte) & L_PTE_EXEC)
 #define pte_special(pte)	(0)
 
+#define pte_present_user(pte) \
+	((pte_val(pte) & (L_PTE_PRESENT | L_PTE_USER)) == \
+	 (L_PTE_PRESENT | L_PTE_USER))
+
 #define PTE_BIT_FUNC(fn,op) \
 static inline pte_t pte_##fn(pte_t pte) { pte_val(pte) op; return pte; }
 
diff --git a/arch/arm/include/asm/tlbflush.h b/arch/arm/include/asm/tlbflush.h
index 40a7092..8ec4775 100644
--- a/arch/arm/include/asm/tlbflush.h
+++ b/arch/arm/include/asm/tlbflush.h
@@ -554,10 +554,18 @@ extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
 /*
  * If PG_dcache_clean is not set for the page, we need to ensure that any
  * cache entries for the kernels virtual memory range are written
- * back to the page.
+ * back to the page. On ARMv6 and later, the cache coherency is handled via
+ * the set_pte_at() function.
  */
+#if __LINUX_ARM_ARCH__ < 6
 extern void update_mmu_cache(struct vm_area_struct *vma, unsigned long addr,
 	pte_t *ptep);
+#else
+static inline void update_mmu_cache(struct vm_area_struct *vma,
+				    unsigned long addr, pte_t *ptep)
+{
+}
+#endif
 
 #endif
 
diff --git a/arch/arm/mm/fault-armv.c b/arch/arm/mm/fault-armv.c
index 58846cb..8440d95 100644
--- a/arch/arm/mm/fault-armv.c
+++ b/arch/arm/mm/fault-armv.c
@@ -28,6 +28,7 @@
 
 static unsigned long shared_pte_mask = L_PTE_MT_BUFFERABLE;
 
+#if __LINUX_ARM_ARCH__ < 6
 /*
  * We take the easy way out of this problem - we make the
  * PTE uncacheable.  However, we leave the write buffer on.
@@ -168,10 +169,8 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long addr,
 		return;
 
 	mapping = page_mapping(page);
-#ifndef CONFIG_SMP
 	if (!test_and_set_bit(PG_dcache_clean, &page->flags))
 		__flush_dcache_page(mapping, page);
-#endif
 	if (mapping) {
 		if (cache_is_vivt())
 			make_coherent(mapping, vma, addr, ptep, pfn);
@@ -179,6 +178,7 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long addr,
 			__flush_icache_all();
 	}
 }
+#endif	/* __LINUX_ARM_ARCH__ < 6 */
 
 /*
  * Check whether the write buffer has physical address aliasing
diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index 9f9919b..78134f5 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -215,6 +215,36 @@ static void __flush_dcache_aliases(struct address_space *mapping, struct page *p
 	flush_dcache_mmap_unlock(mapping);
 }
 
+#if __LINUX_ARM_ARCH__ >= 6
+void __sync_icache_dcache(pte_t pteval)
+{
+	unsigned long pfn;
+	struct page *page;
+	struct address_space *mapping;
+
+	if (!pte_present_user(pteval))
+		return;
+	if (cache_is_vipt_nonaliasing() && !pte_exec(pteval))
+		/* only flush non-aliasing VIPT caches for exec mappings */
+		return;
+	pfn = pte_pfn(pteval);
+	if (!pfn_valid(pfn))
+		return;
+
+	page = pfn_to_page(pfn);
+	if (cache_is_vipt_aliasing())
+		mapping = page_mapping(page);
+	else
+		mapping = NULL;
+
+	if (!test_and_set_bit(PG_dcache_clean, &page->flags))
+		__flush_dcache_page(mapping, page);
+	/* pte_exec() already checked above for non-aliasing VIPT cache */
+	if (cache_is_vipt_nonaliasing() || pte_exec(pteval))
+		__flush_icache_all();
+}
+#endif
+
 /*
  * Ensure cache coherency between kernel mapping and userspace mapping
  * of this page.

[PATCH 3/3] ARM: Use lazy cache flushing on ARMv7 SMP systems

[Catalin Marinas]

ARMv7 processors like Cortex-A9 broadcast the cache maintenance
operations in hardware. This patch allows the
flush_dcache_page/update_mmu_cache pair to work in lazy flushing mode
similar to the UP case.

Note that cache flushing on SMP systems now takes place via the
set_pte_at() call (__sync_icache_dcache) and there is no race with other
CPUs executing code from the new PTE before the cache flushing took
place.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Rabin Vincent <rabin.vincent@stericsson.com>
---
 arch/arm/include/asm/smp_plat.h |    4 ++++
 arch/arm/mm/flush.c             |   13 ++++---------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/arm/include/asm/smp_plat.h b/arch/arm/include/asm/smp_plat.h
index e621530..963a338 100644
--- a/arch/arm/include/asm/smp_plat.h
+++ b/arch/arm/include/asm/smp_plat.h
@@ -13,9 +13,13 @@ static inline int tlb_ops_need_broadcast(void)
 	return ((read_cpuid_ext(CPUID_EXT_MMFR3) >> 12) & 0xf) < 2;
 }
 
+#if !defined(CONFIG_SMP) || __LINUX_ARM_ARCH__ >= 7
+#define cache_ops_need_broadcast()	0
+#else
 static inline int cache_ops_need_broadcast(void)
 {
 	return ((read_cpuid_ext(CPUID_EXT_MMFR3) >> 12) & 0xf) < 1;
 }
+#endif
 
 #endif
diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c
index 78134f5..53f83a7 100644
--- a/arch/arm/mm/flush.c
+++ b/arch/arm/mm/flush.c
@@ -17,6 +17,7 @@
 #include <asm/smp_plat.h>
 #include <asm/system.h>
 #include <asm/tlbflush.h>
+#include <asm/smp_plat.h>
 
 #include "mm.h"
 
@@ -93,12 +94,10 @@ void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig
 #define flush_pfn_alias(pfn,vaddr)	do { } while (0)
 #endif
 
-#ifdef CONFIG_SMP
 static void flush_ptrace_access_other(void *args)
 {
 	__flush_icache_all();
 }
-#endif
 
 static
 void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
@@ -122,11 +121,9 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 	if (vma->vm_flags & VM_EXEC) {
 		unsigned long addr = (unsigned long)kaddr;
 		__cpuc_coherent_kern_range(addr, addr + len);
-#ifdef CONFIG_SMP
 		if (cache_ops_need_broadcast())
 			smp_call_function(flush_ptrace_access_other,
 					  NULL, 1);
-#endif
 	}
 }
 
@@ -276,12 +273,10 @@ void flush_dcache_page(struct page *page)
 
 	mapping = page_mapping(page);
 
-#ifndef CONFIG_SMP
-	if (!PageHighMem(page) && mapping && !mapping_mapped(mapping))
+	if (!cache_ops_need_broadcast() &&
+	    !PageHighMem(page) && mapping && !mapping_mapped(mapping))
 		clear_bit(PG_dcache_clean, &page->flags);
-	else
-#endif
-	{
+	else {
 		__flush_dcache_page(mapping, page);
 		if (mapping && cache_is_vivt())
 			__flush_dcache_aliases(mapping, page);

[PATCH 0/3] Cache maintenance on VIPT caches

[Rabin VINCENT]

On Fri, Jun 25, 2010 at 14:01:27 +0200, Catalin Marinas wrote:
> The first and third patches have already been posted in the same form.
> The second patch have been modified to handle all the VIPT caches via
> __sync_icache_dcache(). The initial use case for this patch was dealing
> with an SMP race condition but following suggestions from Rabin, it was
> extended to cover ARMv6 onwards, both UP and SMP.
> 
> Any Tested-by's are welcome.

This version also fixes the MMC rootfs init crashes, without the need
for the flush_kernel_dcache_page() change:

Tested-by: Rabin Vincent <rabin.vincent@stericsson.com>

Will you be submitting this patch for linux-next?  I ask because the
mmci patches that I posted convert that driver to use the sg_miter API
(which uses flush_kernel_dcache_page() internally), and not do any
flushing inside the driver itself.  Or do you think it would be
appropriate to have the driver call flush_dcache_page() explicitly?
(Although this would be double flushing on systems with aliasing caches
where flush_kernel_dcache_page() is not a no-op.)

Rabin

[PATCH 0/3] Cache maintenance on VIPT caches

[Catalin Marinas]

On Fri, 2010-07-16 at 14:19 +0100, Rabin VINCENT wrote:
> On Fri, Jun 25, 2010 at 14:01:27 +0200, Catalin Marinas wrote:
> > The first and third patches have already been posted in the same form.
> > The second patch have been modified to handle all the VIPT caches via
> > __sync_icache_dcache(). The initial use case for this patch was dealing
> > with an SMP race condition but following suggestions from Rabin, it was
> > extended to cover ARMv6 onwards, both UP and SMP.
> >
> > Any Tested-by's are welcome.
> 
> This version also fixes the MMC rootfs init crashes, without the need
> for the flush_kernel_dcache_page() change:
> 
> Tested-by: Rabin Vincent <rabin.vincent@stericsson.com>

Thanks for testing.

> Will you be submitting this patch for linux-next?  

I'd like to but I'm still waiting for Russell's feedback (the linux-next
rules are that we shouldn't dump patches there that haven't been
reviewed yet).

> I ask because the
> mmci patches that I posted convert that driver to use the sg_miter API
> (which uses flush_kernel_dcache_page() internally), and not do any
> flushing inside the driver itself.  Or do you think it would be
> appropriate to have the driver call flush_dcache_page() explicitly?
> (Although this would be double flushing on systems with aliasing caches
> where flush_kernel_dcache_page() is not a no-op.)

I'm not sure why sg_miter doesn't call flush_dcache_page() but
flush_kernel_dcache_page() (I cc'ed Fujita as he seems to have added
this code).

Anyway, we could actually set the PG_dcache_clean bit in
flush_kernel_dcache_page() and thus avoid additional flushing when the
page gets mapped to user space (on VIPT non-aliasing caches).

I think it's fine for your driver not to call flush_dcache_page() as
long as a form of page flushing is done in the sg_miter API. The only
thing we miss is ARM11MPCore with VIPT non-aliasing caches where
flush_kernel_dcache_page() is a no-op, though we have to do non-lazy
flushing because the cache maintenance operations aren't broadcast to
the other CPUs.

There are several places where we can fix this - (1) implement
flush_kernel_dcache_page() for ARM11MPCore, (2) use flush_dcache_page()
in the sg_miter code or (3) call flush_dcache_page() in the mmci driver.

-- 
Catalin

[PATCH 0/3] Cache maintenance on VIPT caches

[FUJITA Tomonori]

Cc'ed to linux-arch

On Fri, 16 Jul 2010 15:39:17 +0100
Catalin Marinas <catalin.marinas@arm.com> wrote:

> On Fri, 2010-07-16 at 14:19 +0100, Rabin VINCENT wrote:
> > On Fri, Jun 25, 2010 at 14:01:27 +0200, Catalin Marinas wrote:
> > > The first and third patches have already been posted in the same form.
> > > The second patch have been modified to handle all the VIPT caches via
> > > __sync_icache_dcache(). The initial use case for this patch was dealing
> > > with an SMP race condition but following suggestions from Rabin, it was
> > > extended to cover ARMv6 onwards, both UP and SMP.
> > >
> > > Any Tested-by's are welcome.
> > 
> > This version also fixes the MMC rootfs init crashes, without the need
> > for the flush_kernel_dcache_page() change:
> > 
> > Tested-by: Rabin Vincent <rabin.vincent@stericsson.com>
> 
> Thanks for testing.

This patchset for arm handles I/D coherency (and D aliasing) with
PG_arch_1, right? If so, can you please send it to linux-arch? I guess
that some non arm people might be interested in it.


> > I ask because the
> > mmci patches that I posted convert that driver to use the sg_miter API
> > (which uses flush_kernel_dcache_page() internally), and not do any
> > flushing inside the driver itself.  Or do you think it would be
> > appropriate to have the driver call flush_dcache_page() explicitly?
> > (Although this would be double flushing on systems with aliasing caches
> > where flush_kernel_dcache_page() is not a no-op.)
> 
> I'm not sure why sg_miter doesn't call flush_dcache_page() but
> flush_kernel_dcache_page() (I cc'ed Fujita as he seems to have added
> this code).

It is intended to handle D aliasing due to kmap. As cachetlb.txt says
that it is assumed here that the user has no incoherent cached copies
(it was already addressed elsewhere). So flush_dcache_page() does too
much there.

[PATCH 0/3] Cache maintenance on VIPT caches

[Catalin Marinas]

On Tue, 2010-07-20 at 18:56 +0900, FUJITA Tomonori wrote:
> On Fri, 16 Jul 2010 15:39:17 +0100
> Catalin Marinas <catalin.marinas@arm.com> wrote:
> 
> > On Fri, 2010-07-16 at 14:19 +0100, Rabin VINCENT wrote:
> > > On Fri, Jun 25, 2010 at 14:01:27 +0200, Catalin Marinas wrote:
> > > > The first and third patches have already been posted in the same form.
> > > > The second patch have been modified to handle all the VIPT caches via
> > > > __sync_icache_dcache(). The initial use case for this patch was dealing
> > > > with an SMP race condition but following suggestions from Rabin, it was
> > > > extended to cover ARMv6 onwards, both UP and SMP.
> > > >
> > > > Any Tested-by's are welcome.
> > > 
> > > This version also fixes the MMC rootfs init crashes, without the need
> > > for the flush_kernel_dcache_page() change:
> > > 
> > > Tested-by: Rabin Vincent <rabin.vincent@stericsson.com>
> > 
> > Thanks for testing.
> 
> This patchset for arm handles I/D coherency (and D aliasing) with
> PG_arch_1, right? If so, can you please send it to linux-arch? I guess
> that some non arm people might be interested in it.

Yes, I will.

ARM uses lazy D-cache flushing as per cachetlb.txt. However, this file
recommends that the meaning of PG_arch_1 is 'dirty' and the bit is set
by flush_dcache_page(). It works fine if all PIO drivers would call
flush_dcache_page() but there are many (the majority?) that don't and we
end up with data corruption in user space. So rather than changing all
the drivers or proposing a PIO API (tried this as well :)), we changed
the meaning of PG_arch_1 to 'clean'. That's already done by PowerPC and
IA-64.

This was discussed on linux-arch on several occasions and I also posted
a patch to change the recommendations in cachetlb.txt
(http://thread.gmane.org/gmane.linux.kernel.cross-arch/6187).

> > > I ask because the
> > > mmci patches that I posted convert that driver to use the sg_miter API
> > > (which uses flush_kernel_dcache_page() internally), and not do any
> > > flushing inside the driver itself.  Or do you think it would be
> > > appropriate to have the driver call flush_dcache_page() explicitly?
> > > (Although this would be double flushing on systems with aliasing caches
> > > where flush_kernel_dcache_page() is not a no-op.)
> > 
> > I'm not sure why sg_miter doesn't call flush_dcache_page() but
> > flush_kernel_dcache_page() (I cc'ed Fujita as he seems to have added
> > this code).
> 
> It is intended to handle D aliasing due to kmap. As cachetlb.txt says
> that it is assumed here that the user has no incoherent cached copies
> (it was already addressed elsewhere). So flush_dcache_page() does too
> much there.

OK. So in this case a PIO driver using sg_miter should still call
flush_dcache_page() as usual (or not at all as we change the meaning
PG_arch_1 for the architectures where this matters).

Thanks.

-- 
Catalin

[PATCH 0/3] Cache maintenance on VIPT caches

[Catalin Marinas]

On Tue, 2010-07-20 at 12:37 +0100, Catalin Marinas wrote:
> On Tue, 2010-07-20 at 18:56 +0900, FUJITA Tomonori wrote:
> > On Fri, 16 Jul 2010 15:39:17 +0100
> > Catalin Marinas <catalin.marinas@arm.com> wrote:
> >
> > > On Fri, 2010-07-16 at 14:19 +0100, Rabin VINCENT wrote:
> > > > I ask because the
> > > > mmci patches that I posted convert that driver to use the sg_miter API
> > > > (which uses flush_kernel_dcache_page() internally), and not do any
> > > > flushing inside the driver itself.  Or do you think it would be
> > > > appropriate to have the driver call flush_dcache_page() explicitly?
> > > > (Although this would be double flushing on systems with aliasing caches
> > > > where flush_kernel_dcache_page() is not a no-op.)
> > >
> > > I'm not sure why sg_miter doesn't call flush_dcache_page() but
> > > flush_kernel_dcache_page() (I cc'ed Fujita as he seems to have added
> > > this code).
> >
> > It is intended to handle D aliasing due to kmap. As cachetlb.txt says
> > that it is assumed here that the user has no incoherent cached copies
> > (it was already addressed elsewhere). So flush_dcache_page() does too
> > much there.
> 
> OK. So in this case a PIO driver using sg_miter should still call
> flush_dcache_page() as usual (or not at all as we change the meaning
> PG_arch_1 for the architectures where this matters).

Actually the flush_dcache_page() call in drivers is still useful on SMP
systems where the cache maintenance must happen on the CPU that dirtied
the cache (ARM11MPCore), though I have a workaround for this as well
(doing a 'read-for-ownership' before flushing the D-cache).

-- 
Catalin

[PATCH 0/3] Cache maintenance on VIPT caches

[Catalin Marinas]

On Tue, 2010-07-20 at 18:56 +0900, FUJITA Tomonori wrote:
> On Fri, 16 Jul 2010 15:39:17 +0100
> Catalin Marinas <catalin.marinas@arm.com> wrote:
> > I'm not sure why sg_miter doesn't call flush_dcache_page() but
> > flush_kernel_dcache_page() (I cc'ed Fujita as he seems to have added
> > this code).
> 
> It is intended to handle D aliasing due to kmap. As cachetlb.txt says
> that it is assumed here that the user has no incoherent cached copies
> (it was already addressed elsewhere). So flush_dcache_page() does too
> much there.

A related question here - does flush_kernel_dcache_page() need to do
anything for non-highmem pages? For highmem pages, on ARM, we already
handle the cache flushing in kunmap().

-- 
Catalin

[PATCH 0/3] Cache maintenance on VIPT caches

[Nicolas Pitre]

On Tue, 20 Jul 2010, Catalin Marinas wrote:

> On Tue, 2010-07-20 at 18:56 +0900, FUJITA Tomonori wrote:
> > On Fri, 16 Jul 2010 15:39:17 +0100
> > Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > I'm not sure why sg_miter doesn't call flush_dcache_page() but
> > > flush_kernel_dcache_page() (I cc'ed Fujita as he seems to have added
> > > this code).
> > 
> > It is intended to handle D aliasing due to kmap. As cachetlb.txt says
> > that it is assumed here that the user has no incoherent cached copies
> > (it was already addressed elsewhere). So flush_dcache_page() does too
> > much there.
> 
> A related question here - does flush_kernel_dcache_page() need to do
> anything for non-highmem pages? For highmem pages, on ARM, we already
> handle the cache flushing in kunmap().

Well, with your proposed patch we wouldn't anymore.  And semantically we 
shouldn't have to.  Even the fact that kunmap_atomic() does flush the 
cache on VIVT system is an implementation issue not a semantic 
definition (incidentally on VIPT it is not flushed).


Nicolas