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

[Xen-devel] [RFC PATCH] vvmx: replace vmreturn() by vmsucceed() and vmfail*()



Replace vmreturn() by vmsucceed(), vmfail(), vmfail_valid() and
vmfail_invalid(), which are consistent to the pseudo code on Intel
SDM, and allow to return VM instruction error numbers to L1
hypervisor.

Signed-off-by: Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
---
* This patch is based on my patchset "[PATCH v2 0/4] vvmx: fix L1 vmxon".

* I'm not sure whether this patch by itself makes sense or should be sent
  with additional bugfix patches, so I tag it as RFC. Explanation: besides 
  returning error numbers, this patch does not fix other bugs in the individual
  nvmx_handle_*() function, so the error numbers used (especially) for L1
  vmclear, vmptrld and vmwrite are still inconsistent to Intel SDM, or even
  incorrect.
---
 xen/arch/x86/hvm/vmx/vvmx.c        | 107 +++++++++++++++++++++----------------
 xen/include/asm-x86/hvm/vmx/vmcs.h |  15 ++++--
 2 files changed, 74 insertions(+), 48 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index c4f19a0..f9ae756 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -479,28 +479,37 @@ gp_fault:
     return X86EMUL_EXCEPTION;
 }
 
-static void vmreturn(struct cpu_user_regs *regs, enum vmx_ops_result ops_res)
+#define VMSUCCEED_EFLAGS_MASK \
+    (X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF | \
+     X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF)
+
+static void vmsucceed(struct cpu_user_regs *regs)
+{
+    regs->eflags &= ~VMSUCCEED_EFLAGS_MASK;
+}
+
+static void vmfail_valid(struct cpu_user_regs *regs, enum vmx_insn_errno errno)
 {
+    struct vcpu *v = current;
     unsigned long eflags = regs->eflags;
-    unsigned long mask = X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF |
-                         X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF;
 
-    eflags &= ~mask;
+    regs->eflags = (eflags & ~VMSUCCEED_EFLAGS_MASK) | X86_EFLAGS_ZF;
+    set_vvmcs(v, VM_INSTRUCTION_ERROR, errno);
+}
 
-    switch ( ops_res ) {
-    case VMSUCCEED:
-        break;
-    case VMFAIL_VALID:
-        /* TODO: error number, useful for guest VMM debugging */
-        eflags |= X86_EFLAGS_ZF;
-        break;
-    case VMFAIL_INVALID:
-    default:
-        eflags |= X86_EFLAGS_CF;
-        break;
-    }
+static void vmfail_invalid(struct cpu_user_regs *regs)
+{
+    unsigned long eflags = regs->eflags;
+
+    regs->eflags = (eflags & ~VMSUCCEED_EFLAGS_MASK) | X86_EFLAGS_SF;
+}
 
-    regs->eflags = eflags;
+static void vmfail(struct cpu_user_regs *regs, enum vmx_insn_errno errno)
+{
+    if ( vcpu_nestedhvm(current).nv_vvmcxaddr != INVALID_PADDR )
+        vmfail_valid(regs, errno);
+    else
+        vmfail_invalid(regs);
 }
 
 bool_t nvmx_intercepts_exception(
@@ -1338,7 +1347,7 @@ static void virtual_vmexit(struct cpu_user_regs *regs)
         nvmx_update_apicv(v);
 
     nvcpu->nv_vmswitch_in_progress = 0;
-    vmreturn(regs, VMSUCCEED);
+    vmsucceed(regs);
 }
 
 void nvmx_switch_guest(void)
@@ -1392,15 +1401,13 @@ int nvmx_handle_vmxon(struct cpu_user_regs *regs)
 
     if ( nvmx_vcpu_in_vmx(v) )
     {
-        vmreturn(regs,
-                 nvcpu->nv_vvmcxaddr != INVALID_PADDR ?
-                 VMFAIL_VALID : VMFAIL_INVALID);
+        vmfail(regs, VMX_INSN_VMXON_IN_VMX_ROOT);
         return X86EMUL_OKAY;
     }
 
     if ( (gpa & ~PAGE_MASK) || (gpa >> v->domain->arch.paging.gfn_bits) )
     {
-        vmreturn(regs, VMFAIL_INVALID);
+        vmfail_invalid(regs);
         return X86EMUL_OKAY;
     }
 
@@ -1409,7 +1416,7 @@ int nvmx_handle_vmxon(struct cpu_user_regs *regs)
          (nvmcs_revid & ~VMX_BASIC_REVISION_MASK) ||
          ((nvmcs_revid ^ vmx_basic_msr) & VMX_BASIC_REVISION_MASK) )
     {
-        vmreturn(regs, VMFAIL_INVALID);
+        vmfail_invalid(regs);
         return X86EMUL_OKAY;
     }
 
@@ -1425,7 +1432,7 @@ int nvmx_handle_vmxon(struct cpu_user_regs *regs)
                      _mfn(PFN_DOWN(v->arch.hvm_vmx.vmcs_pa)));
     __vmptrld(v->arch.hvm_vmx.vmcs_pa);
     v->arch.hvm_vmx.launched = 0;
-    vmreturn(regs, VMSUCCEED);
+    vmsucceed(regs);
 
     return X86EMUL_OKAY;
 }
@@ -1443,7 +1450,7 @@ int nvmx_handle_vmxoff(struct cpu_user_regs *regs)
     nvmx_purge_vvmcs(v);
     nvmx->vmxon_region_pa = INVALID_PADDR;
 
-    vmreturn(regs, VMSUCCEED);
+    vmsucceed(regs);
     return X86EMUL_OKAY;
 }
 
@@ -1514,7 +1521,7 @@ static int nvmx_vmresume(struct vcpu *v, struct 
cpu_user_regs *regs)
             !(__n2_exec_control(v) & CPU_BASED_ACTIVATE_IO_BITMAP) ) )
         nvcpu->nv_vmentry_pending = 1;
     else
-        vmreturn(regs, VMFAIL_INVALID);
+        vmfail_invalid(regs);
 
     return X86EMUL_OKAY;
 }
@@ -1531,15 +1538,16 @@ int nvmx_handle_vmresume(struct cpu_user_regs *regs)
 
     if ( vcpu_nestedhvm(v).nv_vvmcxaddr == INVALID_PADDR )
     {
-        vmreturn (regs, VMFAIL_INVALID);
+        vmfail_invalid(regs);
         return X86EMUL_OKAY;        
     }
 
     launched = vvmcs_launched(&nvmx->launched_list,
                               PFN_DOWN(v->arch.hvm_vmx.vmcs_shadow_maddr));
-    if ( !launched ) {
-       vmreturn (regs, VMFAIL_VALID);
-       return X86EMUL_OKAY;
+    if ( !launched )
+    {
+        vmfail_valid(regs, VMX_INSN_VMRESUME_NONLAUNCHED_VMCS);
+        return X86EMUL_OKAY;
     }
     return nvmx_vmresume(v,regs);
 }
@@ -1556,15 +1564,16 @@ int nvmx_handle_vmlaunch(struct cpu_user_regs *regs)
 
     if ( vcpu_nestedhvm(v).nv_vvmcxaddr == INVALID_PADDR )
     {
-        vmreturn (regs, VMFAIL_INVALID);
+        vmfail_invalid(regs);
         return X86EMUL_OKAY;
     }
 
     launched = vvmcs_launched(&nvmx->launched_list,
                               PFN_DOWN(v->arch.hvm_vmx.vmcs_shadow_maddr));
-    if ( launched ) {
-       vmreturn (regs, VMFAIL_VALID);
-       return X86EMUL_OKAY;
+    if ( launched )
+    {
+        vmfail_valid(regs, VMX_INSN_VMLAUNCH_NONCLEAR_VMCS);
+        return X86EMUL_OKAY;
     }
     else {
         rc = nvmx_vmresume(v,regs);
@@ -1592,7 +1601,7 @@ int nvmx_handle_vmptrld(struct cpu_user_regs *regs)
 
     if ( gpa == vcpu_2_nvmx(v).vmxon_region_pa || gpa & 0xfff )
     {
-        vmreturn(regs, VMFAIL_INVALID);
+        vmfail_invalid(regs);
         goto out;
     }
 
@@ -1623,7 +1632,7 @@ int nvmx_handle_vmptrld(struct cpu_user_regs *regs)
              !map_io_bitmap_all(v) ||
              !_map_msr_bitmap(v) )
         {
-            vmreturn(regs, VMFAIL_VALID);
+            vmfail_valid(regs, VMX_INSN_VMPTRLD_INVALID_PHYADDR);
             goto out;
         }
     }
@@ -1631,7 +1640,7 @@ int nvmx_handle_vmptrld(struct cpu_user_regs *regs)
     if ( cpu_has_vmx_vmcs_shadowing )
         nvmx_set_vmcs_pointer(v, nvcpu->nv_vvmcx);
 
-    vmreturn(regs, VMSUCCEED);
+    vmsucceed(regs);
 
 out:
     return X86EMUL_OKAY;
@@ -1658,7 +1667,7 @@ int nvmx_handle_vmptrst(struct cpu_user_regs *regs)
     if ( rc != HVMCOPY_okay )
         return X86EMUL_EXCEPTION;
 
-    vmreturn(regs, VMSUCCEED);
+    vmsucceed(regs);
     return X86EMUL_OKAY;
 }
 
@@ -1704,7 +1713,12 @@ int nvmx_handle_vmclear(struct cpu_user_regs *regs)
         }
     }
 
-    vmreturn(regs, rc);
+    if ( rc == VMSUCCEED )
+        vmsucceed(regs);
+    else if ( rc == VMFAIL_VALID )
+        vmfail_valid(regs, VMX_INSN_VMCLEAR_INVALID_PHYADDR);
+    else
+        vmfail_invalid(regs);
 
     return X86EMUL_OKAY;
 }
@@ -1736,7 +1750,7 @@ int nvmx_handle_vmread(struct cpu_user_regs *regs)
         break;
     }
 
-    vmreturn(regs, VMSUCCEED);
+    vmsucceed(regs);
     return X86EMUL_OKAY;
 }
 
@@ -1768,7 +1782,10 @@ int nvmx_handle_vmwrite(struct cpu_user_regs *regs)
         break;
     }
 
-    vmreturn(regs, okay ? VMSUCCEED : VMFAIL_VALID);
+    if ( okay )
+        vmsucceed(regs);
+    else
+        vmfail_valid(regs, VMX_INSN_UNSUPPORTED_VMCS_COMPONENT);
 
     return X86EMUL_OKAY;
 }
@@ -1799,10 +1816,10 @@ int nvmx_handle_invept(struct cpu_user_regs *regs)
         __invept(INVEPT_ALL_CONTEXT, 0, 0);
         break;
     default:
-        vmreturn(regs, VMFAIL_INVALID);
+        vmfail_invalid(regs);
         return X86EMUL_OKAY;
     }
-    vmreturn(regs, VMSUCCEED);
+    vmsucceed(regs);
     return X86EMUL_OKAY;
 }
 
@@ -1824,11 +1841,11 @@ int nvmx_handle_invvpid(struct cpu_user_regs *regs)
         hvm_asid_flush_vcpu_asid(&vcpu_nestedhvm(current).nv_n2asid);
         break;
     default:
-        vmreturn(regs, VMFAIL_INVALID);
+        vmfail_invalid(regs);
         return X86EMUL_OKAY;
     }
 
-    vmreturn(regs, VMSUCCEED);
+    vmsucceed(regs);
     return X86EMUL_OKAY;
 }
 
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h 
b/xen/include/asm-x86/hvm/vmx/vmcs.h
index 894093d..6c3d7ba 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -574,9 +574,18 @@ enum vmcs_field {
 #define VMX_GUEST_MSR 0
 #define VMX_HOST_MSR  1
 
-/* VM Instruction error numbers. */
-#define VMX_INSN_INVALID_CONTROL_STATE       7
-#define VMX_INSN_INVALID_HOST_STATE          8
+/* VM Instruction error numbers */
+enum vmx_insn_errno
+{
+    VMX_INSN_VMCLEAR_INVALID_PHYADDR       = 2,
+    VMX_INSN_VMLAUNCH_NONCLEAR_VMCS        = 4,
+    VMX_INSN_VMRESUME_NONLAUNCHED_VMCS     = 5,
+    VMX_INSN_INVALID_CONTROL_STATE         = 7,
+    VMX_INSN_INVALID_HOST_STATE            = 8,
+    VMX_INSN_VMPTRLD_INVALID_PHYADDR       = 9,
+    VMX_INSN_UNSUPPORTED_VMCS_COMPONENT    = 12,
+    VMX_INSN_VMXON_IN_VMX_ROOT             = 15,
+};
 
 void vmx_disable_intercept_for_msr(struct vcpu *v, u32 msr, int type);
 void vmx_enable_intercept_for_msr(struct vcpu *v, u32 msr, int type);
-- 
2.10.1


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

 


Rackspace

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