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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.12.15 at 11:18, <david.vrabel@xxxxxxxxxx> wrote:
> On 18/12/15 07:53, Tian, Kevin wrote:
>>> 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.
> 
> The first test is to avoid the locked test and clear in the common case.
>  But this is probably better written as
> 
>   if ( cpumask_test_cpu(cpu, ept->invalidate) )
>   {
>       cpumask_clear_cpu(cpu, ept->invalidate);
>       __invept(...);
>    }

The question is what you care for more: Avoiding the invalidation if
you lose the race here, or avoiding the extra conditional. (Either is
correct in this context afaict.)

Jan


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