[PATCH v4 0/6 ] xen: arm: smp & tlb cleanups

[PATCH v4 0/6 ] xen: arm: smp & tlb cleanups

[Ian Campbell]

Various SMP and TLB etc related cleanups.

changes since v3:
        New patches relaxing barriers in TLB and cache flush operations
        
        Make flush_all_xen_data_tlb_range_va and
        flush_all_xen_data_tlb_range_va_local mostly common (the latter
        in a new patch)
        
Ian

[PATCH v4 1/6] 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>
Acked-by: Julien Grall <julien.grall@linaro.org>
---
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 v4 2/6] xen: arm: consolidate body of flush_xen_data_tlb_range_va_local

[Ian Campbell]

This is almost identical on both sub architectures.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: New patch
---
 xen/include/asm-arm/arm32/page.h |   19 +++----------------
 xen/include/asm-arm/arm64/page.h |   19 +++----------------
 xen/include/asm-arm/page.h       |   18 ++++++++++++++++++
 3 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/xen/include/asm-arm/arm32/page.h b/xen/include/asm-arm/arm32/page.h
index b0a2025..d839d03 100644
--- a/xen/include/asm-arm/arm32/page.h
+++ b/xen/include/asm-arm/arm32/page.h
@@ -63,23 +63,10 @@ static inline void flush_xen_data_tlb_local(void)
                  : : "r" (r0) /* dummy */: "memory");
 }
 
-/*
- * 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_local(unsigned long va,
-                                                     unsigned long size)
+/* Flush TLB of local processor for address va. */
+static inline void __flush_xen_data_tlb_one_local(vaddr_t va)
 {
-    unsigned long end = va + size;
-    dsb(sy); /* Ensure preceding are visible */
-    while ( va < end ) {
-        asm volatile(STORE_CP32(0, TLBIMVAH)
-                     : : "r" (va) : "memory");
-        va += PAGE_SIZE;
-    }
-    dsb(sy); /* Ensure completion of the TLB flush */
-    isb();
+    asm volatile(STORE_CP32(0, TLBIMVAH) : : "r" (va) : "memory");
 }
 
 /* Ask the MMU to translate a VA for us */
diff --git a/xen/include/asm-arm/arm64/page.h b/xen/include/asm-arm/arm64/page.h
index 65332a3..897d79b 100644
--- a/xen/include/asm-arm/arm64/page.h
+++ b/xen/include/asm-arm/arm64/page.h
@@ -55,23 +55,10 @@ static inline void flush_xen_data_tlb_local(void)
         : : : "memory");
 }
 
-/*
- * 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_local(unsigned long va,
-                                                     unsigned long size)
+/* Flush TLB of local processor for address va. */
+static inline void  __flush_xen_data_tlb_one_local(vaddr_t va)
 {
-    unsigned long end = va + size;
-    dsb(sy); /* Ensure preceding are visible */
-    while ( va < end ) {
-        asm volatile("tlbi vae2, %0;"
-                     : : "r" (va>>PAGE_SHIFT) : "memory");
-        va += PAGE_SIZE;
-    }
-    dsb(sy); /* Ensure completion of the TLB flush */
-    isb();
+    asm volatile("tlbi vae2, %0;" : : "r" (va>>PAGE_SHIFT) : "memory");
 }
 
 /* Ask the MMU to translate a VA for us */
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index d18ec2a..bbecacf 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -306,6 +306,24 @@ static inline void clean_and_invalidate_xen_dcache_va_range
             : : "r" (_p), "m" (*_p));                                   \
 } while (0)
 
+/*
+ * 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_local(unsigned long va,
+                                                     unsigned long size)
+{
+    unsigned long end = va + size;
+    dsb(sy); /* Ensure preceding are visible */
+    while ( va < end ) {
+        __flush_xen_data_tlb_one_local(va);
+        va += PAGE_SIZE;
+    }
+    dsb(sy); /* Ensure completion of the TLB flush */
+    isb();
+}
+
 /* Flush the dcache for an entire page. */
 void flush_page_to_ram(unsigned long mfn);
 
-- 
1.7.10.4

Re: [PATCH v4 2/6] xen: arm: consolidate body of flush_xen_data_tlb_range_va_local

[Julien Grall]

On 04/03/2014 09:59 AM, Ian Campbell wrote:
> +/*
> + * 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_local(unsigned long va,
> +                                                     unsigned long size)
> +{
> +    unsigned long end = va + size;
> +    dsb(sy); /* Ensure preceding are visible */
> +    while ( va < end ) {

while ( va < end )
{

Except this minor coding style error (I've noticed it was there before):

Acked-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

[PATCH v4 3/6] 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.

Likewise when removing the early mappings

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: also make the change in remove_early_mappings
    consolidatw flush_all_xen_data_tlb_range_va implentation
v3: use dsb(sy) not dsb()
---
 xen/arch/arm/mm.c                |    6 +++---
 xen/include/asm-arm/arm32/page.h |    7 +++++++
 xen/include/asm-arm/arm64/page.h |    7 +++++++
 xen/include/asm-arm/page.h       |   18 ++++++++++++++++++
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index d523f77..362bc8d 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
@@ -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_local(BOOT_FDT_VIRT_START, SECOND_SIZE);
+    flush_xen_data_tlb_range_va(BOOT_FDT_VIRT_START, SECOND_SIZE);
 }
 
 extern void relocate_xen(uint64_t ttbr, void *src, void *dst, size_t len);
diff --git a/xen/include/asm-arm/arm32/page.h b/xen/include/asm-arm/arm32/page.h
index d839d03..ead6b97 100644
--- a/xen/include/asm-arm/arm32/page.h
+++ b/xen/include/asm-arm/arm32/page.h
@@ -69,6 +69,13 @@ static inline void __flush_xen_data_tlb_one_local(vaddr_t va)
     asm volatile(STORE_CP32(0, TLBIMVAH) : : "r" (va) : "memory");
 }
 
+/* Flush TLB of all processors in the inner-shareable domain for
+ * address va. */
+static inline void __flush_xen_data_tlb_one(vaddr_t va)
+{
+    asm volatile(STORE_CP32(0, TLBIMVAHIS) : : "r" (va) : "memory");
+}
+
 /* 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 897d79b..d7ee2fc 100644
--- a/xen/include/asm-arm/arm64/page.h
+++ b/xen/include/asm-arm/arm64/page.h
@@ -61,6 +61,13 @@ static inline void  __flush_xen_data_tlb_one_local(vaddr_t va)
     asm volatile("tlbi vae2, %0;" : : "r" (va>>PAGE_SHIFT) : "memory");
 }
 
+/* Flush TLB of all processors in the inner-shareable domain for
+ * address va. */
+static inline void __flush_xen_data_tlb_one(vaddr_t va)
+{
+    asm volatile("tlbi vae2is, %0;" : : "r" (va>>PAGE_SHIFT) : "memory");
+}
+
 /* 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/page.h b/xen/include/asm-arm/page.h
index bbecacf..a8eeb73 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -324,6 +324,24 @@ 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 ) {
+        __flush_xen_data_tlb_one(va);
+        va += PAGE_SIZE;
+    }
+    dsb(sy); /* Ensure completion of the TLB flush */
+    isb();
+}
+
 /* Flush the dcache for an entire page. */
 void flush_page_to_ram(unsigned long mfn);
 
-- 
1.7.10.4

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

[Julien Grall]

On 04/03/2014 09:59 AM, Ian Campbell wrote:
> +/*
> + * 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 ) {

Same error as the previous patch :):
while ( ... )
{

Except that:

Acked-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

[PATCH v4 4/6] 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 ead6b97..3f2bdc9 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");
 }
 
 /* Flush TLB of local processor for address va. */
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

[PATCH v4 5/6] xen: arm: relax barriers in tlb flushes

[Ian Campbell]

Flushing the local TLB only requires a local barrier.

Flushing the TLB of all processors in the inner-shareable domain only requires
an inner-shareable barrier.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: new patch
---
 xen/include/asm-arm/arm32/flushtlb.h |   16 ++++++++--------
 xen/include/asm-arm/arm32/page.h     |   10 +++++-----
 xen/include/asm-arm/arm64/flushtlb.h |   16 ++++++++--------
 xen/include/asm-arm/arm64/page.h     |   10 +++++-----
 xen/include/asm-arm/page.h           |    8 ++++----
 5 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/xen/include/asm-arm/arm32/flushtlb.h b/xen/include/asm-arm/arm32/flushtlb.h
index bbcc82f..621c5d3 100644
--- a/xen/include/asm-arm/arm32/flushtlb.h
+++ b/xen/include/asm-arm/arm32/flushtlb.h
@@ -4,44 +4,44 @@
 /* Flush local TLBs, current VMID only */
 static inline void flush_tlb_local(void)
 {
-    dsb(sy);
+    dsb(nsh);
 
     WRITE_CP32((uint32_t) 0, TLBIALL);
 
-    dsb(sy);
+    dsb(nsh);
     isb();
 }
 
 /* Flush inner shareable TLBs, current VMID only */
 static inline void flush_tlb(void)
 {
-    dsb(sy);
+    dsb(ish);
 
     WRITE_CP32((uint32_t) 0, TLBIALLIS);
 
-    dsb(sy);
+    dsb(ish);
     isb();
 }
 
 /* Flush local TLBs, all VMIDs, non-hypervisor mode */
 static inline void flush_tlb_all_local(void)
 {
-    dsb(sy);
+    dsb(nsh);
 
     WRITE_CP32((uint32_t) 0, TLBIALLNSNH);
 
-    dsb(sy);
+    dsb(nsh);
     isb();
 }
 
 /* Flush innershareable TLBs, all VMIDs, non-hypervisor mode */
 static inline void flush_tlb_all(void)
 {
-    dsb(sy);
+    dsb(ish);
 
     WRITE_CP32((uint32_t) 0, TLBIALLNSNHIS);
 
-    dsb(sy);
+    dsb(ish);
     isb();
 }
 
diff --git a/xen/include/asm-arm/arm32/page.h b/xen/include/asm-arm/arm32/page.h
index 3f2bdc9..0de3395 100644
--- a/xen/include/asm-arm/arm32/page.h
+++ b/xen/include/asm-arm/arm32/page.h
@@ -12,10 +12,10 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
 {
     asm volatile (
         /* Ensure any writes have completed with the old mappings. */
-        "dsb;"
+        "dsb ish;"
         /* Safely write the entry (STRD is atomic on CPUs that support LPAE) */
         "strd %0, %H0, [%1];"
-        "dsb;"
+        "dsb ish;"
         : : "r" (pte.bits), "r" (p) : "memory");
 }
 
@@ -42,7 +42,7 @@ static inline void flush_xen_text_tlb_local(void)
         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 */
+        "dsb nsh;"                    /* Ensure completion of TLB+BP flush */
         "isb;"
         : : : "memory");
 }
@@ -54,9 +54,9 @@ static inline void flush_xen_text_tlb_local(void)
  */
 static inline void flush_xen_data_tlb_local(void)
 {
-    asm volatile("dsb;" /* Ensure preceding are visible */
+    asm volatile("dsb nsh;" /* Ensure preceding are visible */
                  CMD_CP32(TLBIALLH)
-                 "dsb;" /* Ensure completion of the TLB flush */
+                 "dsb nsh;" /* Ensure completion of the TLB flush */
                  "isb;"
                  : : : "memory");
 }
diff --git a/xen/include/asm-arm/arm64/flushtlb.h b/xen/include/asm-arm/arm64/flushtlb.h
index a73df92..ba7059a 100644
--- a/xen/include/asm-arm/arm64/flushtlb.h
+++ b/xen/include/asm-arm/arm64/flushtlb.h
@@ -5,9 +5,9 @@
 static inline void flush_tlb_local(void)
 {
     asm volatile(
-        "dsb sy;"
+        "dsb nsh;"
         "tlbi vmalle1;"
-        "dsb sy;"
+        "dsb nsh;"
         "isb;"
         : : : "memory");
 }
@@ -16,9 +16,9 @@ static inline void flush_tlb_local(void)
 static inline void flush_tlb(void)
 {
     asm volatile(
-        "dsb sy;"
+        "dsb ish;"
         "tlbi vmalle1is;"
-        "dsb sy;"
+        "dsb ish;"
         "isb;"
         : : : "memory");
 }
@@ -27,9 +27,9 @@ static inline void flush_tlb(void)
 static inline void flush_tlb_all_local(void)
 {
     asm volatile(
-        "dsb sy;"
+        "dsb nsh;"
         "tlbi alle1;"
-        "dsb sy;"
+        "dsb nsh;"
         "isb;"
         : : : "memory");
 }
@@ -38,9 +38,9 @@ static inline void flush_tlb_all_local(void)
 static inline void flush_tlb_all(void)
 {
     asm volatile(
-        "dsb sy;"
+        "dsb ish;"
         "tlbi alle1is;"
-        "dsb sy;"
+        "dsb ish;"
         "isb;"
         : : : "memory");
 }
diff --git a/xen/include/asm-arm/arm64/page.h b/xen/include/asm-arm/arm64/page.h
index d7ee2fc..c680f47 100644
--- a/xen/include/asm-arm/arm64/page.h
+++ b/xen/include/asm-arm/arm64/page.h
@@ -8,9 +8,9 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
 {
     asm volatile (
         /* Ensure any writes have completed with the old mappings. */
-        "dsb sy;"
+        "dsb ish;"
         "str %0, [%1];"         /* Write the entry */
-        "dsb sy;"
+        "dsb ish;"
         : : "r" (pte.bits), "r" (p) : "memory");
 }
 
@@ -35,7 +35,7 @@ static inline void flush_xen_text_tlb_local(void)
         "isb;"       /* Ensure synchronization with previous changes to text */
         "tlbi   alle2;"                 /* Flush hypervisor TLB */
         "ic     iallu;"                 /* Flush I-cache */
-        "dsb    sy;"                    /* Ensure completion of TLB flush */
+        "dsb    nsh;"                   /* Ensure completion of TLB flush */
         "isb;"
         : : : "memory");
 }
@@ -48,9 +48,9 @@ static inline void flush_xen_text_tlb_local(void)
 static inline void flush_xen_data_tlb_local(void)
 {
     asm volatile (
-        "dsb    sy;"                    /* Ensure visibility of PTE writes */
+        "dsb    nsh;"                   /* Ensure visibility of PTE writes */
         "tlbi   alle2;"                 /* Flush hypervisor TLB */
-        "dsb    sy;"                    /* Ensure completion of TLB flush */
+        "dsb    nsh;"                   /* Ensure completion of TLB flush */
         "isb;"
         : : : "memory");
 }
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index a8eeb73..a96e40b 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -315,12 +315,12 @@ 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 */
+    dsb(nsh); /* Ensure preceding are visible */
     while ( va < end ) {
         __flush_xen_data_tlb_one_local(va);
         va += PAGE_SIZE;
     }
-    dsb(sy); /* Ensure completion of the TLB flush */
+    dsb(nsh); /* Ensure completion of the TLB flush */
     isb();
 }
 
@@ -333,12 +333,12 @@ 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 */
+    dsb(ish); /* Ensure preceding are visible */
     while ( va < end ) {
         __flush_xen_data_tlb_one(va);
         va += PAGE_SIZE;
     }
-    dsb(sy); /* Ensure completion of the TLB flush */
+    dsb(ish); /* Ensure completion of the TLB flush */
     isb();
 }
 
-- 
1.7.10.4

Re: [PATCH v4 5/6] xen: arm: relax barriers in tlb flushes

[Julien Grall]

On 04/03/2014 09:59 AM, Ian Campbell wrote:
> @@ -333,12 +333,12 @@ 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 */
> +    dsb(ish); /* Ensure preceding are visible */

I'm a bit lost with ish/nsh/sy/... shall we keep sy here?
flush_xen_data_tlb is used in iounmap and we want to make sure that
every write as been done just before.

Regards,

-- 
Julien Grall

Re: [PATCH v4 5/6] xen: arm: relax barriers in tlb flushes

[Ian Campbell]

On Thu, 2014-04-03 at 12:12 +0100, Julien Grall wrote:
> On 04/03/2014 09:59 AM, Ian Campbell wrote:
> > @@ -333,12 +333,12 @@ 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 */
> > +    dsb(ish); /* Ensure preceding are visible */
> 
> I'm a bit lost with ish/nsh/sy/... shall we keep sy here?
> flush_xen_data_tlb is used in iounmap

Is it? I can't see it. I do see it in clear_fixmap though.

> and we want to make sure that
> every write as been done just before.

The barrier here is to ensure that any writes to the page tables
themselves are complete, not really to ensure that writes using those
page tables are complete.

If users of this call have additional requirements to make sure other
writes complete (which iounmap surely does) then I think they need to
have their own barriers (or further punt this up to their callers).

Anyway, I should certainly hold off on this change until we are sure
that the appropriate barriers are in place in other places which are
current relying on this barrier being too strong. It seems like
clear_fixmap is one such place, as might vunmap or iounmap (which uses
vunmap, I need to think more carefully about which one needs the
barrier).

Incidentally, I think we can actually go one further an use an ishst
(store only) barrier for the dsb before the tlb flush since we only care
about PTE writes not reads. THat is something I might look at later.

Ian.

Re: [PATCH v4 5/6] xen: arm: relax barriers in tlb flushes

[Julien Grall]

On 04/03/2014 01:18 PM, Ian Campbell wrote:
> On Thu, 2014-04-03 at 12:12 +0100, Julien Grall wrote:
>> On 04/03/2014 09:59 AM, Ian Campbell wrote:
>>> @@ -333,12 +333,12 @@ 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 */
>>> +    dsb(ish); /* Ensure preceding are visible */
>>
>> I'm a bit lost with ish/nsh/sy/... shall we keep sy here?
>> flush_xen_data_tlb is used in iounmap
> 
> Is it? I can't see it. I do see it in clear_fixmap though.

Sorry I though it was used by create_xen_entries... that made me think,
create_xen_entries should use flush_xen_data_tlb_range_va as the mapping
is common with the other CPUs.

>> and we want to make sure that
>> every write as been done just before.
> 
> The barrier here is to ensure that any writes to the page tables
> themselves are complete, not really to ensure that writes using those
> page tables are complete.
> 
> If users of this call have additional requirements to make sure other
> writes complete (which iounmap surely does) then I think they need to
> have their own barriers (or further punt this up to their callers).

Right, after looking to the code, write_pte has a dsb.

-- 
Julien Grall

[PATCH v4 6/6] xen: arm: relax barriers when flushing caches

[Ian Campbell]

We only need an inner shareable barrier here.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v4: new patch
---
 xen/include/asm-arm/page.h |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index a96e40b..b4d5597 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -263,20 +263,20 @@ extern size_t cacheline_bytes;
 static inline void clean_xen_dcache_va_range(void *p, unsigned long size)
 {
     void *end;
-    dsb(sy);           /* So the CPU issues all writes to the range */
+    dsb(ish);          /* So the CPU issues all writes to the range */
     for ( end = p + size; p < end; p += cacheline_bytes )
         asm volatile (__clean_xen_dcache_one(0) : : "r" (p));
-    dsb(sy);           /* So we know the flushes happen before continuing */
+    dsb(ish);          /* So we know the flushes happen before continuing */
 }
 
 static inline void clean_and_invalidate_xen_dcache_va_range
     (void *p, unsigned long size)
 {
     void *end;
-    dsb(sy);         /* So the CPU issues all writes to the range */
+    dsb(ish);        /* So the CPU issues all writes to the range */
     for ( end = p + size; p < end; p += cacheline_bytes )
         asm volatile (__clean_and_invalidate_xen_dcache_one(0) : : "r" (p));
-    dsb(sy);         /* So we know the flushes happen before continuing */
+    dsb(ish);        /* So we know the flushes happen before continuing */
 }
 
 /* Macros for flushing a single small item.  The predicate is always
@@ -288,9 +288,9 @@ static inline void clean_and_invalidate_xen_dcache_va_range
         clean_xen_dcache_va_range(_p, sizeof(x));                       \
     else                                                                \
         asm volatile (                                                  \
-            "dsb sy;"   /* Finish all earlier writes */                 \
+            "dsb ish;"  /* Finish all earlier writes */                 \
             __clean_xen_dcache_one(0)                                   \
-            "dsb sy;"   /* Finish flush before continuing */            \
+            "dsb ish;"  /* Finish flush before continuing */            \
             : : "r" (_p), "m" (*_p));                                   \
 } while (0)
 
@@ -300,9 +300,9 @@ static inline void clean_and_invalidate_xen_dcache_va_range
         clean_and_invalidate_xen_dcache_va_range(_p, sizeof(x));        \
     else                                                                \
         asm volatile (                                                  \
-            "dsb sy;"   /* Finish all earlier writes */                 \
+            "dsb ish;"   /* Finish all earlier writes */                \
             __clean_and_invalidate_xen_dcache_one(0)                    \
-            "dsb sy;"   /* Finish flush before continuing */            \
+            "dsb ish;"   /* Finish flush before continuing */           \
             : : "r" (_p), "m" (*_p));                                   \
 } while (0)
 
-- 
1.7.10.4

Re: [PATCH v4 6/6] xen: arm: relax barriers when flushing caches

[Tim Deegan]

At 09:59 +0100 on 03 Apr (1396515585), Ian Campbell wrote:
> We only need an inner shareable barrier here.

Following the logic we had earlier, do the earlier dsbs here only need
to be store barriers?

Tim.

> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> v4: new patch
> ---
>  xen/include/asm-arm/page.h |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index a96e40b..b4d5597 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -263,20 +263,20 @@ extern size_t cacheline_bytes;
>  static inline void clean_xen_dcache_va_range(void *p, unsigned long size)
>  {
>      void *end;
> -    dsb(sy);           /* So the CPU issues all writes to the range */
> +    dsb(ish);          /* So the CPU issues all writes to the range */
>      for ( end = p + size; p < end; p += cacheline_bytes )
>          asm volatile (__clean_xen_dcache_one(0) : : "r" (p));
> -    dsb(sy);           /* So we know the flushes happen before continuing */
> +    dsb(ish);          /* So we know the flushes happen before continuing */
>  }
>  
>  static inline void clean_and_invalidate_xen_dcache_va_range
>      (void *p, unsigned long size)
>  {
>      void *end;
> -    dsb(sy);         /* So the CPU issues all writes to the range */
> +    dsb(ish);        /* So the CPU issues all writes to the range */
>      for ( end = p + size; p < end; p += cacheline_bytes )
>          asm volatile (__clean_and_invalidate_xen_dcache_one(0) : : "r" (p));
> -    dsb(sy);         /* So we know the flushes happen before continuing */
> +    dsb(ish);        /* So we know the flushes happen before continuing */
>  }
>  
>  /* Macros for flushing a single small item.  The predicate is always
> @@ -288,9 +288,9 @@ static inline void clean_and_invalidate_xen_dcache_va_range
>          clean_xen_dcache_va_range(_p, sizeof(x));                       \
>      else                                                                \
>          asm volatile (                                                  \
> -            "dsb sy;"   /* Finish all earlier writes */                 \
> +            "dsb ish;"  /* Finish all earlier writes */                 \
>              __clean_xen_dcache_one(0)                                   \
> -            "dsb sy;"   /* Finish flush before continuing */            \
> +            "dsb ish;"  /* Finish flush before continuing */            \
>              : : "r" (_p), "m" (*_p));                                   \
>  } while (0)
>  
> @@ -300,9 +300,9 @@ static inline void clean_and_invalidate_xen_dcache_va_range
>          clean_and_invalidate_xen_dcache_va_range(_p, sizeof(x));        \
>      else                                                                \
>          asm volatile (                                                  \
> -            "dsb sy;"   /* Finish all earlier writes */                 \
> +            "dsb ish;"   /* Finish all earlier writes */                \
>              __clean_and_invalidate_xen_dcache_one(0)                    \
> -            "dsb sy;"   /* Finish flush before continuing */            \
> +            "dsb ish;"   /* Finish flush before continuing */           \
>              : : "r" (_p), "m" (*_p));                                   \
>  } while (0)
>  
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v4 6/6] xen: arm: relax barriers when flushing caches

[Ian Campbell]

On Thu, 2014-04-03 at 14:55 +0200, Tim Deegan wrote:
> At 09:59 +0100 on 03 Apr (1396515585), Ian Campbell wrote:
> > We only need an inner shareable barrier here.
> 
> Following the logic we had earlier, do the earlier dsbs here only need
> to be store barriers?

I think so, and I'm considering doing that in a followup patch.

Ian.

Re: [PATCH v4 0/6 ] xen: arm: smp & tlb cleanups

[Ian Campbell]

On Thu, 2014-04-03 at 09:59 +0100, Ian Campbell wrote:
> Various SMP and TLB etc related cleanups.

I've applied the first four with Julien's additional acks and the coding
style cleanup which was mentioned.

I'll revisit relaxing the barriers.

Ian.