Xen devel - Nov 2013 - [PATCH] xen/arm: p2m: flush TLB by VMID when a new domain is creating

Once the VMID is marked unused, a new domain can reuse the VMID for its
own. If the TLB is not flushed, entries can contain wrong translation.
When a new p2m is allocated, switch to the new VMID and flush TLB on
every physical CPUs.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
    Changes in v2:
        - This patch was formerly "xen/arm: p2m: flush TLB when a domain
        is destroyed
        - Flush TLB by VMID in p2m_alloc_table. It will avoid to flush
        all TLBs every domain destruction.

---
 xen/arch/arm/p2m.c                   | 10 ++++++++++
 xen/include/asm-arm/arm32/flushtlb.h | 11 +++++++++++
 xen/include/asm-arm/arm64/flushtlb.h | 11 +++++++++++
 3 files changed, 32 insertions(+)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 2d09fef..82dda65 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -302,6 +302,15 @@ int p2m_alloc_table(struct domain *d)
     d->arch.vttbr = page_to_maddr(p2m->first_level)
         | ((uint64_t)p2m->vmid&0xff)<<48;
 
+    p2m_load_VTTBR(d);
+
+    /* Make sure that all TLBs corresponding to the new VMID are flushed
+     * before using it
+     */
+    flush_tlb();
+
+    p2m_load_VTTBR(current->domain);
+
     spin_unlock(&p2m->lock);
 
     return 0;
@@ -357,6 +366,7 @@ static void p2m_free_vmid(struct domain *d)
     spin_lock(&vmid_alloc_lock);
     if ( p2m->vmid != INVALID_VMID )
         clear_bit(p2m->vmid, vmid_mask);
+
     spin_unlock(&vmid_alloc_lock);
 }
 
diff --git a/xen/include/asm-arm/arm32/flushtlb.h
b/xen/include/asm-arm/arm32/flushtlb.h
index a258f58..ab166f3 100644
--- a/xen/include/asm-arm/arm32/flushtlb.h
+++ b/xen/include/asm-arm/arm32/flushtlb.h
@@ -12,6 +12,17 @@ static inline void flush_tlb_local(void)
     isb();
 }
 
+/* Flush inner shareable TLBs, current VMID only */
+static inline void flush_tlb(void)
+{
+    dsb();
+
+    WRITE_CP32((uint32_t) 0, TLBIALLIS);
+
+    dsb();
+    isb();
+}
+
 /* Flush local TLBs, all VMIDs, non-hypervisor mode */
 static inline void flush_tlb_all_local(void)
 {
diff --git a/xen/include/asm-arm/arm64/flushtlb.h
b/xen/include/asm-arm/arm64/flushtlb.h
index d0535a0..9ce79a8 100644
--- a/xen/include/asm-arm/arm64/flushtlb.h
+++ b/xen/include/asm-arm/arm64/flushtlb.h
@@ -12,6 +12,17 @@ static inline void flush_tlb_local(void)
         : : : "memory");
 }
 
+/* Flush innershareable TLBs, current VMID only */
+static inline void flush_tlb(void)
+{
+    asm volatile(
+        "dsb sy;"
+        "tlbi vmalle1is;"
+        "dsb sy;"
+        "isb;"
+        : : : "memory");
+}
+
 /* Flush local TLBs, all VMIDs, non-hypervisor mode */
 static inline void flush_tlb_all_local(void)
 {
-- 
1.8.3.1

On Thu, 14 Nov 2013, Julien Grall wrote:
> Once the VMID is marked unused, a new domain can reuse the VMID for its
> own. If the TLB is not flushed, entries can contain wrong translation.
> When a new p2m is allocated, switch to the new VMID and flush TLB on
> every physical CPUs.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>     Changes in v2:
>         - This patch was formerly "xen/arm: p2m: flush TLB when a
domain
>         is destroyed
>         - Flush TLB by VMID in p2m_alloc_table. It will avoid to flush
>         all TLBs every domain destruction.
> 
> ---
>  xen/arch/arm/p2m.c                   | 10 ++++++++++
>  xen/include/asm-arm/arm32/flushtlb.h | 11 +++++++++++
>  xen/include/asm-arm/arm64/flushtlb.h | 11 +++++++++++
>  3 files changed, 32 insertions(+)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 2d09fef..82dda65 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -302,6 +302,15 @@ int p2m_alloc_table(struct domain *d)
>      d->arch.vttbr = page_to_maddr(p2m->first_level)
>          | ((uint64_t)p2m->vmid&0xff)<<48;
>  
> +    p2m_load_VTTBR(d);
> +
> +    /* Make sure that all TLBs corresponding to the new VMID are flushed
> +     * before using it
> +     */
> +    flush_tlb();
> +
> +    p2m_load_VTTBR(current->domain);
> +
>      spin_unlock(&p2m->lock);
>  
>      return 0;
> @@ -357,6 +366,7 @@ static void p2m_free_vmid(struct domain *d)
>      spin_lock(&vmid_alloc_lock);
>      if ( p2m->vmid != INVALID_VMID )
>          clear_bit(p2m->vmid, vmid_mask);
> +
>      spin_unlock(&vmid_alloc_lock);
>  }
>  
> diff --git a/xen/include/asm-arm/arm32/flushtlb.h
b/xen/include/asm-arm/arm32/flushtlb.h
> index a258f58..ab166f3 100644
> --- a/xen/include/asm-arm/arm32/flushtlb.h
> +++ b/xen/include/asm-arm/arm32/flushtlb.h
> @@ -12,6 +12,17 @@ static inline void flush_tlb_local(void)
>      isb();
>  }
>  
> +/* Flush inner shareable TLBs, current VMID only */
> +static inline void flush_tlb(void)
> +{
> +    dsb();
> +
> +    WRITE_CP32((uint32_t) 0, TLBIALLIS);
> +
> +    dsb();
> +    isb();
> +}
Why only inner shareable? Shouldn''t we use the existing flush_tlb_local
instead?

On 11/14/2013 06:10 PM, Stefano Stabellini wrote:
>
> Why only inner shareable? Shouldn''t we use the existing
flush_tlb_local
> instead?
>
As I understand TLBIALL is only to flush TLB on the local processor, 
rather than TLBIALLIS which is used to broadcast on all processor.

So here we need to flush on every processor TLBs by VMID.

--
Julien Grall

On Thu, 14 Nov 2013, Julien Grall wrote:
> On 11/14/2013 06:10 PM, Stefano Stabellini wrote:
> > 
> > Why only inner shareable? Shouldn''t we use the existing
flush_tlb_local
> > instead?
> > 
> 
> As I understand TLBIALL is only to flush TLB on the local processor, rather
> than TLBIALLIS which is used to broadcast on all processor.
> 
> So here we need to flush on every processor TLBs by VMID.
Right, sorry, my mistake.

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

On Thu, 2013-11-14 at 20:41 +0000, Stefano Stabellini
wrote:
> On Thu, 14 Nov 2013, Julien Grall wrote:
> > On 11/14/2013 06:10 PM, Stefano Stabellini wrote:
> > > 
> > > Why only inner shareable? Shouldn''t we use the existing
flush_tlb_local
> > > instead?
> > > 
> > 
> > As I understand TLBIALL is only to flush TLB on the local processor,
rather
> > than TLBIALLIS which is used to broadcast on all processor.
> > 
> > So here we need to flush on every processor TLBs by VMID.
> 
> Right, sorry, my mistake.
> 
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Agreed, + committed.