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

[Kai Huang <kai.huang@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1935 bytes --]

Hi Jan,

After some thinking, just set/clear p2m->ept.ept_ad is not enough -- we 
also need to __vmwrite it to VMCS's EPTP, and then call ept_sync_domain. 
I have verified attached patch can work.

Which implementation would you prefer, existing code or with attached 
patch? If you prefer the latter, please provide comments.

Thanks,
-Kai

On 10/14/2015 09:19 AM, Kai Huang wrote:
> Hi Jan,
>
> Our QA tested this patch but this patch broke PML. Neither GUI display 
> (video ram tracking also uses PML) nor live migration works. I'll 
> investigate what's wrong and get back to you.
>
> Thanks,
> -Kai
>
> On 09/30/2015 08:45 PM, Kai Huang wrote:
>> On Wed, Sep 30, 2015 at 5:54 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 30.09.15 at 10:58, <kaih.linux@gmail.com> wrote:
>>>> Good to me, if you have tested it. Sorry I cannot test it as I am
>>>> taking vacation  until Oct.8.
>>> Note how I asked for help with testing ...
>>>
>>>> On Mon, Sep 28, 2015 at 10:42 PM, Jan Beulich <JBeulich@suse.com> 
>>>> wrote:
>>>>> There's no point in enabling the extra feature for every domain when
>>>>> we're not meaning to use it (yet). Just setting the flag should be
>>>>> sufficient - the domain is required to be paused for PML enabling
>>>>> anyway, i.e. hardware will pick up the new setting the next time
>>>>> each vCPU of the guest gets scheduled.
>>>>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>> Cc: Kai Huang <kai.huang@linux.intel.com>
>>>>> ---
>>>>> VT-x maintainers, Kai: Me lacking the hardware to test this, may I 
>>>>> ask
>>>>> for your help here?
>>> ... here. This patch can certainly wait until you get back from
>>> vacation.
>> Thanks. I'll test it or ask someone has machine to test it after I 
>> get back.
>>
>> Thanks,
>> -Kai
>>> Jan
>>>
>>
>>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>


[-- Attachment #2: 0001-x86-ept-defer-enabling-EPT-A-D-bit-until-PML-is-enab.patch --]
[-- Type: text/x-patch, Size: 3468 bytes --]

>From cd01ef0908ee6d0931ea15ff25606f76fe859757 Mon Sep 17 00:00:00 2001
From: Kai Huang <kai.huang@linux.intel.com>
Date: Wed, 14 Oct 2015 17:01:24 +0800
Subject: [PATCH] x86/ept: defer enabling EPT A/D bit until PML is enabled.

Signed-off-by: Kai Huang <kai.huang@linux.intel.com>
---
 xen/arch/x86/hvm/vmx/vmcs.c | 20 ++++++++++++++++++++
 xen/arch/x86/mm/p2m-ept.c   |  2 --
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 3592a88..9bb278b 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1382,6 +1382,8 @@ bool_t vmx_vcpu_pml_enabled(const struct vcpu *v)
 
 int vmx_vcpu_enable_pml(struct vcpu *v)
 {
+    struct p2m_domain *p2m = p2m_get_hostp2m(v->domain);
+
     if ( vmx_vcpu_pml_enabled(v) )
         return 0;
 
@@ -1399,6 +1401,9 @@ int vmx_vcpu_enable_pml(struct vcpu *v)
     __vmwrite(SECONDARY_VM_EXEC_CONTROL,
               v->arch.hvm_vmx.secondary_exec_control);
 
+    /* we leave ept_sync_domain to vmx_domain_enable_pml */
+    __vmwrite(EPT_POINTER, ept_get_eptp(&p2m->ept));
+
     vmx_vmcs_exit(v);
 
     return 0;
@@ -1406,6 +1411,8 @@ int vmx_vcpu_enable_pml(struct vcpu *v)
 
 void vmx_vcpu_disable_pml(struct vcpu *v)
 {
+    struct p2m_domain *p2m = p2m_get_hostp2m(v->domain);
+
     if ( !vmx_vcpu_pml_enabled(v) )
         return;
 
@@ -1418,6 +1425,9 @@ void vmx_vcpu_disable_pml(struct vcpu *v)
     __vmwrite(SECONDARY_VM_EXEC_CONTROL,
               v->arch.hvm_vmx.secondary_exec_control);
 
+    /* we leave ept_sync_domain to vmx_domain_enable_pml */
+    __vmwrite(EPT_POINTER, ept_get_eptp(&p2m->ept));
+
     vmx_vmcs_exit(v);
 
     v->domain->arch.paging.free_page(v->domain, v->arch.hvm_vmx.pml_pg);
@@ -1492,6 +1502,7 @@ bool_t vmx_domain_pml_enabled(const struct domain *d)
  */
 int vmx_domain_enable_pml(struct domain *d)
 {
+    struct p2m_domain *p2m = p2m_get_hostp2m(d);
     struct vcpu *v;
     int rc;
 
@@ -1500,10 +1511,14 @@ int vmx_domain_enable_pml(struct domain *d)
     if ( vmx_domain_pml_enabled(d) )
         return 0;
 
+    p2m->ept.ept_ad = 1;
+
     for_each_vcpu( d, v )
         if ( (rc = vmx_vcpu_enable_pml(v)) != 0 )
             goto error;
 
+    ept_sync_domain(p2m);
+
     d->arch.hvm_domain.vmx.status |= VMX_DOMAIN_PML_ENABLED;
 
     return 0;
@@ -1523,6 +1538,7 @@ int vmx_domain_enable_pml(struct domain *d)
  */
 void vmx_domain_disable_pml(struct domain *d)
 {
+    struct p2m_domain *p2m = p2m_get_hostp2m(d);
     struct vcpu *v;
 
     ASSERT(atomic_read(&d->pause_count));
@@ -1530,10 +1546,14 @@ void vmx_domain_disable_pml(struct domain *d)
     if ( !vmx_domain_pml_enabled(d) )
         return;
 
+    p2m->ept.ept_ad = 0;
+
     for_each_vcpu( d, v )
         vmx_vcpu_disable_pml(v);
 
     d->arch.hvm_domain.vmx.status &= ~VMX_DOMAIN_PML_ENABLED;
+
+    ept_sync_domain(p2m);
 }
 
 /*
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 74ce9e0..0d689b0 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -1166,8 +1166,6 @@ int ept_p2m_init(struct p2m_domain *p2m)
 
     if ( cpu_has_vmx_pml )
     {
-        /* Enable EPT A/D bits if we are going to use PML. */
-        ept->ept_ad = cpu_has_vmx_pml ? 1 : 0;
         p2m->enable_hardware_log_dirty = ept_enable_pml;
         p2m->disable_hardware_log_dirty = ept_disable_pml;
         p2m->flush_hardware_cached_dirty = ept_flush_pml_buffers;
-- 
2.1.4


[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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