[PATCH v2 0/8] x86/vvmx: various fixes

[PATCH v2 0/8] x86/vvmx: various fixes

[Sergey Dyasli]

These were found by running nested VMX tests from kvm-unit-tests.

Sergey Dyasli (8):
  x86/vvmx: introduce nvmx_vcpu_preinit()
  x86/nestedhvm: introduce vvmcx_valid()
  x86/vvmx: add VMX_INSN_INVEPT_INVVPID_INVALID_OP errno
  x86/vvmx: correct vmfail() usage for vmptrld and vmclear
  x86/vvmx: add VMX_INSN_VMPTRLD_WITH_VMXON_PTR errno
  x86/vvmx: refactor nvmx_handle_vmclear()
  x86/vvmx: correctly report vvmcs size
  x86/vvmx: fix I/O and MSR bitmaps mapping

 xen/arch/x86/hvm/svm/nestedsvm.c    |   2 +-
 xen/arch/x86/hvm/vmx/vmx.c          |   4 +-
 xen/arch/x86/hvm/vmx/vvmx.c         | 211 ++++++++++++++++++----------
 xen/include/asm-x86/hvm/nestedhvm.h |   5 +
 xen/include/asm-x86/hvm/vmx/vmcs.h  |   3 +
 xen/include/asm-x86/hvm/vmx/vvmx.h  |   1 +
 6 files changed, 150 insertions(+), 76 deletions(-)

-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 1/8] x86/vvmx: introduce nvmx_vcpu_preinit()

[Sergey Dyasli]

And call it during vmx_vcpu_initialise(). This allows to safely use
vvmx functions that rely on the values inside struct nestedvmx and
struct nestedvcpu, independently of the nested virtualisation
(HVM_PARAM_NESTEDHVM) status of a domain.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v2:
- new patch
---
 xen/arch/x86/hvm/vmx/vmx.c         |  4 ++--
 xen/arch/x86/hvm/vmx/vvmx.c        | 10 ++++++++++
 xen/include/asm-x86/hvm/vmx/vvmx.h |  1 +
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index e065f8bbdb..b33a3d6abd 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -429,8 +429,6 @@ static int vmx_vcpu_initialise(struct vcpu *v)
 
     INIT_LIST_HEAD(&v->arch.hvm.vmx.pi_blocking.list);
 
-    vcpu_2_nvmx(v).vmxon_region_pa = INVALID_PADDR;
-
     if ( (rc = vmx_create_vmcs(v)) != 0 )
     {
         dprintk(XENLOG_WARNING,
@@ -464,6 +462,8 @@ static int vmx_vcpu_initialise(struct vcpu *v)
 
     vmx_install_vlapic_mapping(v);
 
+    nvmx_vcpu_preinit(v);
+
     return 0;
 }
 
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index dfd08e2d0a..c8bc8e6d11 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -57,6 +57,16 @@ void nvmx_cpu_dead(unsigned int cpu)
     per_cpu(vvmcs_buf, cpu) = NULL;
 }
 
+/* This function initialises fields that are not 0 by default */
+void nvmx_vcpu_preinit(struct vcpu *v)
+{
+    struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
+    struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
+
+    nvmx->vmxon_region_pa = INVALID_PADDR;
+    nvcpu->nv_vvmcxaddr = INVALID_PADDR;
+}
+
 int nvmx_vcpu_initialise(struct vcpu *v)
 {
     struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h b/xen/include/asm-x86/hvm/vmx/vvmx.h
index 6b9c4ae0b2..199dfa1432 100644
--- a/xen/include/asm-x86/hvm/vmx/vvmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vvmx.h
@@ -83,6 +83,7 @@ union vmx_inst_info {
     u32 word;
 };
 
+void nvmx_vcpu_preinit(struct vcpu *v);
 int nvmx_vcpu_initialise(struct vcpu *v);
 void nvmx_vcpu_destroy(struct vcpu *v);
 int nvmx_vcpu_reset(struct vcpu *v);
-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 1/8] x86/vvmx: introduce nvmx_vcpu_preinit()

[Andrew Cooper]

On 06/11/18 12:07, Sergey Dyasli wrote:
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index dfd08e2d0a..c8bc8e6d11 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -57,6 +57,16 @@ void nvmx_cpu_dead(unsigned int cpu)
>      per_cpu(vvmcs_buf, cpu) = NULL;
>  }
>  
> +/* This function initialises fields that are not 0 by default */
> +void nvmx_vcpu_preinit(struct vcpu *v)
> +{
> +    struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
> +    struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
> +
> +    nvmx->vmxon_region_pa = INVALID_PADDR;
> +    nvcpu->nv_vvmcxaddr = INVALID_PADDR;

vmxon_region_pa is VT-x specific, but nv_vvmcxaddr is common.  Instead
of adding yet another function which will be folded eventually, I'd just
fix hvm_vcpu_initialise() (which is where this code will eventually move).

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 2/8] x86/nestedhvm: introduce vvmcx_valid()

[Sergey Dyasli]

As a convenient helper function and refactor the code to use it.

No functional change.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
CC: Brian Woods <brian.woods@amd.com>

v2:
- Use the new helper in nestedsvm.c
---
 xen/arch/x86/hvm/svm/nestedsvm.c    |  2 +-
 xen/arch/x86/hvm/vmx/vvmx.c         | 17 ++++++++---------
 xen/include/asm-x86/hvm/nestedhvm.h |  5 +++++
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 088b3fd562..9660202210 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -68,7 +68,7 @@ int nestedsvm_vmcb_map(struct vcpu *v, uint64_t vmcbaddr)
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
 
     if (nv->nv_vvmcx != NULL && nv->nv_vvmcxaddr != vmcbaddr) {
-        ASSERT(nv->nv_vvmcxaddr != INVALID_PADDR);
+        ASSERT(vvmcx_valid(v));
         hvm_unmap_guest_frame(nv->nv_vvmcx, 1);
         nv->nv_vvmcx = NULL;
         nv->nv_vvmcxaddr = INVALID_PADDR;
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index c8bc8e6d11..d437f33193 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -489,8 +489,7 @@ static void vmfail(struct cpu_user_regs *regs, enum vmx_insn_errno errno)
     if ( errno == VMX_INSN_SUCCEED )
         return;
 
-    if ( vcpu_nestedhvm(current).nv_vvmcxaddr != INVALID_PADDR &&
-         errno != VMX_INSN_FAIL_INVALID )
+    if ( vvmcx_valid(current) && errno != VMX_INSN_FAIL_INVALID )
         vmfail_valid(regs, errno);
     else
         vmfail_invalid(regs);
@@ -773,7 +772,7 @@ static void nvmx_purge_vvmcs(struct vcpu *v)
     int i;
 
     __clear_current_vvmcs(v);
-    if ( nvcpu->nv_vvmcxaddr != INVALID_PADDR )
+    if ( vvmcx_valid(v) )
         hvm_unmap_guest_frame(nvcpu->nv_vvmcx, 1);
     nvcpu->nv_vvmcx = NULL;
     nvcpu->nv_vvmcxaddr = INVALID_PADDR;
@@ -1564,7 +1563,7 @@ static int nvmx_vmresume(struct vcpu *v, struct cpu_user_regs *regs)
     struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
 
     /* check VMCS is valid and IO BITMAP is set */
-    if ( (nvcpu->nv_vvmcxaddr != INVALID_PADDR) &&
+    if ( vvmcx_valid(v) &&
             ((nvmx->iobitmap[0] && nvmx->iobitmap[1]) ||
             !(__n2_exec_control(v) & CPU_BASED_ACTIVATE_IO_BITMAP) ) )
         nvcpu->nv_vmentry_pending = 1;
@@ -1581,7 +1580,7 @@ static int nvmx_handle_vmresume(struct cpu_user_regs *regs)
     struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
     unsigned long intr_shadow;
 
-    if ( vcpu_nestedhvm(v).nv_vvmcxaddr == INVALID_PADDR )
+    if ( !vvmcx_valid(v) )
     {
         vmfail_invalid(regs);
         return X86EMUL_OKAY;        
@@ -1612,7 +1611,7 @@ static int nvmx_handle_vmlaunch(struct cpu_user_regs *regs)
     unsigned long intr_shadow;
     int rc;
 
-    if ( vcpu_nestedhvm(v).nv_vvmcxaddr == INVALID_PADDR )
+    if ( !vvmcx_valid(v) )
     {
         vmfail_invalid(regs);
         return X86EMUL_OKAY;
@@ -1665,7 +1664,7 @@ static int nvmx_handle_vmptrld(struct cpu_user_regs *regs)
     if ( nvcpu->nv_vvmcxaddr != gpa )
         nvmx_purge_vvmcs(v);
 
-    if ( nvcpu->nv_vvmcxaddr == INVALID_PADDR )
+    if ( !vvmcx_valid(v) )
     {
         bool_t writable;
         void *vvmcx = hvm_map_guest_frame_rw(paddr_to_pfn(gpa), 1, &writable);
@@ -1804,7 +1803,7 @@ static int nvmx_handle_vmread(struct cpu_user_regs *regs)
     if ( rc != X86EMUL_OKAY )
         return rc;
 
-    if ( vcpu_nestedhvm(v).nv_vvmcxaddr == INVALID_PADDR )
+    if ( !vvmcx_valid(v) )
     {
         vmfail_invalid(regs);
         return X86EMUL_OKAY;
@@ -1846,7 +1845,7 @@ static int nvmx_handle_vmwrite(struct cpu_user_regs *regs)
     if ( decode_vmx_inst(regs, &decode, &operand) != X86EMUL_OKAY )
         return X86EMUL_EXCEPTION;
 
-    if ( vcpu_nestedhvm(v).nv_vvmcxaddr == INVALID_PADDR )
+    if ( !vvmcx_valid(v) )
     {
         vmfail_invalid(regs);
         return X86EMUL_OKAY;
diff --git a/xen/include/asm-x86/hvm/nestedhvm.h b/xen/include/asm-x86/hvm/nestedhvm.h
index 9d1c2742b5..e09fa9d47d 100644
--- a/xen/include/asm-x86/hvm/nestedhvm.h
+++ b/xen/include/asm-x86/hvm/nestedhvm.h
@@ -92,4 +92,9 @@ static inline void nestedhvm_set_cr(struct vcpu *v, unsigned int cr,
         v->arch.hvm.nvcpu.guest_cr[cr] = value;
 }
 
+static inline bool vvmcx_valid(const struct vcpu *v)
+{
+    return vcpu_nestedhvm(v).nv_vvmcxaddr != INVALID_PADDR;
+}
+
 #endif /* _HVM_NESTEDHVM_H */
-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 2/8] x86/nestedhvm: introduce vvmcx_valid()

[Boris Ostrovsky]

On 11/6/18 7:07 AM, Sergey Dyasli wrote:
> As a convenient helper function and refactor the code to use it.
>
> No functional change.
>
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> ---
> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> CC: Brian Woods <brian.woods@amd.com>
>
> v2:
> - Use the new helper in nestedsvm.c

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 2/8] x86/nestedhvm: introduce vvmcx_valid()

[Wei Liu]

On Tue, Nov 06, 2018 at 12:07:53PM +0000, Sergey Dyasli wrote:
> As a convenient helper function and refactor the code to use it.
> 
> No functional change.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 2/8] x86/nestedhvm: introduce vvmcx_valid()

[Tian, Kevin]

> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
> Sent: Tuesday, November 6, 2018 8:08 PM
> 
> As a convenient helper function and refactor the code to use it.
> 
> No functional change.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 3/8] x86/vvmx: add VMX_INSN_INVEPT_INVVPID_INVALID_OP errno

[Sergey Dyasli]

And use it in nvmx_handle_invept() and nvmx_handle_invvpid().

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v2:
- new patch
---
 xen/arch/x86/hvm/vmx/vvmx.c        | 4 ++--
 xen/include/asm-x86/hvm/vmx/vmcs.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index d437f33193..ba4f6d67fe 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1901,7 +1901,7 @@ static int nvmx_handle_invept(struct cpu_user_regs *regs)
         __invept(INVEPT_ALL_CONTEXT, 0);
         break;
     default:
-        vmfail_invalid(regs);
+        vmfail(regs, VMX_INSN_INVEPT_INVVPID_INVALID_OP);
         return X86EMUL_OKAY;
     }
     vmsucceed(regs);
@@ -1926,7 +1926,7 @@ static int nvmx_handle_invvpid(struct cpu_user_regs *regs)
         hvm_asid_flush_vcpu_asid(&vcpu_nestedhvm(current).nv_n2asid);
         break;
     default:
-        vmfail_invalid(regs);
+        vmfail(regs, VMX_INSN_INVEPT_INVVPID_INVALID_OP);
         return X86EMUL_OKAY;
     }
 
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index 76dd04a72d..6d0ae56fc1 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -538,6 +538,7 @@ enum vmx_insn_errno
     VMX_INSN_UNSUPPORTED_VMCS_COMPONENT    = 12,
     VMX_INSN_VMXON_IN_VMX_ROOT             = 15,
     VMX_INSN_VMENTRY_BLOCKED_BY_MOV_SS     = 26,
+    VMX_INSN_INVEPT_INVVPID_INVALID_OP     = 28,
     VMX_INSN_FAIL_INVALID                  = ~0,
 };
 
-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 3/8] x86/vvmx: add VMX_INSN_INVEPT_INVVPID_INVALID_OP errno

[Tian, Kevin]

> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
> Sent: Tuesday, November 6, 2018 8:08 PM
> 
> And use it in nvmx_handle_invept() and nvmx_handle_invvpid().
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 4/8] x86/vvmx: correct vmfail() usage for vmptrld and vmclear

[Sergey Dyasli]

Calling vmfail_valid() is correct only if vvmcx is valid. Modify
functions to use vmfail() instead which performs the necessary check.

While at it, add ASSERTs into vmfail_valid/invalid() to quickly catch
an incorrect usage in the future.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v2:
- Added ASSERTs
---
 xen/arch/x86/hvm/vmx/vvmx.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index ba4f6d67fe..cc6942cb79 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -473,14 +473,19 @@ static void vmfail_valid(struct cpu_user_regs *regs, enum vmx_insn_errno errno)
     struct vcpu *v = current;
     unsigned int eflags = regs->eflags;
 
+    ASSERT(vvmcx_valid(v));
+
     regs->eflags = (eflags & ~X86_EFLAGS_ARITH_MASK) | X86_EFLAGS_ZF;
     set_vvmcs(v, VM_INSTRUCTION_ERROR, errno);
 }
 
 static void vmfail_invalid(struct cpu_user_regs *regs)
 {
+    struct vcpu *v = current;
     unsigned int eflags = regs->eflags;
 
+    ASSERT(!vvmcx_valid(v));
+
     regs->eflags = (eflags & ~X86_EFLAGS_ARITH_MASK) | X86_EFLAGS_CF;
 }
 
@@ -1700,7 +1705,7 @@ static int nvmx_handle_vmptrld(struct cpu_user_regs *regs)
              !map_io_bitmap_all(v) ||
              !_map_msr_bitmap(v) )
         {
-            vmfail_valid(regs, VMX_INSN_VMPTRLD_INVALID_PHYADDR);
+            vmfail(regs, VMX_INSN_VMPTRLD_INVALID_PHYADDR);
             goto out;
         }
     }
@@ -1784,7 +1789,7 @@ static int nvmx_handle_vmclear(struct cpu_user_regs *regs)
     if ( rc == VMSUCCEED )
         vmsucceed(regs);
     else if ( rc == VMFAIL_VALID )
-        vmfail_valid(regs, VMX_INSN_VMCLEAR_INVALID_PHYADDR);
+        vmfail(regs, VMX_INSN_VMCLEAR_INVALID_PHYADDR);
     else
         vmfail_invalid(regs);
 
-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 4/8] x86/vvmx: correct vmfail() usage for vmptrld and vmclear

[Tian, Kevin]

> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
> Sent: Tuesday, November 6, 2018 8:08 PM
> 
> Calling vmfail_valid() is correct only if vvmcx is valid. Modify
> functions to use vmfail() instead which performs the necessary check.
> 
> While at it, add ASSERTs into vmfail_valid/invalid() to quickly catch
> an incorrect usage in the future.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 5/8] x86/vvmx: add VMX_INSN_VMPTRLD_WITH_VMXON_PTR errno

[Sergey Dyasli]

And make nvmx_handle_vmptrld() return the new errno in case the provided
address is the same as vmxon region address.

While at it, correct the return value for not-4KB-aligned case.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
---
v2:
- Added Acked-by
---
 xen/arch/x86/hvm/vmx/vvmx.c        | 10 ++++++++--
 xen/include/asm-x86/hvm/vmx/vmcs.h |  1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index cc6942cb79..af661b3180 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1660,9 +1660,15 @@ static int nvmx_handle_vmptrld(struct cpu_user_regs *regs)
     if ( rc != X86EMUL_OKAY )
         return rc;
 
-    if ( gpa == vcpu_2_nvmx(v).vmxon_region_pa || gpa & 0xfff )
+    if ( gpa & 0xfff )
     {
-        vmfail_invalid(regs);
+        vmfail(regs, VMX_INSN_VMPTRLD_INVALID_PHYADDR);
+        goto out;
+    }
+
+    if ( gpa == vcpu_2_nvmx(v).vmxon_region_pa )
+    {
+        vmfail(regs, VMX_INSN_VMPTRLD_WITH_VMXON_PTR);
         goto out;
     }
 
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index 6d0ae56fc1..eae4e5397e 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -534,6 +534,7 @@ enum vmx_insn_errno
     VMX_INSN_INVALID_CONTROL_STATE         = 7,
     VMX_INSN_INVALID_HOST_STATE            = 8,
     VMX_INSN_VMPTRLD_INVALID_PHYADDR       = 9,
+    VMX_INSN_VMPTRLD_WITH_VMXON_PTR        = 10,
     VMX_INSN_VMPTRLD_INCORRECT_VMCS_ID     = 11,
     VMX_INSN_UNSUPPORTED_VMCS_COMPONENT    = 12,
     VMX_INSN_VMXON_IN_VMX_ROOT             = 15,
-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 6/8] x86/vvmx: refactor nvmx_handle_vmclear()

[Sergey Dyasli]

1. Add VMX_INSN_VMCLEAR_WITH_VMXON_PTR errno and add the appropriate
   check to the function.

2. Correct the return value for not-4KB-aligned case and for invalid
   physaddr (when hvm_map_guest_frame_rw() fails).

3. Remove enum vmx_ops_result and use vmfail/vmsucceed() calls directly.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v2:
- Removal of enum vmx_ops_result and refactoring
---
 xen/arch/x86/hvm/vmx/vvmx.c        | 52 +++++++++++++++++-------------
 xen/include/asm-x86/hvm/vmx/vmcs.h |  1 +
 2 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index af661b3180..2f5370ceed 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -217,12 +217,6 @@ struct vmx_inst_decoded {
     unsigned int reg2;
 };
 
-enum vmx_ops_result {
-    VMSUCCEED,
-    VMFAIL_VALID,
-    VMFAIL_INVALID,
-};
-
 #define CASE_SET_REG(REG, reg)      \
     case VMX_REG_ ## REG: regs->reg = value; break
 #define CASE_GET_REG(REG, reg)      \
@@ -1764,16 +1758,26 @@ static int nvmx_handle_vmclear(struct cpu_user_regs *regs)
     if ( rc != X86EMUL_OKAY )
         return rc;
 
-    BUILD_BUG_ON(X86EMUL_OKAY != VMSUCCEED); /* rc = VMSUCCEED; */
+    if ( gpa == vcpu_2_nvmx(v).vmxon_region_pa )
+    {
+        vmfail(regs, VMX_INSN_VMCLEAR_WITH_VMXON_PTR);
+        goto out;
+    }
+
     if ( gpa & 0xfff )
-        rc = VMFAIL_INVALID;
-    else if ( gpa == nvcpu->nv_vvmcxaddr )
+    {
+        vmfail(regs, VMX_INSN_VMCLEAR_INVALID_PHYADDR);
+        goto out;
+    }
+
+    if ( gpa == nvcpu->nv_vvmcxaddr )
     {
         if ( cpu_has_vmx_vmcs_shadowing )
             nvmx_clear_vmcs_pointer(v, nvcpu->nv_vvmcx);
         clear_vvmcs_launched(&nvmx->launched_list,
                              PFN_DOWN(v->arch.hvm.vmx.vmcs_shadow_maddr));
         nvmx_purge_vvmcs(v);
+        vmsucceed(regs);
     }
     else 
     {
@@ -1781,24 +1785,26 @@ static int nvmx_handle_vmclear(struct cpu_user_regs *regs)
         bool_t writable;
 
         vvmcs = hvm_map_guest_frame_rw(paddr_to_pfn(gpa), 0, &writable);
-        if ( vvmcs ) 
+
+        if ( !vvmcs )
         {
-            if ( writable )
-                clear_vvmcs_launched(&nvmx->launched_list,
-                                     mfn_x(domain_page_map_to_mfn(vvmcs)));
-            else
-                rc = VMFAIL_VALID;
-            hvm_unmap_guest_frame(vvmcs, 0);
+            vmfail(regs, VMX_INSN_VMCLEAR_INVALID_PHYADDR);
+            goto out;
         }
-    }
 
-    if ( rc == VMSUCCEED )
-        vmsucceed(regs);
-    else if ( rc == VMFAIL_VALID )
-        vmfail(regs, VMX_INSN_VMCLEAR_INVALID_PHYADDR);
-    else
-        vmfail_invalid(regs);
+        if ( writable )
+        {
+            clear_vvmcs_launched(&nvmx->launched_list,
+                                 mfn_x(domain_page_map_to_mfn(vvmcs)));
+            vmsucceed(regs);
+        }
+        else
+            vmfail(regs, VMX_INSN_VMCLEAR_INVALID_PHYADDR);
 
+        hvm_unmap_guest_frame(vvmcs, 0);
+    }
+
+out:
     return X86EMUL_OKAY;
 }
 
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index eae4e5397e..b3e800138e 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -529,6 +529,7 @@ enum vmx_insn_errno
 {
     VMX_INSN_SUCCEED                       = 0,
     VMX_INSN_VMCLEAR_INVALID_PHYADDR       = 2,
+    VMX_INSN_VMCLEAR_WITH_VMXON_PTR        = 3,
     VMX_INSN_VMLAUNCH_NONCLEAR_VMCS        = 4,
     VMX_INSN_VMRESUME_NONLAUNCHED_VMCS     = 5,
     VMX_INSN_INVALID_CONTROL_STATE         = 7,
-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 6/8] x86/vvmx: refactor nvmx_handle_vmclear()

[Tian, Kevin]

> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
> Sent: Tuesday, November 6, 2018 8:08 PM
> 
> 1. Add VMX_INSN_VMCLEAR_WITH_VMXON_PTR errno and add the
> appropriate
>    check to the function.
> 
> 2. Correct the return value for not-4KB-aligned case and for invalid
>    physaddr (when hvm_map_guest_frame_rw() fails).
> 
> 3. Remove enum vmx_ops_result and use vmfail/vmsucceed() calls directly.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 7/8] x86/vvmx: correctly report vvmcs size

[Sergey Dyasli]

The size of Xen's virtual vmcs region is 4096 bytes (see comment about
Virtual VMCS layout in include/asm-x86/hvm/vmx/vvmx.h). Correctly report
it to the guest in case when VMCS shadowing is not available instead of
providing H/W value (which is usually smaller).

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v2:
- slight commit message change
---
 xen/arch/x86/hvm/vmx/vvmx.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 2f5370ceed..37d3cdd859 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2101,6 +2101,14 @@ int nvmx_msr_read_intercept(unsigned int msr, u64 *msr_content)
         data = (host_data & (~0ul << 32)) |
                (vmcs->vmcs_revision_id & 0x7fffffff);
         unmap_domain_page(vmcs);
+
+        if ( !cpu_has_vmx_vmcs_shadowing )
+        {
+            /* Report vmcs_region_size as 4096 */
+            data &= ~VMX_BASIC_VMCS_SIZE_MASK;
+            data |= 1ULL << 44;
+        }
+
         break;
     }
     case MSR_IA32_VMX_PINBASED_CTLS:
-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 7/8] x86/vvmx: correctly report vvmcs size

[Wei Liu]

On Tue, Nov 06, 2018 at 12:07:58PM +0000, Sergey Dyasli wrote:
> The size of Xen's virtual vmcs region is 4096 bytes (see comment about
> Virtual VMCS layout in include/asm-x86/hvm/vmx/vvmx.h). Correctly report
> it to the guest in case when VMCS shadowing is not available instead of
> providing H/W value (which is usually smaller).
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> ---
> v2:
> - slight commit message change
> ---
>  xen/arch/x86/hvm/vmx/vvmx.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 2f5370ceed..37d3cdd859 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -2101,6 +2101,14 @@ int nvmx_msr_read_intercept(unsigned int msr, u64 *msr_content)
>          data = (host_data & (~0ul << 32)) |
>                 (vmcs->vmcs_revision_id & 0x7fffffff);
>          unmap_domain_page(vmcs);
> +
> +        if ( !cpu_has_vmx_vmcs_shadowing )
> +        {
> +            /* Report vmcs_region_size as 4096 */
> +            data &= ~VMX_BASIC_VMCS_SIZE_MASK;
> +            data |= 1ULL << 44;

Can you introduce a define for this to avoid using a magic number?

Wei.

> +        }
> +
>          break;
>      }
>      case MSR_IA32_VMX_PINBASED_CTLS:
> -- 
> 2.17.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 7/8] x86/vvmx: correctly report vvmcs size

[Sergey Dyasli]

On 07/11/2018 13:28, Wei Liu wrote:
> On Tue, Nov 06, 2018 at 12:07:58PM +0000, Sergey Dyasli wrote:
>> The size of Xen's virtual vmcs region is 4096 bytes (see comment about
>> Virtual VMCS layout in include/asm-x86/hvm/vmx/vvmx.h). Correctly report
>> it to the guest in case when VMCS shadowing is not available instead of
>> providing H/W value (which is usually smaller).
>>
>> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
>> ---
>> v2:
>> - slight commit message change
>> ---
>>  xen/arch/x86/hvm/vmx/vvmx.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
>> index 2f5370ceed..37d3cdd859 100644
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -2101,6 +2101,14 @@ int nvmx_msr_read_intercept(unsigned int msr, u64 *msr_content)
>>          data = (host_data & (~0ul << 32)) |
>>                 (vmcs->vmcs_revision_id & 0x7fffffff);
>>          unmap_domain_page(vmcs);
>> +
>> +        if ( !cpu_has_vmx_vmcs_shadowing )
>> +        {
>> +            /* Report vmcs_region_size as 4096 */
>> +            data &= ~VMX_BASIC_VMCS_SIZE_MASK;
>> +            data |= 1ULL << 44;
> 
> Can you introduce a define for this to avoid using a magic number?

I don't see much point in making a define for several reasons:

1. It's not going to be used anywhere else

2. Intel SDM clearly states that VMCS size is "at most 4096 (bit 44
   is set if and only if bits 43:32 are clear)"

3. My VMX MSRs series will introduce a nice struct, after which this
   assignment will look like "basic.vmcs_region_size = 4096;"

--
Thanks,
Sergey

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 7/8] x86/vvmx: correctly report vvmcs size

[Wei Liu]

On Thu, Nov 08, 2018 at 10:28:24AM +0000, Sergey Dyasli wrote:
> On 07/11/2018 13:28, Wei Liu wrote:
> > On Tue, Nov 06, 2018 at 12:07:58PM +0000, Sergey Dyasli wrote:
> >> The size of Xen's virtual vmcs region is 4096 bytes (see comment about
> >> Virtual VMCS layout in include/asm-x86/hvm/vmx/vvmx.h). Correctly report
> >> it to the guest in case when VMCS shadowing is not available instead of
> >> providing H/W value (which is usually smaller).
> >>
> >> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> >> ---
> >> v2:
> >> - slight commit message change
> >> ---
> >>  xen/arch/x86/hvm/vmx/vvmx.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> >> index 2f5370ceed..37d3cdd859 100644
> >> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> >> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> >> @@ -2101,6 +2101,14 @@ int nvmx_msr_read_intercept(unsigned int msr, u64 *msr_content)
> >>          data = (host_data & (~0ul << 32)) |
> >>                 (vmcs->vmcs_revision_id & 0x7fffffff);
> >>          unmap_domain_page(vmcs);
> >> +
> >> +        if ( !cpu_has_vmx_vmcs_shadowing )
> >> +        {
> >> +            /* Report vmcs_region_size as 4096 */
> >> +            data &= ~VMX_BASIC_VMCS_SIZE_MASK;
> >> +            data |= 1ULL << 44;
> > 
> > Can you introduce a define for this to avoid using a magic number?
> 
> I don't see much point in making a define for several reasons:
> 
> 1. It's not going to be used anywhere else
> 
> 2. Intel SDM clearly states that VMCS size is "at most 4096 (bit 44
>    is set if and only if bits 43:32 are clear)"
> 
> 3. My VMX MSRs series will introduce a nice struct, after which this
>    assignment will look like "basic.vmcs_region_size = 4096;"
> 

Fair enough. I have no objection anymore.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 7/8] x86/vvmx: correctly report vvmcs size

[Tian, Kevin]

> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
> Sent: Tuesday, November 6, 2018 8:08 PM
> 
> The size of Xen's virtual vmcs region is 4096 bytes (see comment about
> Virtual VMCS layout in include/asm-x86/hvm/vmx/vvmx.h). Correctly
> report
> it to the guest in case when VMCS shadowing is not available instead of
> providing H/W value (which is usually smaller).
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 8/8] x86/vvmx: fix I/O and MSR bitmaps mapping

[Sergey Dyasli]

Currently Xen tries to map bitmaps during emulation of vmptrld and
vmwrite. This is wrong: a guest can store arbitrary values in those
fields.

Make bitmaps mapping happen only during a nested vmentry and only if
the appropriate execution controls are turned on by L1 hypervisor.

For performance reasons, Xen maps bitmaps only:

    1. During the first nested vmentry
    2. After L1 has changed an appropriate vmcs field
    3. After nvmx_purge_vvmcs() was previously called

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
---
v2:
- slight commit message change
---
 xen/arch/x86/hvm/vmx/vvmx.c | 105 +++++++++++++++++++++++-------------
 1 file changed, 68 insertions(+), 37 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 37d3cdd859..37dbd1abb7 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -717,6 +717,17 @@ static void __clear_current_vvmcs(struct vcpu *v)
         __vmpclear(nvcpu->nv_n2vmcx_pa);
 }
 
+static void unmap_msr_bitmap(struct vcpu *v)
+{
+    struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
+
+    if ( nvmx->msrbitmap )
+    {
+        hvm_unmap_guest_frame(nvmx->msrbitmap, 1);
+        nvmx->msrbitmap = NULL;
+    }
+}
+
 /*
  * Refreshes the MSR bitmap mapping for the current nested vcpu.  Returns true
  * for a successful mapping, and returns false for MSR_BITMAP parameter errors
@@ -727,12 +738,7 @@ static bool __must_check _map_msr_bitmap(struct vcpu *v)
     struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
     uint64_t gpa;
 
-    if ( nvmx->msrbitmap )
-    {
-        hvm_unmap_guest_frame(nvmx->msrbitmap, 1);
-        nvmx->msrbitmap = NULL;
-    }
-
+    unmap_msr_bitmap(v);
     gpa = get_vvmcs(v, MSR_BITMAP);
 
     if ( !IS_ALIGNED(gpa, PAGE_SIZE) )
@@ -743,6 +749,17 @@ static bool __must_check _map_msr_bitmap(struct vcpu *v)
     return nvmx->msrbitmap != NULL;
 }
 
+static void unmap_io_bitmap(struct vcpu *v, unsigned int idx)
+{
+    struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
+
+    if ( nvmx->iobitmap[idx] )
+    {
+        hvm_unmap_guest_frame(nvmx->iobitmap[idx], 1);
+        nvmx->iobitmap[idx] = NULL;
+    }
+}
+
 static bool_t __must_check _map_io_bitmap(struct vcpu *v, u64 vmcs_reg)
 {
     struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
@@ -750,8 +767,7 @@ static bool_t __must_check _map_io_bitmap(struct vcpu *v, u64 vmcs_reg)
     int index;
 
     index = vmcs_reg == IO_BITMAP_A ? 0 : 1;
-    if (nvmx->iobitmap[index])
-        hvm_unmap_guest_frame(nvmx->iobitmap[index], 1);
+    unmap_io_bitmap(v, index);
     gpa = get_vvmcs(v, vmcs_reg);
     nvmx->iobitmap[index] = hvm_map_guest_frame_ro(gpa >> PAGE_SHIFT, 1);
 
@@ -766,7 +782,6 @@ static inline bool_t __must_check map_io_bitmap_all(struct vcpu *v)
 
 static void nvmx_purge_vvmcs(struct vcpu *v)
 {
-    struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
     struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
     int i;
 
@@ -776,16 +791,11 @@ static void nvmx_purge_vvmcs(struct vcpu *v)
     nvcpu->nv_vvmcx = NULL;
     nvcpu->nv_vvmcxaddr = INVALID_PADDR;
     v->arch.hvm.vmx.vmcs_shadow_maddr = 0;
-    for (i=0; i<2; i++) {
-        if ( nvmx->iobitmap[i] ) {
-            hvm_unmap_guest_frame(nvmx->iobitmap[i], 1);
-            nvmx->iobitmap[i] = NULL;
-        }
-    }
-    if ( nvmx->msrbitmap ) {
-        hvm_unmap_guest_frame(nvmx->msrbitmap, 1);
-        nvmx->msrbitmap = NULL;
-    }
+
+    for ( i = 0; i < 2; i++ )
+        unmap_io_bitmap(v, i);
+
+    unmap_msr_bitmap(v);
 }
 
 u64 nvmx_get_tsc_offset(struct vcpu *v)
@@ -1556,20 +1566,34 @@ static void clear_vvmcs_launched(struct list_head *launched_list,
     }
 }
 
-static int nvmx_vmresume(struct vcpu *v, struct cpu_user_regs *regs)
+static enum vmx_insn_errno nvmx_vmresume(struct vcpu *v)
 {
     struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
     struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
+    unsigned int exec_ctrl;
 
-    /* check VMCS is valid and IO BITMAP is set */
-    if ( vvmcx_valid(v) &&
-            ((nvmx->iobitmap[0] && nvmx->iobitmap[1]) ||
-            !(__n2_exec_control(v) & CPU_BASED_ACTIVATE_IO_BITMAP) ) )
-        nvcpu->nv_vmentry_pending = 1;
-    else
-        vmfail_invalid(regs);
+    ASSERT(vvmcx_valid(v));
+    exec_ctrl = __n2_exec_control(v);
 
-    return X86EMUL_OKAY;
+    if ( exec_ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
+    {
+        if ( (nvmx->iobitmap[0] == NULL || nvmx->iobitmap[1] == NULL) &&
+             !map_io_bitmap_all(v) )
+            goto invalid_control_state;
+    }
+
+    if ( exec_ctrl & CPU_BASED_ACTIVATE_MSR_BITMAP )
+    {
+        if ( nvmx->msrbitmap == NULL && !_map_msr_bitmap(v) )
+            goto invalid_control_state;
+    }
+
+    nvcpu->nv_vmentry_pending = 1;
+
+    return VMX_INSN_SUCCEED;
+
+invalid_control_state:
+    return VMX_INSN_INVALID_CONTROL_STATE;
 }
 
 static int nvmx_handle_vmresume(struct cpu_user_regs *regs)
@@ -1578,6 +1602,7 @@ static int nvmx_handle_vmresume(struct cpu_user_regs *regs)
     struct vcpu *v = current;
     struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
     unsigned long intr_shadow;
+    int rc;
 
     if ( !vvmcx_valid(v) )
     {
@@ -1599,7 +1624,12 @@ static int nvmx_handle_vmresume(struct cpu_user_regs *regs)
         vmfail_valid(regs, VMX_INSN_VMRESUME_NONLAUNCHED_VMCS);
         return X86EMUL_OKAY;
     }
-    return nvmx_vmresume(v,regs);
+
+    rc = nvmx_vmresume(v);
+    if ( rc )
+        vmfail_valid(regs, rc);
+
+    return X86EMUL_OKAY;
 }
 
 static int nvmx_handle_vmlaunch(struct cpu_user_regs *regs)
@@ -1631,13 +1661,16 @@ static int nvmx_handle_vmlaunch(struct cpu_user_regs *regs)
         return X86EMUL_OKAY;
     }
     else {
-        rc = nvmx_vmresume(v,regs);
-        if ( rc == X86EMUL_OKAY )
+        rc = nvmx_vmresume(v);
+        if ( rc )
+            vmfail_valid(regs, rc);
+        else
         {
             if ( set_vvmcs_launched(&nvmx->launched_list,
                                     PFN_DOWN(v->arch.hvm.vmx.vmcs_shadow_maddr)) < 0 )
                 return X86EMUL_UNHANDLEABLE;
         }
+        rc = X86EMUL_OKAY;
     }
     return rc;
 }
@@ -1701,9 +1734,7 @@ static int nvmx_handle_vmptrld(struct cpu_user_regs *regs)
                 vvmcx = NULL;
             }
         }
-        if ( !vvmcx ||
-             !map_io_bitmap_all(v) ||
-             !_map_msr_bitmap(v) )
+        else
         {
             vmfail(regs, VMX_INSN_VMPTRLD_INVALID_PHYADDR);
             goto out;
@@ -1879,13 +1910,13 @@ static int nvmx_handle_vmwrite(struct cpu_user_regs *regs)
     switch ( vmcs_encoding & ~VMCS_HIGH(0) )
     {
     case IO_BITMAP_A:
-        okay = _map_io_bitmap(v, IO_BITMAP_A);
+        unmap_io_bitmap(v, 0);
         break;
     case IO_BITMAP_B:
-        okay = _map_io_bitmap(v, IO_BITMAP_B);
+        unmap_io_bitmap(v, 1);
         break;
     case MSR_BITMAP:
-        okay = _map_msr_bitmap(v);
+        unmap_msr_bitmap(v);
         break;
     }
 
-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 8/8] x86/vvmx: fix I/O and MSR bitmaps mapping

[Tian, Kevin]

> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
> Sent: Tuesday, November 6, 2018 8:08 PM
> 
> Currently Xen tries to map bitmaps during emulation of vmptrld and
> vmwrite. This is wrong: a guest can store arbitrary values in those
> fields.
> 
> Make bitmaps mapping happen only during a nested vmentry and only if
> the appropriate execution controls are turned on by L1 hypervisor.
> 
> For performance reasons, Xen maps bitmaps only:
> 
>     1. During the first nested vmentry
>     2. After L1 has changed an appropriate vmcs field
>     3. After nvmx_purge_vvmcs() was previously called
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel