[RFC PATCH 2/2] ARMv7: Invalidate the TLB before freeing page tables

On Tue, Feb 15, 2011 at 02:42:06PM +0000, Catalin Marinas wrote:

On Tue, 2011-02-15 at 12:14 +0000, Russell King - ARM Linux wrote:

quoted

On Tue, Feb 15, 2011 at 11:32:42AM +0000, Russell King - ARM Linux wrote:

quoted

The point of TLB shootdown is that we unmap the entries from the page
tables, then issue the TLB flushes, and then free the pages and page
tables after that.  All that Peter's patch tries to do is to get ARM to
use the generic stuff.

As Peter's patch preserves the current behaviour, that's not sufficient.
So, let's do this our own way and delay pages and page table frees on
ARMv6 and v7.  Untested.

ARMv7 should be enough, I'm not aware of any pre-v7 with this behaviour.

ARM11MPCore.  Any SMP system can access a page which was free'd by the
tlb code but hasn't been flushed from the hardware TLBs.  So maybe we
want it to be "defined(CONFIG_SMP) || defined(CONFIG_CPU_32v7)" ?

quoted

diff --git a/arch/arm/include/asm/tlb.h b/arch/arm/include/asm/tlb.h
index f41a6f5..1ca3e16 100644
--- a/arch/arm/include/asm/tlb.h
+++ b/arch/arm/include/asm/tlb.h

@@ -30,6 +30,16 @@
 #include <asm/pgalloc.h>

 /*
+ * As v6 and v7 speculatively prefetch, which can drag new entries into the
+ * TLB, we need to delay freeing pages and page tables.
+ */
+#if defined(CONFIG_CPU_32v6) || defined(CONFIG_CPU_32v7)
+#define tlb_fast_mode(tlb)     0
+#else
+#define tlb_fast_mode(tlb)     1
+#endif

We could make this v7 only. If you want it to be more dynamic, we can
check the MMFR0[3:0] bits (Cortex-A15 sets them to 4). But
architecturally we should assume that intermediate page table levels may
be cached.

I don't think that a runtime check justifies the optimization.  We're
talking about the difference between storing a set of pages in an array
and freeing them later vs freeing them one at a time.  Doing a test per
page is probably more expensive than just storing them in an array.

quoted

-#define tlb_remove_page(tlb,page)      free_page_and_swap_cache(page)
-#define pte_free_tlb(tlb, ptep, addr)  pte_free((tlb)->mm, ptep)
+#define pte_free_tlb(tlb, ptep, addr)  __pte_free_tlb(tlb, ptep, addr)
 #define pmd_free_tlb(tlb, pmdp, addr)  pmd_free((tlb)->mm, pmdp)

With LPAE, we'll need a __pmd_free_tlb() but I can add this as part of
my patches.

Yes.

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks.