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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.10.15 at 04:21, <kai.huang@xxxxxxxxxxxxxxx> wrote:
> Existing PML implementation turns on EPT A/D bit unconditionally if PML is
> supported by hardware. This works but enabling of EPT A/D bit can be 
> deferred
> until PML get enabled. There's no point in enabling the extra feature for 
> every
> domain when we're not meaning to use it (yet).
> 
> Sanity live migration and GUI display were tested on Broadwell Machine.
> 
> Signed-off-by: Kai Huang <kai.huang@xxxxxxxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

There's so little in this patch that came from me that I don't think this is
warranted; but if you want to keep it, the order needs to be switched.
Instead I'd suggest Suggested-by:.

> +void vmx_domain_update_eptp(struct domain *d)
> +{
> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +    struct vcpu *v;
> +
> +    ASSERT(atomic_read(&d->pause_count));

This should imo check controller_pause_count.

> +    for_each_vcpu( d, v )

Coding style: You need to settle on whether you want to treat
for_each_vcpu like a keyword (then there's a blank missing before
the opening paren) or like a normal identifier (then the blanks
immediately inside the parens need to go away).

>  static void ept_flush_pml_buffers(struct p2m_domain *p2m)
>  {
> +    /* Domain must have been paused */
> +    ASSERT(atomic_read(&p2m->domain->pause_count));

This seems unrelated - did you really mean it to go into this patch?

Jan


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