[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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





On 10/16/2015 04:17 PM, Jan Beulich wrote:
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:.
I'll change it to 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.
This function is called between domain_pause and domain_unpause, and domain_pause increases d->pause_count, not d->controller_pause_count, so we should check d->pause_count, right?


+    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).
Oh. I will add a blank before the opening paren.


  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?
This function is also supposed to be called when domain is paused, so making it consistent with ept_enable{disable}_pml, I also added the ASSERT here. Is this reasonable?

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.