[PATCH 0/3 v3] xen: arm: smp & tlb cleanups

[PATCH 0/3 v3] xen: arm: smp & tlb cleanups

[Ian Campbell]

A bit of a grab bag, most of this has been posted individually before
either during the 4.4 freeze or sooner.

changes since v2:
        put _local at end of function names in the first patch, and fix
        a missing subst.
        
        drop the original 3rd patch

[PATCH 1/3] xen: arm: clarify naming of the Xen TLB flushing functions

[Ian Campbell]

All of the flush_xen_*_tlb functions operate on the local processor only. Add
_local to the name and update the comments to clarify.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: flush_xen_data_tlb_local_range_va => flush_xen_data_tlb_range_va_local
    fixed missing subst in setup_pagetables
---
 xen/arch/arm/mm.c                |   24 ++++++++++++------------
 xen/include/asm-arm/arm32/page.h |   21 +++++++++++++--------
 xen/include/asm-arm/arm64/page.h |   20 ++++++++++++--------
 3 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 3161d79..d523f77 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -215,7 +215,7 @@ void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes)
     pte.pt.table = 1; /* 4k mappings always have this bit set */
     pte.pt.xn = 1;
     write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
-    flush_xen_data_tlb_range_va(FIXMAP_ADDR(map), PAGE_SIZE);
+    flush_xen_data_tlb_range_va_local(FIXMAP_ADDR(map), PAGE_SIZE);
 }
 
 /* Remove a mapping from a fixmap entry */
@@ -223,7 +223,7 @@ void clear_fixmap(unsigned map)
 {
     lpae_t pte = {0};
     write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
-    flush_xen_data_tlb_range_va(FIXMAP_ADDR(map), PAGE_SIZE);
+    flush_xen_data_tlb_range_va_local(FIXMAP_ADDR(map), PAGE_SIZE);
 }
 
 #ifdef CONFIG_DOMAIN_PAGE
@@ -301,7 +301,7 @@ void *map_domain_page(unsigned long mfn)
      * We may not have flushed this specific subpage at map time,
      * since we only flush the 4k page not the superpage
      */
-    flush_xen_data_tlb_range_va(va, PAGE_SIZE);
+    flush_xen_data_tlb_range_va_local(va, PAGE_SIZE);
 
     return (void *)va;
 }
@@ -403,7 +403,7 @@ void __init remove_early_mappings(void)
 {
     lpae_t pte = {0};
     write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START), pte);
-    flush_xen_data_tlb_range_va(BOOT_FDT_VIRT_START, SECOND_SIZE);
+    flush_xen_data_tlb_range_va_local(BOOT_FDT_VIRT_START, SECOND_SIZE);
 }
 
 extern void relocate_xen(uint64_t ttbr, void *src, void *dst, size_t len);
@@ -421,7 +421,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
     dest_va = BOOT_RELOC_VIRT_START;
     pte = mfn_to_xen_entry(xen_paddr >> PAGE_SHIFT, WRITEALLOC);
     write_pte(xen_second + second_table_offset(dest_va), pte);
-    flush_xen_data_tlb_range_va(dest_va, SECOND_SIZE);
+    flush_xen_data_tlb_range_va_local(dest_va, SECOND_SIZE);
 
     /* Calculate virt-to-phys offset for the new location */
     phys_offset = xen_paddr - (unsigned long) _start;
@@ -473,7 +473,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
     dest_va = BOOT_RELOC_VIRT_START;
     pte = mfn_to_xen_entry(xen_paddr >> PAGE_SHIFT, WRITEALLOC);
     write_pte(boot_second + second_table_offset(dest_va), pte);
-    flush_xen_data_tlb_range_va(dest_va, SECOND_SIZE);
+    flush_xen_data_tlb_range_va_local(dest_va, SECOND_SIZE);
 #ifdef CONFIG_ARM_64
     ttbr = (uintptr_t) xen_pgtable + phys_offset;
 #else
@@ -521,7 +521,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
     /* From now on, no mapping may be both writable and executable. */
     WRITE_SYSREG32(READ_SYSREG32(SCTLR_EL2) | SCTLR_WXN, SCTLR_EL2);
     /* Flush everything after setting WXN bit. */
-    flush_xen_text_tlb();
+    flush_xen_text_tlb_local();
 
 #ifdef CONFIG_ARM_32
     per_cpu(xen_pgtable, 0) = cpu0_pgtable;
@@ -594,7 +594,7 @@ void __cpuinit mmu_init_secondary_cpu(void)
 {
     /* From now on, no mapping may be both writable and executable. */
     WRITE_SYSREG32(READ_SYSREG32(SCTLR_EL2) | SCTLR_WXN, SCTLR_EL2);
-    flush_xen_text_tlb();
+    flush_xen_text_tlb_local();
 }
 
 /* Create Xen's mappings of memory.
@@ -622,7 +622,7 @@ static void __init create_32mb_mappings(lpae_t *second,
         write_pte(p + i, pte);
         pte.pt.base += 1 << LPAE_SHIFT;
     }
-    flush_xen_data_tlb();
+    flush_xen_data_tlb_local();
 }
 
 #ifdef CONFIG_ARM_32
@@ -701,7 +701,7 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
         vaddr += FIRST_SIZE;
     }
 
-    flush_xen_data_tlb();
+    flush_xen_data_tlb_local();
 }
 #endif
 
@@ -845,7 +845,7 @@ static int create_xen_entries(enum xenmap_operation op,
                 BUG();
         }
     }
-    flush_xen_data_tlb_range_va(virt, PAGE_SIZE * nr_mfns);
+    flush_xen_data_tlb_range_va_local(virt, PAGE_SIZE * nr_mfns);
 
     rc = 0;
 
@@ -908,7 +908,7 @@ static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg)
         }
         write_pte(xen_xenmap + i, pte);
     }
-    flush_xen_text_tlb();
+    flush_xen_text_tlb_local();
 }
 
 /* Release all __init and __initdata ranges to be reused */
diff --git a/xen/include/asm-arm/arm32/page.h b/xen/include/asm-arm/arm32/page.h
index 191a108..b0a2025 100644
--- a/xen/include/asm-arm/arm32/page.h
+++ b/xen/include/asm-arm/arm32/page.h
@@ -27,13 +27,15 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
 #define __clean_and_invalidate_xen_dcache_one(R) STORE_CP32(R, DCCIMVAC)
 
 /*
- * Flush all hypervisor mappings from the TLB and branch predictor.
+ * Flush all hypervisor mappings from the TLB and branch predictor of
+ * the local processor.
+ *
  * This is needed after changing Xen code mappings.
  *
  * The caller needs to issue the necessary DSB and D-cache flushes
  * before calling flush_xen_text_tlb.
  */
-static inline void flush_xen_text_tlb(void)
+static inline void flush_xen_text_tlb_local(void)
 {
     register unsigned long r0 asm ("r0");
     asm volatile (
@@ -47,10 +49,11 @@ static inline void flush_xen_text_tlb(void)
 }
 
 /*
- * Flush all hypervisor mappings from the data TLB. This is not
- * sufficient when changing code mappings or for self modifying code.
+ * Flush all hypervisor mappings from the data TLB of the local
+ * processor. This is not sufficient when changing code mappings or
+ * for self modifying code.
  */
-static inline void flush_xen_data_tlb(void)
+static inline void flush_xen_data_tlb_local(void)
 {
     register unsigned long r0 asm ("r0");
     asm volatile("dsb;" /* Ensure preceding are visible */
@@ -61,10 +64,12 @@ static inline void flush_xen_data_tlb(void)
 }
 
 /*
- * Flush a range of VA's hypervisor mappings from the data TLB. This is not
- * sufficient when changing code mappings or for self modifying code.
+ * Flush a range of VA's hypervisor mappings from the data TLB of the
+ * local processor. This is not sufficient when changing code mappings
+ * or for self modifying code.
  */
-static inline void flush_xen_data_tlb_range_va(unsigned long va, unsigned long size)
+static inline void flush_xen_data_tlb_range_va_local(unsigned long va,
+                                                     unsigned long size)
 {
     unsigned long end = va + size;
     dsb(sy); /* Ensure preceding are visible */
diff --git a/xen/include/asm-arm/arm64/page.h b/xen/include/asm-arm/arm64/page.h
index 20b4c5a..65332a3 100644
--- a/xen/include/asm-arm/arm64/page.h
+++ b/xen/include/asm-arm/arm64/page.h
@@ -22,13 +22,14 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
 #define __clean_and_invalidate_xen_dcache_one(R) "dc  civac, %" #R ";"
 
 /*
- * Flush all hypervisor mappings from the TLB
+ * Flush all hypervisor mappings from the TLB of the local processor.
+ *
  * This is needed after changing Xen code mappings.
  *
  * The caller needs to issue the necessary DSB and D-cache flushes
  * before calling flush_xen_text_tlb.
  */
-static inline void flush_xen_text_tlb(void)
+static inline void flush_xen_text_tlb_local(void)
 {
     asm volatile (
         "isb;"       /* Ensure synchronization with previous changes to text */
@@ -40,10 +41,11 @@ static inline void flush_xen_text_tlb(void)
 }
 
 /*
- * Flush all hypervisor mappings from the data TLB. This is not
- * sufficient when changing code mappings or for self modifying code.
+ * Flush all hypervisor mappings from the data TLB of the local
+ * processor. This is not sufficient when changing code mappings or
+ * for self modifying code.
  */
-static inline void flush_xen_data_tlb(void)
+static inline void flush_xen_data_tlb_local(void)
 {
     asm volatile (
         "dsb    sy;"                    /* Ensure visibility of PTE writes */
@@ -54,10 +56,12 @@ static inline void flush_xen_data_tlb(void)
 }
 
 /*
- * Flush a range of VA's hypervisor mappings from the data TLB. This is not
- * sufficient when changing code mappings or for self modifying code.
+ * Flush a range of VA's hypervisor mappings from the data TLB of the
+ * local processor. This is not sufficient when changing code mappings
+ * or for self modifying code.
  */
-static inline void flush_xen_data_tlb_range_va(unsigned long va, unsigned long size)
+static inline void flush_xen_data_tlb_range_va_local(unsigned long va,
+                                                     unsigned long size)
 {
     unsigned long end = va + size;
     dsb(sy); /* Ensure preceding are visible */
-- 
1.7.10.4

[PATCH 2/3] xen: arm: flush TLB on all CPUs when setting or clearing fixmaps

[Ian Campbell]

These mappings are global and therefore need flushing on all processors. Add
flush_all_xen_data_tlb_range_va which accomplishes this.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v3: use dsb(sy) not dsb()
---
 xen/arch/arm/mm.c                |    4 ++--
 xen/include/asm-arm/arm32/page.h |   19 +++++++++++++++++++
 xen/include/asm-arm/arm64/page.h |   19 +++++++++++++++++++
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index d523f77..b966a5c 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -215,7 +215,7 @@ void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes)
     pte.pt.table = 1; /* 4k mappings always have this bit set */
     pte.pt.xn = 1;
     write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
-    flush_xen_data_tlb_range_va_local(FIXMAP_ADDR(map), PAGE_SIZE);
+    flush_xen_data_tlb_range_va(FIXMAP_ADDR(map), PAGE_SIZE);
 }
 
 /* Remove a mapping from a fixmap entry */
@@ -223,7 +223,7 @@ void clear_fixmap(unsigned map)
 {
     lpae_t pte = {0};
     write_pte(xen_fixmap + third_table_offset(FIXMAP_ADDR(map)), pte);
-    flush_xen_data_tlb_range_va_local(FIXMAP_ADDR(map), PAGE_SIZE);
+    flush_xen_data_tlb_range_va(FIXMAP_ADDR(map), PAGE_SIZE);
 }
 
 #ifdef CONFIG_DOMAIN_PAGE
diff --git a/xen/include/asm-arm/arm32/page.h b/xen/include/asm-arm/arm32/page.h
index b0a2025..2b2bbe6 100644
--- a/xen/include/asm-arm/arm32/page.h
+++ b/xen/include/asm-arm/arm32/page.h
@@ -82,6 +82,25 @@ static inline void flush_xen_data_tlb_range_va_local(unsigned long va,
     isb();
 }
 
+/*
+ * Flush a range of VA's hypervisor mappings from the data TLB on all
+ * processors in the inner-shareable domain. This is not sufficient
+ * when changing code mappings or for self modifying code.
+ */
+static inline void flush_xen_data_tlb_range_va(unsigned long va,
+                                               unsigned long size)
+{
+    unsigned long end = va + size;
+    dsb(sy); /* Ensure preceding are visible */
+    while ( va < end ) {
+        asm volatile(STORE_CP32(0, TLBIMVAHIS)
+                     : : "r" (va) : "memory");
+        va += PAGE_SIZE;
+    }
+    dsb(sy); /* Ensure completion of the TLB flush */
+    isb();
+}
+
 /* Ask the MMU to translate a VA for us */
 static inline uint64_t __va_to_par(vaddr_t va)
 {
diff --git a/xen/include/asm-arm/arm64/page.h b/xen/include/asm-arm/arm64/page.h
index 65332a3..fdc652c 100644
--- a/xen/include/asm-arm/arm64/page.h
+++ b/xen/include/asm-arm/arm64/page.h
@@ -74,6 +74,25 @@ static inline void flush_xen_data_tlb_range_va_local(unsigned long va,
     isb();
 }
 
+/*
+ * Flush a range of VA's hypervisor mappings from the data TLB of all
+ * processors in the inner-shareable domain. This is not sufficient
+ * when changing code mappings or for self modifying code.
+ */
+static inline void flush_xen_data_tlb_range_va(unsigned long va,
+                                               unsigned long size)
+{
+    unsigned long end = va + size;
+    dsb(sy); /* Ensure preceding are visible */
+    while ( va < end ) {
+        asm volatile("tlbi vae2is, %0;"
+                     : : "r" (va>>PAGE_SHIFT) : "memory");
+        va += PAGE_SIZE;
+    }
+    dsb(sy); /* Ensure completion of the TLB flush */
+    isb();
+}
+
 /* Ask the MMU to translate a VA for us */
 static inline uint64_t __va_to_par(vaddr_t va)
 {
-- 
1.7.10.4

[PATCH 3/3] xen: arm32: don't force the compiler to allocate a dummy register

[Ian Campbell]

TLBIALLH, ICIALLU and BPIALL make no use of their register argument. Instead
of making the compiler allocate a dummy register just hardcode r0, there is no
need to represent this in the inline asm since the register is neither
clobbered nor used in any way.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
 xen/include/asm-arm/arm32/page.h      |   14 ++++++--------
 xen/include/asm-arm/arm32/processor.h |    4 ++++
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/xen/include/asm-arm/arm32/page.h b/xen/include/asm-arm/arm32/page.h
index 2b2bbe6..1a49d59 100644
--- a/xen/include/asm-arm/arm32/page.h
+++ b/xen/include/asm-arm/arm32/page.h
@@ -37,15 +37,14 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
  */
 static inline void flush_xen_text_tlb_local(void)
 {
-    register unsigned long r0 asm ("r0");
     asm volatile (
         "isb;"                        /* Ensure synchronization with previous changes to text */
-        STORE_CP32(0, TLBIALLH)       /* Flush hypervisor TLB */
-        STORE_CP32(0, ICIALLU)        /* Flush I-cache */
-        STORE_CP32(0, BPIALL)         /* Flush branch predictor */
+        CMD_CP32(TLBIALLH)            /* Flush hypervisor TLB */
+        CMD_CP32(ICIALLU)             /* Flush I-cache */
+        CMD_CP32(BPIALL)              /* Flush branch predictor */
         "dsb;"                        /* Ensure completion of TLB+BP flush */
         "isb;"
-        : : "r" (r0) /*dummy*/ : "memory");
+        : : : "memory");
 }
 
 /*
@@ -55,12 +54,11 @@ static inline void flush_xen_text_tlb_local(void)
  */
 static inline void flush_xen_data_tlb_local(void)
 {
-    register unsigned long r0 asm ("r0");
     asm volatile("dsb;" /* Ensure preceding are visible */
-                 STORE_CP32(0, TLBIALLH)
+                 CMD_CP32(TLBIALLH)
                  "dsb;" /* Ensure completion of the TLB flush */
                  "isb;"
-                 : : "r" (r0) /* dummy */: "memory");
+                 : : : "memory");
 }
 
 /*
diff --git a/xen/include/asm-arm/arm32/processor.h b/xen/include/asm-arm/arm32/processor.h
index 8a35cee..f41644d 100644
--- a/xen/include/asm-arm/arm32/processor.h
+++ b/xen/include/asm-arm/arm32/processor.h
@@ -69,6 +69,10 @@ struct cpu_user_regs
 #define LOAD_CP64(r, name...)  "mrrc " __stringify(CP64(%r, %H##r, name)) ";"
 #define STORE_CP64(r, name...) "mcrr " __stringify(CP64(%r, %H##r, name)) ";"
 
+/* Issue a CP operation which takes no argument,
+ * uses r0 as a placeholder register. */
+#define CMD_CP32(name...)      "mcr " __stringify(CP32(r0, name)) ";"
+
 #ifndef __ASSEMBLY__
 
 /* C wrappers */
-- 
1.7.10.4

Re: [PATCH 1/3] xen: arm: clarify naming of the Xen TLB flushing functions

[Julien Grall]

On 04/02/2014 03:12 PM, Ian Campbell wrote:
> All of the flush_xen_*_tlb functions operate on the local processor only. Add
> _local to the name and update the comments to clarify.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>

-- 
Julien Grall

Re: [PATCH 2/3] xen: arm: flush TLB on all CPUs when setting or clearing fixmaps

[Julien Grall]

On 04/02/2014 03:12 PM, Ian Campbell wrote:
> These mappings are global and therefore need flushing on all processors. Add
> flush_all_xen_data_tlb_range_va which accomplishes this.

I think remove_early_mappings should also use flush_xen_data_range_va.

I'm wondering why BOOT_FDT_VIRT_START is removed so late (i.e in
discard_initial_modules). It can be done once the DTB is copied in setup_mm.

[..]

>  
> +/*
> + * Flush a range of VA's hypervisor mappings from the data TLB on all
> + * processors in the inner-shareable domain. This is not sufficient
> + * when changing code mappings or for self modifying code.
> + */
> +static inline void flush_xen_data_tlb_range_va(unsigned long va,
> +                                               unsigned long size)
> +{
> +    unsigned long end = va + size;
> +    dsb(sy); /* Ensure preceding are visible */
> +    while ( va < end ) {
> +        asm volatile(STORE_CP32(0, TLBIMVAHIS)
> +                     : : "r" (va) : "memory");
> +        va += PAGE_SIZE;
> +    }
> +    dsb(sy); /* Ensure completion of the TLB flush */
> +    isb();
> +}
> +

This loop is exactly the same on arm64 (except the TLBIMVAHIS), is it
possible to have a common code like clean_xen_dcache_va_range?

Regards,

-- 
Julien Grall

Re: [PATCH 2/3] xen: arm: flush TLB on all CPUs when setting or clearing fixmaps

[Ian Campbell]

On Wed, 2014-04-02 at 15:46 +0100, Julien Grall wrote:
> On 04/02/2014 03:12 PM, Ian Campbell wrote:
> > These mappings are global and therefore need flushing on all processors. Add
> > flush_all_xen_data_tlb_range_va which accomplishes this.
> 
> I think remove_early_mappings should also use flush_xen_data_range_va.

Probably. I'll make that change too.

> I'm wondering why BOOT_FDT_VIRT_START is removed so late (i.e in
> discard_initial_modules). It can be done once the DTB is copied in setup_mm.

I'm not sure, I think it just seemed like a convenient place to do it.
Feel free to move it earlier.

> > +/*
> > + * Flush a range of VA's hypervisor mappings from the data TLB on all
> > + * processors in the inner-shareable domain. This is not sufficient
> > + * when changing code mappings or for self modifying code.
> > + */
> > +static inline void flush_xen_data_tlb_range_va(unsigned long va,
> > +                                               unsigned long size)
> > +{
> > +    unsigned long end = va + size;
> > +    dsb(sy); /* Ensure preceding are visible */
> > +    while ( va < end ) {
> > +        asm volatile(STORE_CP32(0, TLBIMVAHIS)
> > +                     : : "r" (va) : "memory");
> > +        va += PAGE_SIZE;
> > +    }
> > +    dsb(sy); /* Ensure completion of the TLB flush */
> > +    isb();
> > +}
> > +
> 
> This loop is exactly the same on arm64 (except the TLBIMVAHIS), is it
> possible to have a common code like clean_xen_dcache_va_range?

flush_xen_data_tlb_range_va_local has the same issue. I'll fix them
both.

Ian.

Re: [PATCH 2/3] xen: arm: flush TLB on all CPUs when setting or clearing fixmaps

[Ian Campbell]

On Wed, 2014-04-02 at 16:22 +0100, Ian Campbell wrote:
> > This loop is exactly the same on arm64 (except the TLBIMVAHIS), is it
> > possible to have a common code like clean_xen_dcache_va_range?
> 
> flush_xen_data_tlb_range_va_local has the same issue. I'll fix them
> both.

Actually there is a second difference which I didn't remember which is 
        : : "r" (va) : "memory");
vs:
     : : "r" (va>>PAGE_SHIFT) : "memory");

That's a lot harder to abstract away conveniently. I could have a
#define for the shift amount, I guess. I'll have a play and see if I can
come up with something satisfactory. If not I'll fall back to keeping
the loops per subarch.

Ian.

Re: [PATCH 2/3] xen: arm: flush TLB on all CPUs when setting or clearing fixmaps

[Julien Grall]

On 04/02/2014 05:54 PM, Ian Campbell wrote:
> That's a lot harder to abstract away conveniently. I could have a
> #define for the shift amount, I guess. I'll have a play and see if I can
> come up with something satisfactory. If not I'll fall back to keeping
> the loops per subarch.

Why can't you put the whole assembly inline in a macro?

-- 
Julien Grall