Re: [Xen-devel] [PATCH] x86/altp2m: propagate ept.ad changes to all active altp2ms

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 9/28/18 2:55 PM, Razvan Cojocaru wrote:
> This patch is a pre-requisite for fixing the logdirty VGA issue
> (display freezes when switching to a new altp2m view early in a
> domain's lifetime), but sent separately for easier review.
> The new ept_set_ad_sync() function has been added to update all
> active altp2ms' ept.ad. New altp2ms will inherit the hostp2m's
> ept.ad value. ept_set_ad_sync() is now also the code
> responsible for locking updated p2ms.
> 
> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> Suggested-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> ---
>  xen/arch/x86/mm/p2m-ept.c | 55 
> ++++++++++++++++++++++++++++++++++++++++-------
>  xen/arch/x86/mm/p2m.c     |  8 -------
>  2 files changed, 47 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index d376966..432ff5c 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -17,6 +17,7 @@
>  
>  #include <xen/domain_page.h>
>  #include <xen/sched.h>
> +#include <asm/altp2m.h>
>  #include <asm/current.h>
>  #include <asm/paging.h>
>  #include <asm/types.h>
> @@ -1218,34 +1219,67 @@ static void ept_tlb_flush(struct p2m_domain *p2m)
>      ept_sync_domain_mask(p2m, p2m->domain->dirty_cpumask);
>  }
>  
> +static void ept_set_ad_sync(struct p2m_domain *p2m, int value)
> +{
> +    struct domain *d = p2m->domain;
> +    unsigned int i;
> +
> +    if ( likely(!altp2m_active(d)) )
> +    {
> +        p2m_lock(p2m);
> +        p2m->ept.ad = value;
> +        p2m_unlock(p2m);
> +
> +        return;
> +    }
> +
> +    for ( i = 0; i < MAX_ALTP2M; i++ )
> +    {
> +        if ( d->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
> +            continue;
> +
> +        p2m = d->arch.altp2m_p2m[i];
> +
> +        p2m_lock(p2m);
> +        p2m->ept.ad = value;
> +        p2m_unlock(p2m);
> +    }
> +}
> +
>  static void ept_enable_pml(struct p2m_domain *p2m)
>  {
> +    struct domain *d = p2m->domain;
> +
>      /* Domain must have been paused */
> -    ASSERT(atomic_read(&p2m->domain->pause_count));
> +    ASSERT(atomic_read(&d->pause_count));
>  
>      /*
>       * No need to return whether vmx_domain_enable_pml has succeeded, as
>       * ept_p2m_type_to_flags will do the check, and write protection will be
>       * used if PML is not enabled.
>       */
> -    if ( vmx_domain_enable_pml(p2m->domain) )
> +    if ( vmx_domain_enable_pml(d) )
>          return;
>  
>      /* Enable EPT A/D bit for PML */
> -    p2m->ept.ad = 1;
> -    vmx_domain_update_eptp(p2m->domain);
> +    ept_set_ad_sync(p2m, 1);
> +
> +    vmx_domain_update_eptp(d);
>  }
>  
>  static void ept_disable_pml(struct p2m_domain *p2m)
>  {
> +    struct domain *d = p2m->domain;
> +
>      /* Domain must have been paused */
> -    ASSERT(atomic_read(&p2m->domain->pause_count));
> +    ASSERT(atomic_read(&d->pause_count));
>  
> -    vmx_domain_disable_pml(p2m->domain);
> +    vmx_domain_disable_pml(d);
>  
>      /* Disable EPT A/D bit */
> -    p2m->ept.ad = 0;
> -    vmx_domain_update_eptp(p2m->domain);
> +    ept_set_ad_sync(p2m, 0);
> +
> +    vmx_domain_update_eptp(d);
>  }
>  
>  static void ept_flush_pml_buffers(struct p2m_domain *p2m)
> @@ -1386,8 +1420,13 @@ void setup_ept_dump(void)
>  void p2m_init_altp2m_ept(struct domain *d, unsigned int i)
>  {
>      struct p2m_domain *p2m = d->arch.altp2m_p2m[i];
> +    struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
>      struct ept_data *ept;
>  
> +    p2m_lock(hostp2m);
> +    p2m->ept.ad = hostp2m->ept.ad;
> +    p2m_unlock(hostp2m);

Just realised I shouldn't lock the hostp2m here, sorry for the oversight.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel