[PATCH 1/1 V4] x86/AMD: Fix nested svm crash due to assertion in __virt_to_maddr

* [PATCH 1/1 V4] x86/AMD: Fix nested svm crash due to assertion in __virt_to_maddr
@ 2013-07-26 21:46 suravee.suthikulpanit
  2013-07-29 10:43 ` Tim Deegan
  0 siblings, 1 reply; 8+ messages in thread
From: suravee.suthikulpanit @ 2013-07-26 21:46 UTC (permalink / raw)
  To: xen-devel, JBeulich, tim, chegger; +Cc: Suravee Suthikulpanit

From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

Fix assertion in __virt_to_maddr when starting nested SVM guest
in debug mode. Investigation has shown that svm_vmsave/svm_vmload
make use of __pa() with invalid address.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
Changes in V4:
	- Use get_page_from_gfn instead.

 xen/arch/x86/hvm/svm/svm.c        |   49 +++++++++++++++++++++++++++++++++++--
 xen/include/asm-x86/hvm/svm/svm.h |   11 ++++++---
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 4cc4b15..e3b3cab 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1795,6 +1795,27 @@ svm_vmexit_do_vmrun(struct cpu_user_regs *regs,
     return;
 }
 
+static struct page_info *
+_get_vmcb_page(struct domain *d, uint64_t vmcbaddr)
+{
+    struct page_info *page;
+    p2m_type_t p2mt;
+
+    page = get_page_from_gfn(d, vmcbaddr >> PAGE_SHIFT, 
+                            &p2mt, P2M_ALLOC | P2M_UNSHARE);
+    
+    if (!page)
+        return NULL;
+
+    if ( !p2m_is_ram(p2mt) || p2m_is_readonly(p2mt) )
+    {
+        put_page(page);
+        return NULL; 
+    }
+
+    return  page;
+}
+
 static void
 svm_vmexit_do_vmload(struct vmcb_struct *vmcb,
                      struct cpu_user_regs *regs,
@@ -1802,6 +1823,7 @@ svm_vmexit_do_vmload(struct vmcb_struct *vmcb,
 {
     int ret;
     unsigned int inst_len;
+    struct page_info *page;
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
 
     if ( (inst_len = __get_instruction_length(v, INSTR_VMLOAD)) == 0 )
@@ -1819,7 +1841,19 @@ svm_vmexit_do_vmload(struct vmcb_struct *vmcb,
         goto inject;
     }
 
-    svm_vmload(nv->nv_vvmcx);
+    /* Need to translate L1-GPA to MPA */
+    page = _get_vmcb_page(v->domain, nv->nv_vvmcxaddr);
+    if (!page)
+    {
+        gdprintk(XENLOG_ERR,
+            "VMLOAD: mapping vmcb L1-GPA to MPA failed, injecting #UD\n");
+        ret = TRAP_invalid_op;
+        goto inject;
+    }
+
+    svm_vmload_pa(page_to_mfn(page) << PAGE_SHIFT);
+    put_page(page);
+
     /* State in L1 VMCB is stale now */
     v->arch.hvm_svm.vmcb_in_sync = 0;
 
@@ -1838,6 +1872,7 @@ svm_vmexit_do_vmsave(struct vmcb_struct *vmcb,
 {
     int ret;
     unsigned int inst_len;
+    struct page_info *page;
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
 
     if ( (inst_len = __get_instruction_length(v, INSTR_VMSAVE)) == 0 )
@@ -1855,8 +1890,18 @@ svm_vmexit_do_vmsave(struct vmcb_struct *vmcb,
         goto inject;
     }
 
-    svm_vmsave(nv->nv_vvmcx);
+    /* Need to translate L1-GPA to MPA */
+    page = _get_vmcb_page(v->domain, nv->nv_vvmcxaddr);
+    if (!page)
+    {
+        gdprintk(XENLOG_ERR,
+            "VMSAVE: mapping vmcb L1-GPA to MPA failed, injecting #UD\n");
+        ret = TRAP_invalid_op;
+        goto inject;
+    }
 
+    svm_vmsave_pa(page_to_mfn(page) << PAGE_SHIFT);
+    put_page(page);
     __update_guest_eip(regs, inst_len);
     return;
 
diff --git a/xen/include/asm-x86/hvm/svm/svm.h b/xen/include/asm-x86/hvm/svm/svm.h
index 64e7e25..1ffe6d6 100644
--- a/xen/include/asm-x86/hvm/svm/svm.h
+++ b/xen/include/asm-x86/hvm/svm/svm.h
@@ -41,18 +41,21 @@
 #define SVM_REG_R14 (14)
 #define SVM_REG_R15 (15)
 
-static inline void svm_vmload(void *vmcb)
+#define svm_vmload(x)     svm_vmload_pa(__pa(x))
+#define svm_vmsave(x)     svm_vmsave_pa(__pa(x))
+
+static inline void svm_vmload_pa(paddr_t vmcb)
 {
     asm volatile (
         ".byte 0x0f,0x01,0xda" /* vmload */
-        : : "a" (__pa(vmcb)) : "memory" );
+        : : "a" (vmcb) : "memory" );
 }
 
-static inline void svm_vmsave(void *vmcb)
+static inline void svm_vmsave_pa(paddr_t vmcb)
 {
     asm volatile (
         ".byte 0x0f,0x01,0xdb" /* vmsave */
-        : : "a" (__pa(vmcb)) : "memory" );
+        : : "a" (vmcb) : "memory" );
 }
 
 static inline void svm_invlpga(unsigned long vaddr, uint32_t asid)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread