Skip to contentSkip to conversation

arm64: change PoC D-cache flush to PoU

PATCH Superseded Ashok Kumar 2 participants 4 messages Dec 15, 2015 arm64

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover arm64: change PoC D-cache flush to PoU

For keeping I and D coherent, dcache flush till PoU(Point of Unification) should be sufficient instead of doing till PoC(Point of coherence). In SoC with more levels of cache, there could be a performance hit in doing flush till PoC as __flush_dcache_area does both flush and invalidate. Introduced new API __clean_dcache_area_pou which does only clean till PoU.

Also deferred dcache flush in __cpu_copy_user_page to __sync_icache_dcache.

changes since v1 [1]:
Incorporated Mark Rutland's review comments of
* renaming __flush_dcache_area_pou to __clean_dcache_area_pou
* using inner shareable domain for dsb in __clean_dcache_area_pou
* having a common macro for __flush_dcache_area and __clean_dcache_area_pou.

Thanks,
Ashok

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-December/393527.html

Ashok Kumar (2):
arm64: Defer dcache flush in __cpu_copy_user_page
arm64: Use PoU cache instr for I/D coherency

 arch/arm64/include/asm/cacheflush.h |  1 +
 arch/arm64/mm/cache.S               | 47 ++++++++++++++++++++++++++++++-------
 arch/arm64/mm/copypage.c            |  3 ++-
 arch/arm64/mm/flush.c               | 14 +++++++----
 4 files changed, 51 insertions(+), 14 deletions(-)
Signature · 1 line
2.1.0
#1 lore raw

PATCH v2 1/2 arm64: Defer dcache flush in __cpu_copy_user_page

Defer dcache flushing to __sync_icache_dcache by calling flush_dcache_page which clears PG_dcache_clean flag.

---
 arch/arm64/mm/copypage.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/mm/copypage.c +2 -1
index 13bbc3be..22e4cb4 100644
@@ -24,8 +24,9 @@
 
 void __cpu_copy_user_page(void *kto, const void *kfrom, unsigned long vaddr)
 {
+	struct page *page = virt_to_page(kto);
 	copy_page(kto, kfrom);
-	__flush_dcache_area(kto, PAGE_SIZE);
+	flush_dcache_page(page);
 }
 EXPORT_SYMBOL_GPL(__cpu_copy_user_page);
 
Signature · 1 line
2.1.0
#2 lore raw

PATCH v2 2/2 arm64: Use PoU cache instr for I/D coherency

In reply to Ashok Kumar

In systems with three levels of cache(PoU at L1 and PoC at L3), PoC cache flush instructions flushes L2 and L3 caches which could affect performance.
For cache flushes for I and D coherency, PoU should suffice. So changing all I and D coherency related cache flushes to PoU.

Introduced a new __clean_dcache_area_pou API for dcache flush till PoU and provided a common macro for __flush_dcache_area and __clean_dcache_area_pou.

---
 arch/arm64/include/asm/cacheflush.h |  1 +
 arch/arm64/mm/cache.S               | 47 ++++++++++++++++++++++++++++++-------
 arch/arm64/mm/flush.c               | 14 +++++++----
 3 files changed, 49 insertions(+), 13 deletions(-)
arch/arm64/include/asm/cacheflush.h +1 -0
index c75b8d0..6a5ecbd 100644
@@ -68,6 +68,7 @@
 extern void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned long end);
 extern void flush_icache_range(unsigned long start, unsigned long end);
 extern void __flush_dcache_area(void *addr, size_t len);
+extern void __clean_dcache_area_pou(void *addr, size_t len);
 extern long __flush_cache_user_range(unsigned long start, unsigned long end);
 
 static inline void flush_cache_mm(struct mm_struct *mm)
arch/arm64/mm/cache.S +38 -9

66 lines not on this page · raw message

arch/arm64/mm/flush.c +10 -4
index c26b804..4d4d15e 100644
@@ -41,7 +41,7 @@ static void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 	if (vma->vm_flags & VM_EXEC) {
 		unsigned long addr = (unsigned long)kaddr;
 		if (icache_is_aliasing()) {
-			__flush_dcache_area(kaddr, len);
+			__clean_dcache_area_pou(kaddr, len);
 			__flush_icache_all();
 		} else {
 			flush_icache_range(addr, addr + len);
@@ -75,9 +75,15 @@ void __sync_icache_dcache(pte_t pte, unsigned long addr)
 		return;
 
 	if (!test_and_set_bit(PG_dcache_clean, &page->flags)) {
-		__flush_dcache_area(page_address(page),
-				PAGE_SIZE << compound_order(page));
-		__flush_icache_all();
+		if (icache_is_aliasing()) {
+			__clean_dcache_area_pou(page_address(page),
+					    PAGE_SIZE << compound_order(page));
+			__flush_icache_all();
+		} else {
+			flush_icache_range((unsigned long)page_address(page),
+					   (unsigned long)page_address(page) +
+					   (PAGE_SIZE << compound_order(page)));
+		}
 	} else if (icache_is_aivivt()) {
 		__flush_icache_all();
 	}
Signature · 1 line
2.1.0
reviewer
#3 lore raw

PATCH v2 2/2 arm64: Use PoU cache instr for I/D coherency

Quoted 34 lines · Ashok Kumar

In systems with three levels of cache(PoU at L1 and PoC at L3), PoC cache flush instructions flushes L2 and L3 caches which could affect performance.
For cache flushes for I and D coherency, PoU should suffice. So changing all I and D coherency related cache flushes to PoU.

Introduced a new __clean_dcache_area_pou API for dcache flush till PoU and provided a common macro for __flush_dcache_area and __clean_dcache_area_pou.

---
 arch/arm64/include/asm/cacheflush.h |  1 +
 arch/arm64/mm/cache.S               | 47 ++++++++++++++++++++++++++++++-------
 arch/arm64/mm/flush.c               | 14 +++++++----
 3 files changed, 49 insertions(+), 13 deletions(-)
arch/arm64/include/asm/cacheflush.h +1 -0
index c75b8d0..6a5ecbd 100644
@@ -68,6 +68,7 @@
 extern void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned long end);
 extern void flush_icache_range(unsigned long start, unsigned long end);
 extern void __flush_dcache_area(void *addr, size_t len);
+extern void __clean_dcache_area_pou(void *addr, size_t len);
 extern long __flush_cache_user_range(unsigned long start, unsigned long end);
 
 static inline void flush_cache_mm(struct mm_struct *mm)
arch/arm64/mm/cache.S +0 -0
index eb48d5d..1fdcb38 100644

arch/arm64/mm/cache.S ENDPROC(flush_icache_range) · line 82

Quoted 4 lines
arch/arm64/mm/cache.S +1 -0
@@ -79,28 +79,57 @@ ENDPROC(flush_icache_range)
 ENDPROC(__flush_cache_user_range)
 
 /*
+ * Macro to flush/invalidate cache lines till PoU/PoC

To keep this short and not use the confusing "flush" terminology, I think it would be better to just say:

Macro to perform a data cache maintenance for the interval [kaddr, kkaddr + size).

Quoted 17 lines
arch/arm64/mm/cache.S +17 -0
+ *
+ * 	op:		operation passed to dc instruction
+ * 	domain:		domain used in dsb instruciton
+ * 	kaddr:		starting virtual address of the region
+ * 	size:		size of the region
+ * 	Corrupts: 	kaddr, size, tmp1, tmp2
+ */
+	.macro dcache_by_line_op op, domain, kaddr, size, tmp1, tmp2
+	dcache_line_size \tmp1, \tmp2
+	add	\size, \kaddr, \size
+	sub	\tmp2, \tmp1, #1
+	bic	\kaddr, \kaddr, \tmp2
+1:	dc	\op, \kaddr
+	add	\kaddr, \kaddr, \tmp1
+	cmp	\kaddr, \size
+	b.lo	1b
+	dsb	\domain

arch/arm64/mm/cache.S ENDPROC(flush_icache_range) · line 109

Quoted 10 lines
arch/arm64/mm/cache.S +5 -0
+	.endm
+
+/*
  *	__flush_dcache_area(kaddr, size)
  *
  *	Ensure that the data held in the page kaddr is written back to the
  *	page in question.
+ *	Flush and invalidate D-cache lines belonging to
+ *	address <kaddr, kaddr+size> till PoC.
  *

Similarly we can replace all 4 lines of description here with:

Ensure that any D-cache lines for the interval [kaddr, kaddr+size) are cleaned and invalidated to the PoC.

Quoted 12 lines
arch/arm64/mm/cache.S +0 -8
  *	- kaddr   - kernel address
  *	- size    - size in question
  */
 ENTRY(__flush_dcache_area)
-	dcache_line_size x2, x3
-	add	x1, x0, x1
-	sub	x3, x2, #1
-	bic	x0, x0, x3
-1:	dc	civac, x0			// clean & invalidate D line / unified line
-	add	x0, x0, x2
-	cmp	x0, x1
-	b.lo	1b

arch/arm64/mm/cache.S ENDPROC(flush_icache_range) · line 122

Quoted 10 lines
arch/arm64/mm/cache.S +5 -1
-	dsb	sy
+	dcache_by_line_op civac, sy, x0, x1, x2, x3
 	ret
 ENDPROC(__flush_dcache_area)
 
 /*
+ *	__clean_dcache_area_pou(kaddr, size)
+ *
+ * 	Flush D-cache lines belonging to address
+ * 	<kaddr, kaddr+size> till PoU.

Similarly here:

Ensure that any D-cache lines for the interval [kaddr, kaddr+size) are cleaned to the PoU.

Quoted 32 lines
arch/arm64/mm/cache.S +10 -0
+ *
+ *	- kaddr   - kernel address
+ *	- size    - size in question
+ */
+ENTRY(__clean_dcache_area_pou)
+	dcache_by_line_op cvau, ish, x0, x1, x2, x3
+	ret
+ENDPROC(__clean_dcache_area_pou)
+
+/*
  *	__inval_cache_range(start, end)
  *	- start   - start address of region
  *	- end     - end address of region
arch/arm64/mm/flush.c +1 -3
index c26b804..4d4d15e 100644
@@ -41,7 +41,7 @@ static void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
 	if (vma->vm_flags & VM_EXEC) {
 		unsigned long addr = (unsigned long)kaddr;
 		if (icache_is_aliasing()) {
-			__flush_dcache_area(kaddr, len);
+			__clean_dcache_area_pou(kaddr, len);
 			__flush_icache_all();
 		} else {
 			flush_icache_range(addr, addr + len);
@@ -75,9 +75,15 @@ void __sync_icache_dcache(pte_t pte, unsigned long addr)
 		return;
 
 	if (!test_and_set_bit(PG_dcache_clean, &page->flags)) {
-		__flush_dcache_area(page_address(page),
-				PAGE_SIZE << compound_order(page));

arch/arm64/mm/flush.c void __sync_icache_dcache(pte_t pte, unsigned long addr) · line 86

Quoted 10 lines
arch/arm64/mm/flush.c +9 -1
-		__flush_icache_all();
+		if (icache_is_aliasing()) {
+			__clean_dcache_area_pou(page_address(page),
+					    PAGE_SIZE << compound_order(page));
+			__flush_icache_all();
+		} else {
+			flush_icache_range((unsigned long)page_address(page),
+					   (unsigned long)page_address(page) +
+					   (PAGE_SIZE << compound_order(page)));
+		}

This now looks identical to the guts of flush_ptrace_access.

Let's extract that into a new helper to call in both cases, e.g.

	void sync_icache_aliases(void *kaddr, unsigned long len)
	{
		unsigned long addr = (unsigned long)kaddr;
		if (icache_is_aliasing()) {
			__flush_dcache_area(kaddr, len);
			__flush_icache_all();
		} else {
			flush_icache_range(addr, addr + len);
		}
	}

That would make this call site a lot simpler:

sync_icache_aliases(page_address(page),
PAGE_SIZE << compound_order(page))

Other than that, this looks good to me.

With the changes suggested above:

Thanks,
Mark.