Re: [Xen-devel] [PATCH 11/18] arm/altp2m: Make flush_tlb_domain ready for altp2m.

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

On 07/04/2016 01:45 PM, Sergej Proskurin wrote:
> This commit makes sure that the TLB of a domain considers flushing all
> of the associated altp2m views. Therefore, in case a different domain
> (not the currently active domain) shall flush its TLBs, the current
> implementation loops over all VTTBRs of the different altp2m mappings
> per vCPU and flushes the TLBs. This way, a change of one of the altp2m
> mapping is considered.  At this point, it must be considered that the
> domain --whose TLBs are to be flushed-- is not locked.
> 
> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/arch/arm/p2m.c | 71 
> ++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 63 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 7e721f9..019f10e 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -15,6 +15,8 @@
>  #include <asm/hardirq.h>
>  #include <asm/page.h>
>  
> +#include <asm/altp2m.h>
> +
>  #ifdef CONFIG_ARM_64
>  static unsigned int __read_mostly p2m_root_order;
>  static unsigned int __read_mostly p2m_root_level;
> @@ -79,12 +81,41 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr)
>                   P2M_ROOT_LEVEL, P2M_ROOT_PAGES);
>  }
>  
> +static uint64_t p2m_get_altp2m_vttbr(struct vcpu *v)
> +{
> +    struct domain *d = v->domain;
> +    uint16_t index = vcpu_altp2m(v).p2midx;
> +
> +    if ( index == INVALID_ALTP2M )
> +        return INVALID_MFN;
> +
> +    BUG_ON(index >= MAX_ALTP2M);
> +
> +    return d->arch.altp2m_vttbr[index];
> +}
> +
> +static void p2m_load_altp2m_VTTBR(struct vcpu *v)
> +{
> +    struct domain *d = v->domain;
> +    uint64_t vttbr = p2m_get_altp2m_vttbr(v);
> +
> +    if ( is_idle_domain(d) )
> +        return;
> +
> +    BUG_ON(vttbr == INVALID_MFN);
> +    WRITE_SYSREG64(vttbr, VTTBR_EL2);
> +
> +    isb(); /* Ensure update is visible */
> +}
> +
>  static void p2m_load_VTTBR(struct domain *d)
>  {
>      if ( is_idle_domain(d) )
>          return;
> +
>      BUG_ON(!d->arch.vttbr);
>      WRITE_SYSREG64(d->arch.vttbr, VTTBR_EL2);
> +
>      isb(); /* Ensure update is visible */
>  }
>  
> @@ -101,7 +132,11 @@ void p2m_restore_state(struct vcpu *n)
>      WRITE_SYSREG(hcr & ~HCR_VM, HCR_EL2);
>      isb();
>  
> -    p2m_load_VTTBR(n->domain);
> +    if ( altp2m_active(n->domain) )
> +        p2m_load_altp2m_VTTBR(n);
> +    else
> +        p2m_load_VTTBR(n->domain);
> +
>      isb();
>  
>      if ( is_32bit_domain(n->domain) )
> @@ -119,22 +154,42 @@ void p2m_restore_state(struct vcpu *n)
>  void flush_tlb_domain(struct domain *d)
>  {
>      unsigned long flags = 0;
> +    struct vcpu *v = NULL;
>  
> -    /* Update the VTTBR if necessary with the domain d. In this case,
> -     * it's only necessary to flush TLBs on every CPUs with the current VMID
> -     * (our domain).
> +    /*
> +     * Update the VTTBR if necessary with the domain d. In this case, it is 
> only
> +     * necessary to flush TLBs on every CPUs with the current VMID (our
> +     * domain).
>       */
>      if ( d != current->domain )
>      {
>          local_irq_save(flags);
> -        p2m_load_VTTBR(d);
> -    }
>  
> -    flush_tlb();
> +        /* If altp2m is active, update VTTBR and flush TLBs of every VCPU */
> +        if ( altp2m_active(d) )
> +        {
> +            for_each_vcpu( d, v )
> +            {
> +                p2m_load_altp2m_VTTBR(v);
> +                flush_tlb();
> +            }
> +        }
> +        else
> +        {
> +            p2m_load_VTTBR(d);
> +            flush_tlb();
> +        }
> +    }
> +    else
> +        flush_tlb();

Flushing of all vCPU's TLBs must also be performed on the current
domain. Does it make sense to flush the current domain's TLBs at this
point as well? If the current domain's TLBs were modified and needs
flushing, it will be done during the loading of the VTTBR at the next
VMENTRY anyway.

>  
>      if ( d != current->domain )
>      {
> -        p2m_load_VTTBR(current->domain);
> +        /* Make sure altp2m mapping is valid. */
> +        if ( altp2m_active(current->domain) )
> +            p2m_load_altp2m_VTTBR(current);
> +        else
> +            p2m_load_VTTBR(current->domain);
>          local_irq_restore(flags);
>      }
>  }
> 

-- 
M.Sc. Sergej Proskurin
Wissenschaftlicher Mitarbeiter

Technische Universität München
Fakultät für Informatik
Lehrstuhl für Sicherheit in der Informatik

Boltzmannstraße 3
85748 Garching (bei München)

Tel. +49 (0)89 289-18592
Fax +49 (0)89 289-18579

proskurin@xxxxxxxxxxxxx
www.sec.in.tum.de

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