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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

>          return ret;
> -    }
> -    if ( ret > 0 )
> -        needs_sync = sync_on;

This deletes the only use of sync_on, i.e. the patch should change
needs_sync back to bool_t at once.

Jan


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