Re: [Xen-devel] [PATCH] x86/EPT: defer enabling of A/D maintenance until PML get enabled

[Kai Huang <kai.huang@xxxxxxxxxxxxxxx>]

On 10/15/2015 03:35 PM, Kai Huang wrote:
> On 10/15/2015 03:11 PM, Jan Beulich wrote:
> > > > > On 15.10.15 at 08:42, <kai.huang@xxxxxxxxxxxxxxx> wrote:
> > > Thanks for your comments Jan. Actually I am not happy with combining
> > > with EPT A/D bit update with PML enabling to single function. After
> > > thinking again, how about adding a separate vmx function (ex,
> > > vmx_domain_update_eptp) to update EPTP of VMCS of all vcpus of domain
> > > after p2m->ept.ept_ad is updated. Another good is this function can 
> > > also
> > > be used in the future for other runtime updates to p2m->ept.
> > >
> > > What's your idea?
> > I don't mind, but that's really more of a question to the VMX 
> > maintainers.
> Then I would prefer this way.
>
> Kevin,
>
> Do you have any comments on this thread?
> > > --- a/xen/arch/x86/mm/p2m-ept.c
> > > +++ b/xen/arch/x86/mm/p2m-ept.c
> > > @@ -1129,17 +1129,26 @@ void ept_sync_domain(struct p2m_domain *p2m)
> > >
> > >    static void ept_enable_pml(struct p2m_domain *p2m)
> > >    {
> > >        /*
> > > -     * No need to check if vmx_domain_enable_pml has succeeded or 
> > > not, as
> > > +     * No need to return if vmx_domain_enable_pml has succeeded or 
> > > not, as
> > It seems to me that you'd better use "whether" instead of "if" now
> > (and then perhaps also drop the "or not").
> OK. Thanks.
> > >         * ept_p2m_type_to_flags will do the check, and write 
> > > protection will be
> > >         * used if PML is not enabled.
> > >         */
> > > -    vmx_domain_enable_pml(p2m->domain);
> > > +    if ( vmx_domain_enable_pml(p2m->domain) )
> > > +        return;
> > > +
> > > +    p2m->ept.ept_ad = 1;
> > > +    vmx_domain_update_eptp(p2m->domain);
> > Shouldn't you enable A/D _before_ enabling PML, at least without
> > having a domain-is-paused check here?
> Looks we don't have such function. How about just add 
> ASSERT(atomic_read(&d->pause_count)), just the same as in 
> vmx_domain_enable_pml ?
I mean we can enable A/D before enabling PML, but if so we need 
additional code to clear A/D bit if vmx_domain_enable_pml failed. My 
thinking is considering  the function is called when domain is paused, 
so there's no difference to enable A/D before or after enabling PML.
Thanks,
-Kai
> Thanks,
> -Kai
> > Jan
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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