Re: [Xen-devel] [PATCH] xen/arm: p2m: Correctly flush TLB in create_p2m_entries

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 8 Jan 2014, Julien Grall wrote:
> The p2m is shared between VCPUs for each domain. Currently Xen only flush
> TLB on the local PCPU. This could result to mismatch between the mapping in 
> the
> p2m and TLBs.
> 
> Flush TLBs used by this domain on every PCPU.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>

The fix makes sense to me.

> ---
> 
> This is a possible bug fix (found by reading the code) for Xen 4.4. I have
> added a small optimisation to avoid flushing all TLBs when a VCPU of this
> domain is running on the current cpu.
> 
> The downside of this patch is the function can be a little bit slower because
> Xen is flushing more TLBs.

Yes, I wonder how much slower it is going to be, considering that the flush
is executed for every iteration of the loop.


>  xen/arch/arm/p2m.c                   |    7 ++++++-
>  xen/include/asm-arm/arm32/flushtlb.h |    6 +++---
>  xen/include/asm-arm/arm64/flushtlb.h |    6 +++---
>  3 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 11f4714..9ab0378 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -374,7 +374,12 @@ static int create_p2m_entries(struct domain *d,
>          }
>  
>          if ( flush )
> -            flush_tlb_all_local();
> +        {
> +            if ( current->domain == d )
> +                flush_tlb();
> +            else
> +                flush_tlb_all();
> +        }
>  
>          /* Preempt every 2MiB (mapped) or 32 MiB (unmapped) - arbitrary */
>          if ( op == RELINQUISH && count >= 0x2000 )
> diff --git a/xen/include/asm-arm/arm32/flushtlb.h 
> b/xen/include/asm-arm/arm32/flushtlb.h
> index ab166f3..6ff6f75 100644
> --- a/xen/include/asm-arm/arm32/flushtlb.h
> +++ b/xen/include/asm-arm/arm32/flushtlb.h
> @@ -23,12 +23,12 @@ static inline void flush_tlb(void)
>      isb();
>  }
>  
> -/* Flush local TLBs, all VMIDs, non-hypervisor mode */
> -static inline void flush_tlb_all_local(void)
> +/* Flush inner shareable TLBs, all VMIDs, non-hypervisor mode */
> +static inline void flush_tlb_all(void)
>  {
>      dsb();
>  
> -    WRITE_CP32((uint32_t) 0, TLBIALLNSNH);
> +    WRITE_CP32((uint32_t) 0, TLBIALLNSNHIS);
>  
>      dsb();
>      isb();
> diff --git a/xen/include/asm-arm/arm64/flushtlb.h 
> b/xen/include/asm-arm/arm64/flushtlb.h
> index 9ce79a8..687eda1 100644
> --- a/xen/include/asm-arm/arm64/flushtlb.h
> +++ b/xen/include/asm-arm/arm64/flushtlb.h
> @@ -23,12 +23,12 @@ static inline void flush_tlb(void)
>          : : : "memory");
>  }
>  
> -/* Flush local TLBs, all VMIDs, non-hypervisor mode */
> -static inline void flush_tlb_all_local(void)
> +/* Flush inner shareable TLBs, all VMIDs, non-hypervisor mode */
> +static inline void flush_tlb_all(void)
>  {
>      asm volatile(
>          "dsb sy;"
> -        "tlbi alle1;"
> +        "tlbi alle1is;"
>          "dsb sy;"
>          "isb;"
>          : : : "memory");
> -- 
> 1.7.10.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel