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

Re: [Xen-devel] [PATCH v2] Nested VMX: CR emulation fix up



Jan Beulich wrote on 2013-10-24:
>>>> Yang Zhang <yang.z.zhang@xxxxxxxxx> 10/23/13 9:20 AM >>>
>> From: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>> 
>> This patch fixs two issues:
>> 1. The CR_READ_SHADOW should only cover the value that L2 wirtes to
>> CR when L2 is running. But currently, L0 wirtes wrong value to it
>> during virtual vmentry and L2's CR access emualtion.
>> 
>> 2. L2 changed cr[0/4] in a way that did not change any of L1's
>> shadowed bits, but did change L0 shadowed bits. In this case, the
>> effective cr[0/4] value that L1 would like to write into the hardware
>> is consist of the L2-owned bits from the new value combined with the
>> L1-owned bits from L1's guest cr[0/4].
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>> Acked-by: "Dong, Eddie" <eddie.dong@xxxxxxxxx>
> 
> I don't think this can be counted as ack - neither do I recall having
> seen this ack on the list, nor did you Cc Eddie (I specifically did not alter 
> the Cc list here), i.e.
> he would also have no way to communicate his objection to this.

He already acked the first version. Please have a double check or check the 
attached mail (in case of his mail only send to our internal mailbox).

> Please be transparent here (and I think I told you to Cc maintainers
> on your patches enough times by now).

Sorry. It's my wrong git configuration. This time I used another machine to 
send the patch. And it obviously didn't auto-cc to all body signatures which 
did in my previous machine.

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


Best regards,
Yang

--- Begin Message ---
Acked-by Eddie.dong@xxxxxxxxx

-----Original Message-----
From: Zhang, Yang Z
Sent: Tuesday, October 08, 2013 3:30 PM
To: xen-devel@xxxxxxxxxxxxxxxxxxx
Cc: JBeulich@xxxxxxxx; Dong, Eddie; Zhang, Yang Z
Subject: [PATCH] Nested VMX: CR emulation fix up

From: Yang Zhang <yang.z.zhang@xxxxxxxxx>

This patch fixs two issues:
1. The CR_READ_SHADOW should only cover the value that L2 wirtes to CR when L2 
is running. But currently, L0 wirtes wrong value to it during virtual vmentry 
and L2's CR access emualtion.
2. L2 changed cr0 in a way that did not change any of L1's shadowed bits, but 
did change L0 shadowed bits. In this case, the effective cr0 value that L1 
would like to write into the hardware is consist of the L2-owned bits from the 
new value combined with the L1-owned bits from L1's guest cr0.

Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
---
 xen/arch/x86/hvm/hvm.c         |    6 ++++
 xen/arch/x86/hvm/vmx/vmx.c     |   51 ++++++++++++++++++++++++++++++++++++++-
 xen/arch/x86/hvm/vmx/vvmx.c    |    2 +
 xen/include/asm-x86/hvm/vcpu.h |    3 ++
 4 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 
de81e45..77ff40e 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1817,6 +1817,9 @@ int hvm_set_cr0(unsigned long value)
     }

     v->arch.hvm_vcpu.guest_cr[0] = value;
+    if ( !nestedhvm_vmswitch_in_progress(v) &&
+         nestedhvm_vcpu_in_guestmode(v) )
+        v->arch.hvm_vcpu.nvcpu.guest_cr[0] = value;
     hvm_update_guest_cr(v, 0);

     if ( (value ^ old_value) & X86_CR0_PG ) { @@ -1899,6 +1902,9 @@ int 
hvm_set_cr4(unsigned long value)
     }

     v->arch.hvm_vcpu.guest_cr[4] = value;
+    if ( !nestedhvm_vmswitch_in_progress(v) &&
+         nestedhvm_vcpu_in_guestmode(v) )
+        v->arch.hvm_vcpu.nvcpu.guest_cr[4] = value;
     hvm_update_guest_cr(v, 4);
     hvm_memory_event_cr4(value, old_cr);

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 
9ca8632..279a00a 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1096,6 +1096,30 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned 
int cr)
             vmx_update_cpu_exec_control(v);
         }

+        if ( nestedhvm_vcpu_in_guestmode(v) )
+        {
+            if ( !nestedhvm_vmswitch_in_progress(v) )
+            {
+                /*
+                 * We get here when L2 changed cr0 in a way that did not change
+                 * any of L1's shadowed bits (see nvmx_n2_vmexit_handler),
+                 * but did change L0 shadowed bits. So we first calculate the
+                 * effective cr0 value that L1 would like to write into the
+                 * hardware. It consists of the L2-owned bits from the new
+                 * value combined with the L1-owned bits from L1's guest cr0.
+                 */
+                v->arch.hvm_vcpu.guest_cr[0] &=
+                    ~__get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, 
CR0_GUEST_HOST_MASK);
+                v->arch.hvm_vcpu.guest_cr[0] |=
+                    __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, GUEST_CR0) &
+                    __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, 
CR0_GUEST_HOST_MASK);
+            }
+             /* nvcpu.guest_cr[0] is what L2 write to cr0 actually. */
+            __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.nvcpu.guest_cr[0]);
+        }
+        else
+            __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[0]);
+
         if ( !(v->arch.hvm_vcpu.guest_cr[0] & X86_CR0_TS) )
         {
             if ( v != current )
@@ -1144,7 +1168,6 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned 
int cr)
         v->arch.hvm_vcpu.hw_cr[0] =
             v->arch.hvm_vcpu.guest_cr[0] | hw_cr0_mask;
         __vmwrite(GUEST_CR0, v->arch.hvm_vcpu.hw_cr[0]);
-        __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[0]);

         /* Changing CR0 can change some bits in real CR4. */
         vmx_update_guest_cr(v, 4);
@@ -1169,6 +1192,31 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned 
int cr)
         v->arch.hvm_vcpu.hw_cr[4] = HVM_CR4_HOST_MASK;
         if ( paging_mode_hap(v->domain) )
             v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_PAE;
+
+        if ( nestedhvm_vcpu_in_guestmode(v) )
+        {
+            if ( !nestedhvm_vmswitch_in_progress(v) )
+            {
+                /*
+                 * We get here when L2 changed cr4 in a way that did not change
+                 * any of L1's shadowed bits (see nvmx_n2_vmexit_handler),
+                 * but did change L0 shadowed bits. So we first calculate the
+                 * effective cr4 value that L1 would like to write into the
+                 * hardware. It consists of the L2-owned bits from the new
+                 * value combined with the L1-owned bits from L1's guest cr4.
+                 */
+                v->arch.hvm_vcpu.guest_cr[4] &=
+                    ~__get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, 
CR4_GUEST_HOST_MASK);
+                v->arch.hvm_vcpu.guest_cr[4] |=
+                    __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, GUEST_CR4) &
+                    __get_vvmcs(vcpu_nestedhvm(v).nv_vvmcx, 
CR4_GUEST_HOST_MASK);
+            }
+             /* nvcpu.guest_cr[4] is what L2 write to cr4 actually. */
+            __vmwrite(CR4_READ_SHADOW, v->arch.hvm_vcpu.nvcpu.guest_cr[4]);
+        }
+        else
+            __vmwrite(CR4_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[4]);
+
         v->arch.hvm_vcpu.hw_cr[4] |= v->arch.hvm_vcpu.guest_cr[4];
         if ( v->arch.hvm_vmx.vmx_realmode )
             v->arch.hvm_vcpu.hw_cr[4] |= X86_CR4_VME; @@ -1188,7 +1236,6 @@ 
static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr)
             v->arch.hvm_vcpu.hw_cr[4] &= ~X86_CR4_SMEP;
         }
         __vmwrite(GUEST_CR4, v->arch.hvm_vcpu.hw_cr[4]);
-        __vmwrite(CR4_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[4]);
         break;
     default:
         BUG();
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c index 
2b2de77..6b2d51e 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1042,6 +1042,8 @@ static void load_shadow_guest_state(struct vcpu *v)
     vvmcs_to_shadow_bulk(v, ARRAY_SIZE(vmcs_gstate_field),
                          vmcs_gstate_field);

+    nvcpu->guest_cr[0] = __get_vvmcs(vvmcs, CR0_READ_SHADOW);
+    nvcpu->guest_cr[4] = __get_vvmcs(vvmcs, CR4_READ_SHADOW);
     hvm_set_cr0(__get_vvmcs(vvmcs, GUEST_CR0));
     hvm_set_cr4(__get_vvmcs(vvmcs, GUEST_CR4));
     hvm_set_cr3(__get_vvmcs(vvmcs, GUEST_CR3)); diff --git 
a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h index 
e8b8cd7..ab5349a 100644
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -100,6 +100,9 @@ struct nestedvcpu {
      */
     bool_t nv_ioport80;
     bool_t nv_ioportED;
+
+    /* L2's control-resgister, just as the L2 sees them. */
+    unsigned long       guest_cr[5];
 };

 #define vcpu_nestedhvm(v) ((v)->arch.hvm_vcpu.nvcpu)
--
1.7.1


--- End Message ---
_______________________________________________
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®.