Re: [Xen-devel] [PATCHv5 1/2] x86/ept: invalidate guest physical mappings on VMENTER

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: David Vrabel [mailto:david.vrabel@xxxxxxxxxx]
> Sent: Thursday, December 17, 2015 11:17 PM

[...]

> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index f7c5e4f..cca35f2 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c

[...]

> @@ -3507,6 +3495,16 @@ void vmx_vmenter_helper(const struct cpu_user_regs 
> *regs)
>      if ( unlikely(need_flush) )
>          vpid_sync_all();
> 
> +    if ( paging_mode_hap(curr->domain) )
> +    {
> +        struct ept_data *ept = &p2m_get_hostp2m(curr->domain)->ept;
> +        unsigned int cpu = smp_processor_id();
> +
> +        if ( cpumask_test_cpu(cpu, ept->invalidate)
> +             && cpumask_test_and_clear_cpu(cpu, ept->invalidate) )

Just test_and_clear should be enough.

> +            __invept(INVEPT_SINGLE_CONTEXT, ept_get_eptp(ept), 0);
> +    }
> +
>   out:
>      HVMTRACE_ND(VMENTRY, 0, 1/*cycles*/, 0, 0, 0, 0, 0, 0, 0);
> 
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index eef0372..6e0cf89 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -1089,9 +1089,10 @@ static void ept_memory_type_changed(struct p2m_domain
> *p2m)
> 
>  static void __ept_sync_domain(void *info)
>  {
> -    struct ept_data *ept = &((struct p2m_domain *)info)->ept;
> -
> -    __invept(INVEPT_SINGLE_CONTEXT, ept_get_eptp(ept), 0);
> +    /*
> +     * The invalidate will be done before VMENTER (see

invalidate -> invalidation?

> +     * vmx_vmenter_helper()).
> +     */
>  }
> 
>  void ept_sync_domain(struct p2m_domain *p2m)
> @@ -1107,16 +1108,10 @@ void ept_sync_domain(struct p2m_domain *p2m)
>      if ( nestedhvm_enabled(d) && !p2m_is_nestedp2m(p2m) )
>          p2m_flush_nestedp2m(d);
> 
> -    /*
> -     * Flush active cpus synchronously. Flush others the next time this 
> domain
> -     * is scheduled onto them. We accept the race of other CPUs adding to
> -     * the ept_synced mask before on_selected_cpus() reads it, resulting in
> -     * unnecessary extra flushes, to avoid allocating a cpumask_t on the 
> stack.
> -     */
> -    cpumask_and(ept_get_synced_mask(ept),
> -                d->domain_dirty_cpumask, &cpu_online_map);
> +    /* May need to invalidate on all PCPUs. */

It'd be good to add your earlier description why invalidating all PCPUs are
OK here, to help others better understand the logic later.

Thanks
Kevin


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