Re: [Xen-devel] [PATCH v2 1/4] x86/EPT: force re-evaluation of memory type as necessary

[Tim Deegan <tim@xxxxxxx>]

At 12:24 +0100 on 03 Apr (1396524290), Jan Beulich wrote:
> >>> On 03.04.14 at 12:18, <tim@xxxxxxx> wrote:
> > Hi, 
> > 
> > This version looks correct to me, but this tristate: 
> > 
> > At 13:17 +0000 on 28 Mar (1396009028), Jan Beulich wrote:
> >> +    p2m_lock(p2m);
> >> +
> >> +    okay = -curr->arch.hvm_vmx.ept_spurious_misconfig;
> > 
> > [...]
> > 
> >> +    if ( okay > 0 )
> >> +    {
> >> +        struct vcpu *v;
> >> +
> >> +        for_each_vcpu ( curr->domain, v )
> >> +            v->arch.hvm_vmx.ept_spurious_misconfig = 1;
> >> +    }
> >> +    curr->arch.hvm_vmx.ept_spurious_misconfig = 0;
> >> +    ept_sync_domain(p2m);
> >> +    p2m_unlock(p2m);
> >> +
> >> +    return !!okay;
> > 
> > seems a bit baroque.  Can we go for 'bool_t okay = 0' at the top and then
> > 
> >     if ( okay )
> >     {
> >         struct vcpu *v;
> > 
> >         for_each_vcpu ( curr->domain, v )
> >             v->arch.hvm_vmx.ept_spurious_misconfig = 1;
> >     }
> >     else
> >         okay = curr->arch.hvm_vmx.ept_spurious_misconfig; 
> >     curr->arch.hvm_vmx.ept_spurious_misconfig = 0;
> >     ept_sync_domain(p2m);
> >     p2m_unlock(p2m);
> > 
> >     return okay;
> > 
> > instead?
> 
> Actually, trying this I find that this conflicts with the added use in
> patch 2, so I'd prefer to keep it the way it is.

Oh.  On closer inspection, in patch 2, when you split a superpage you
set okay = 0 and break, which should cause this whole function to
return 0, killing the guest.  Shouldn't that be a 'continue'?

And I'm not sure that setting okay = 0 is the right thing to do
anyway.  I guess what you're looking for is to avoid a false negative
if you split the superpage and then don't clear an invalid EMT in the
new leaf; but you might have cleared an invalid EMT at a higher
level, in which case okay should not be touched; 'continue' will DTRT.

Or maybe I'm confused.  Time for lunch and I'll stare at it again. :)

Tim.

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