[v1,2/3] x86/vvmx: correct nested shadow VMCS handling

Commit Message

Sergey Dyasli March 13, 2017, 10:51 a.m. UTC

Currently xen always sets the shadow VMCS-indicator bit on nested
vmptrld and always clears it on nested vmclear.  This behavior is
wrong when the guest loads a shadow VMCS: shadow bit will be lost
on nested vmclear.

Fix this by checking if the guest has provided a shadow VMCS.

Signed-off-by: Sergey Dyasli <[e-mail]>
---
 xen/arch/x86/hvm/vmx/vvmx.c        | 22 ++++++++++++++++++----
 xen/include/asm-x86/hvm/vmx/vvmx.h |  1 +
 2 files changed, 19 insertions(+), 4 deletions(-)

Comments

Tian, Kevin March 14, 2017, 9:11 a.m. UTC | #1

> From: Sergey Dyasli [mailto:[e-mail]]
> Sent: Monday, March 13, 2017 6:52 PM
> 
> Currently xen always sets the shadow VMCS-indicator bit on nested vmptrld
> and always clears it on nested vmclear.  This behavior is wrong when the
> guest loads a shadow VMCS: shadow bit will be lost on nested vmclear.
> 
> Fix this by checking if the guest has provided a shadow VMCS.
> 
> Signed-off-by: Sergey Dyasli <[e-mail]>

Acked-by: Kevin Tian <[e-mail]>

Patch

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 09e4250..3017849 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1119,10 +1119,19 @@  static bool_t nvmx_vpid_enabled(const struct vcpu *v)
 
 static void nvmx_set_vmcs_pointer(struct vcpu *v, struct vmcs_struct *vvmcs)
 {
+    struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
     paddr_t vvmcs_maddr = v->arch.hvm_vmx.vmcs_shadow_maddr;
 
     __vmpclear(vvmcs_maddr);
-    vvmcs->vmcs_revision_id |= VMCS_RID_TYPE_MASK;
+    if ( !nvmx->shadow_vmcs )
+    {
+        /*
+         * We must set the shadow VMCS-indicator in order for the next vmentry
+         * to succeed with a newly set up link pointer in vmcs01.
+         * Note: guest can see that this bit was set.
+         */
+        vvmcs->vmcs_revision_id |= VMCS_RID_TYPE_MASK;
+    }
     __vmwrite(VMCS_LINK_POINTER, vvmcs_maddr);
     __vmwrite(VMREAD_BITMAP, page_to_maddr(v->arch.hvm_vmx.vmread_bitmap));
     __vmwrite(VMWRITE_BITMAP, page_to_maddr(v->arch.hvm_vmx.vmwrite_bitmap));
@@ -1130,10 +1139,13 @@  static void nvmx_set_vmcs_pointer(struct vcpu *v, struct vmcs_struct *vvmcs)
 
 static void nvmx_clear_vmcs_pointer(struct vcpu *v, struct vmcs_struct *vvmcs)
 {
+    struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
     paddr_t vvmcs_maddr = v->arch.hvm_vmx.vmcs_shadow_maddr;
 
     __vmpclear(vvmcs_maddr);
-    vvmcs->vmcs_revision_id &= ~VMCS_RID_TYPE_MASK;
+    if ( !nvmx->shadow_vmcs )
+        vvmcs->vmcs_revision_id &= ~VMCS_RID_TYPE_MASK;
+    nvmx->shadow_vmcs = false;
     __vmwrite(VMCS_LINK_POINTER, ~0ul);
     __vmwrite(VMREAD_BITMAP, 0);
     __vmwrite(VMWRITE_BITMAP, 0);
@@ -1674,12 +1686,14 @@  int nvmx_handle_vmptrld(struct cpu_user_regs *regs)
         {
             if ( writable )
             {
+                struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
                 struct vmcs_struct *vvmcs = vvmcx;
 
+                nvmx->shadow_vmcs =
+                    vvmcs->vmcs_revision_id & ~VMX_BASIC_REVISION_MASK;
                 if ( ((vvmcs->vmcs_revision_id ^ vmx_basic_msr) &
                                          VMX_BASIC_REVISION_MASK) ||
-                     (!cpu_has_vmx_vmcs_shadowing &&
-                      (vvmcs->vmcs_revision_id & ~VMX_BASIC_REVISION_MASK)) )
+                     (!cpu_has_vmx_vmcs_shadowing && nvmx->shadow_vmcs) )
                 {
                     hvm_unmap_guest_frame(vvmcx, 1);
                     vmfail(regs, VMX_INSN_VMPTRLD_INCORRECT_VMCS_ID);
diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h b/xen/include/asm-x86/hvm/vmx/vvmx.h
index ca2fb25..9a65218 100644
--- a/xen/include/asm-x86/hvm/vmx/vvmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vvmx.h
@@ -51,6 +51,7 @@  struct nestedvmx {
     } ept;
     uint32_t guest_vpid;
     struct list_head launched_list;
+    bool shadow_vmcs;
 };
 
 #define vcpu_2_nvmx(v)	(vcpu_nestedhvm(v).u.nvmx)