Re: [Xen-devel] [PATCHv1 1/3] x86/ept: remove unnecessary sync after resolving misconfigured entries

[David Vrabel <dvrabel@xxxxxxxxxx>]

On 10/11/2015 12:22, Jan Beulich wrote:
>>>> On 06.11.15 at 18:37, <david.vrabel@xxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/mm/p2m-ept.c
>> +++ b/xen/arch/x86/mm/p2m-ept.c
>> @@ -644,7 +644,6 @@ bool_t ept_handle_misconfig(uint64_t gpa)
>>      spurious = curr->arch.hvm_vmx.ept_spurious_misconfig;
>>      rc = resolve_misconfig(p2m, PFN_DOWN(gpa));
>>      curr->arch.hvm_vmx.ept_spurious_misconfig = 0;
>> -    ept_sync_domain(p2m);
>>  
>>      p2m_unlock(p2m);
>>  
>> @@ -692,12 +691,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long 
>> gfn, mfn_t mfn,
>>      /* Carry out any eventually pending earlier changes first. */
>>      ret = resolve_misconfig(p2m, gfn);
>>      if ( ret < 0 )
>> -    {
>> -        ept_sync_domain(p2m);
> 
> Is avoiding this sync really a win? It shouldn't be needed according
> to your analysis, I agree, but if it doesn't do any harm I'd prefer it
> to be kept (and the deletion above to be converted to a similar,
> conditional sync too). After all there also shouldn't be any error
> here, yet if there was one, wanting to be on the safe side calls for
> doing a sync imo.

Unnecessary calls to ept_domain_sync() are confusing.

David

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