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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.11.15 at 17:18, <dvrabel@xxxxxxxxxx> wrote:

> 
> 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.

Again - if there was an error, how do you know it's unnecessary?
But anyway - I'll let the VM maintainers decide.

Jan


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