From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Vrabel <david.vrabel@citrix.com>
Subject: Re: [PATCHv5 1/2] x86/ept: invalidate guest physical
 mappings on VMENTER
Date: Fri, 18 Dec 2015 11:37:51 +0000
Message-ID: <5673F00F.7070006@citrix.com>
References: <1450365426-23167-1-git-send-email-david.vrabel@citrix.com>
	<1450365426-23167-2-git-send-email-david.vrabel@citrix.com>
	<AADFC41AFE54684AB9EE6CBC0274A5D15F74E124@SHSMSX101.ccr.corp.intel.com>
	<5673DD64.90500@citrix.com>
	<5673EEDA02000078000C1150@prv-mh.provo.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta5.messagelabs.com ([195.245.231.135])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <prvs=787c6add4=david.vrabel@citrix.com>)
	id 1a9tMF-0001Zs-PQ
	for xen-devel@lists.xenproject.org; Fri, 18 Dec 2015 11:37:59 +0000
In-Reply-To: <5673EEDA02000078000C1150@prv-mh.provo.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Kevin Tian <kevin.tian@intel.com>, Jun Nakajima <jun.nakajima@intel.com>, "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
List-Id: xen-devel@lists.xenproject.org

On 18/12/15 10:32, Jan Beulich wrote:
>>>> On 18.12.15 at 11:18, <david.vrabel@citrix.com> wrote:
>> On 18/12/15 07:53, Tian, Kevin wrote:
>>>> From: David Vrabel [mailto:david.vrabel@citrix.com]
>>>> 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.)

There is no race since each pcpu clears its own bit.

David