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

[Xen-changelog] [xen-unstable] SVM: clean up __get_instruction_length_from_list()



# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1210077138 -3600
# Node ID 01aa7c088e983cd54b61faeb3ff533581714a26f
# Parent  e6f20d5ed5fe7e24eab12977d812bd499794ba30
SVM: clean up __get_instruction_length_from_list()

Remove unused arguments, fix its behaviour near page boundaries,
inject appropriate pagefaults, and inject #GP if the instruction is
not decodable or %eip is not pointing to valid RAM.

Signed-off-by: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Signed-off-by: Keir Fraser <keir.fraser@xxxxxxxxxx>
---
 xen/arch/x86/hvm/svm/emulate.c        |  115 +++++++++++++++++-----------------
 xen/arch/x86/hvm/svm/svm.c            |   68 ++++++++++----------
 xen/include/asm-x86/hvm/svm/emulate.h |   11 +--
 3 files changed, 100 insertions(+), 94 deletions(-)

diff -r e6f20d5ed5fe -r 01aa7c088e98 xen/arch/x86/hvm/svm/emulate.c
--- a/xen/arch/x86/hvm/svm/emulate.c    Tue May 06 11:05:00 2008 +0100
+++ b/xen/arch/x86/hvm/svm/emulate.c    Tue May 06 13:32:18 2008 +0100
@@ -28,18 +28,6 @@
 #include <asm/hvm/svm/emulate.h>
 
 #define MAX_INST_LEN 15
-
-static int inst_copy_from_guest(
-    unsigned char *buf, unsigned long guest_eip, int inst_len)
-{
-    struct vmcb_struct *vmcb = current->arch.hvm_svm.vmcb;
-    uint32_t pfec = (vmcb->cpl == 3) ? PFEC_user_mode : 0;
-    if ( (inst_len > MAX_INST_LEN) || (inst_len <= 0) )
-        return 0;
-    if ( hvm_fetch_from_guest_virt_nofault(buf, guest_eip, inst_len, pfec) )
-        return 0;
-    return inst_len;
-}
 
 static unsigned int is_prefix(u8 opc)
 {
@@ -73,12 +61,7 @@ static unsigned long svm_rip2pointer(str
     return p;
 }
 
-/* 
- * Here's how it works:
- * First byte: Length. 
- * Following bytes: Opcode bytes. 
- * Special case: Last byte, if zero, doesn't need to match. 
- */
+/* First byte: Length. Following bytes: Opcode bytes. */
 #define MAKE_INSTR(nm, ...) static const u8 OPCODE_##nm[] = { __VA_ARGS__ }
 MAKE_INSTR(INVD,   2, 0x0f, 0x08);
 MAKE_INSTR(WBINVD, 2, 0x0f, 0x09);
@@ -101,70 +84,90 @@ static const u8 *opc_bytes[INSTR_MAX_COU
     [INSTR_INT3]   = OPCODE_INT3
 };
 
+static int fetch(struct vcpu *v, u8 *buf, unsigned long addr, int len)
+{
+    uint32_t pfec = (v->arch.hvm_svm.vmcb->cpl == 3) ? PFEC_user_mode : 0;
+
+    switch ( hvm_fetch_from_guest_virt(buf, addr, len, pfec) )
+    {
+    case HVMCOPY_okay:
+        return 1;
+    case HVMCOPY_bad_gva_to_gfn:
+        /* OK just to give up; we'll have injected #PF already */
+        return 0;
+    case HVMCOPY_bad_gfn_to_mfn:
+    default:
+        /* Not OK: fetches from non-RAM pages are not supportable. */
+        gdprintk(XENLOG_WARNING, "Bad instruction fetch at %#lx (%#lx)\n",
+                 (unsigned long) guest_cpu_user_regs()->eip, addr);
+        hvm_inject_exception(TRAP_gp_fault, 0, 0);
+        return 0;
+    }
+}
+
 int __get_instruction_length_from_list(struct vcpu *v,
-        enum instruction_index *list, unsigned int list_count, 
-        u8 *guest_eip_buf, enum instruction_index *match)
+        enum instruction_index *list, unsigned int list_count)
 {
     struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
     unsigned int i, j, inst_len = 0;
-    int found = 0;
     enum instruction_index instr = 0;
-    u8 buffer[MAX_INST_LEN];
-    u8 *buf;
+    u8 buf[MAX_INST_LEN];
     const u8 *opcode = NULL;
+    unsigned long fetch_addr;
+    unsigned int fetch_len;
 
-    if ( guest_eip_buf )
+    /* Fetch up to the next page break; we'll fetch from the next page
+     * later if we have to. */
+    fetch_addr = svm_rip2pointer(v);
+    fetch_len = min_t(unsigned int, MAX_INST_LEN,
+                      PAGE_SIZE - (fetch_addr & ~PAGE_MASK));
+    if ( !fetch(v, buf, fetch_addr, fetch_len) )
+        return 0;
+
+    while ( (inst_len < MAX_INST_LEN) && is_prefix(buf[inst_len]) )
     {
-        buf = guest_eip_buf;
-    }
-    else
-    {
-        if ( inst_copy_from_guest(buffer, svm_rip2pointer(v), MAX_INST_LEN)
-             != MAX_INST_LEN )
-            return 0;
-        buf = buffer;
+        inst_len++;
+        if ( inst_len >= fetch_len )
+        {
+            if ( !fetch(v, buf + fetch_len, fetch_addr + fetch_len,
+                        MAX_INST_LEN - fetch_len) )
+                return 0;
+            fetch_len = MAX_INST_LEN;
+        }
     }
 
     for ( j = 0; j < list_count; j++ )
     {
         instr = list[j];
         opcode = opc_bytes[instr];
-        ASSERT(opcode);
 
-        while ( (inst_len < MAX_INST_LEN) && 
-                is_prefix(buf[inst_len]) && 
-                !is_prefix(opcode[1]) )
-            inst_len++;
-
-        ASSERT(opcode[0] <= 15);    /* Make sure the table is correct. */
-        found = 1;
-
-        for ( i = 0; i < opcode[0]; i++ )
+        for ( i = 0; (i < opcode[0]) && ((inst_len + i) < MAX_INST_LEN); i++ )
         {
-            /* If the last byte is zero, we just accept it without checking */
-            if ( (i == (opcode[0]-1)) && (opcode[i+1] == 0) )
-                break;
+            if ( (inst_len + i) >= fetch_len ) 
+            { 
+                if ( !fetch(v, buf + fetch_len, 
+                            fetch_addr + fetch_len, 
+                            MAX_INST_LEN - fetch_len) ) 
+                    return 0;
+                fetch_len = MAX_INST_LEN;
+            }
 
             if ( buf[inst_len+i] != opcode[i+1] )
-            {
-                found = 0;
-                break;
-            }
+                goto mismatch;
         }
-
-        if ( found )
-            goto done;
+        goto done;
+    mismatch: ;
     }
 
-    printk("%s: Mismatch between expected and actual instruction bytes: "
-            "eip = %lx\n",  __func__, (unsigned long)vmcb->rip);
+    gdprintk(XENLOG_WARNING,
+             "%s: Mismatch between expected and actual instruction bytes: "
+             "eip = %lx\n",  __func__, (unsigned long)vmcb->rip);
+    hvm_inject_exception(TRAP_gp_fault, 0, 0);
     return 0;
 
  done:
     inst_len += opcode[0];
     ASSERT(inst_len <= MAX_INST_LEN);
-    if ( match )
-        *match = instr;
     return inst_len;
 }
 
diff -r e6f20d5ed5fe -r 01aa7c088e98 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c        Tue May 06 11:05:00 2008 +0100
+++ b/xen/arch/x86/hvm/svm/svm.c        Tue May 06 13:32:18 2008 +0100
@@ -84,7 +84,10 @@ static void inline __update_guest_eip(
 {
     struct vcpu *curr = current;
 
-    if ( unlikely((inst_len == 0) || (inst_len > 15)) )
+    if ( unlikely(inst_len == 0) )
+        return;
+
+    if ( unlikely(inst_len > 15) )
     {
         gdprintk(XENLOG_ERR, "Bad instruction length %u\n", inst_len);
         domain_crash(curr->domain);
@@ -907,8 +910,7 @@ static void svm_vmexit_do_cpuid(struct c
 {
     unsigned int eax, ebx, ecx, edx, inst_len;
 
-    inst_len = __get_instruction_length(current, INSTR_CPUID, NULL);
-    if ( inst_len == 0 ) 
+    if ( (inst_len = __get_instruction_length(current, INSTR_CPUID)) == 0 )
         return;
 
     eax = regs->eax;
@@ -1083,13 +1085,15 @@ static void svm_do_msr_access(struct cpu
 
     if ( vmcb->exitinfo1 == 0 )
     {
+        if ( (inst_len = __get_instruction_length(v, INSTR_RDMSR)) == 0 )
+            return;
         rc = hvm_msr_read_intercept(regs);
-        inst_len = __get_instruction_length(v, INSTR_RDMSR, NULL);
     }
     else
     {
+        if ( (inst_len = __get_instruction_length(v, INSTR_WRMSR)) == 0 )
+            return;
         rc = hvm_msr_write_intercept(regs);
-        inst_len = __get_instruction_length(v, INSTR_WRMSR, NULL);
     }
 
     if ( rc == X86EMUL_OKAY )
@@ -1101,34 +1105,36 @@ static void svm_vmexit_do_hlt(struct vmc
 {
     unsigned int inst_len;
 
-    inst_len = __get_instruction_length(current, INSTR_HLT, NULL);
+    if ( (inst_len = __get_instruction_length(current, INSTR_HLT)) == 0 )
+        return;
+    __update_guest_eip(regs, inst_len);
+
+    hvm_hlt(regs->eflags);
+}
+
+static void wbinvd_ipi(void *info)
+{
+    wbinvd();
+}
+
+static void svm_wbinvd_intercept(void)
+{
+    if ( !list_empty(&(domain_hvm_iommu(current->domain)->pdev_list)) )
+        on_each_cpu(wbinvd_ipi, NULL, 1, 1);
+}
+
+static void svm_vmexit_do_invalidate_cache(struct cpu_user_regs *regs)
+{
+    enum instruction_index list[] = { INSTR_INVD, INSTR_WBINVD };
+    int inst_len;
+
+    inst_len = __get_instruction_length_from_list(
+        current, list, ARRAY_SIZE(list));
     if ( inst_len == 0 )
         return;
-    __update_guest_eip(regs, inst_len);
-
-    hvm_hlt(regs->eflags);
-}
-
-static void wbinvd_ipi(void *info)
-{
-    wbinvd();
-}
-
-static void svm_wbinvd_intercept(void)
-{
-    if ( !list_empty(&(domain_hvm_iommu(current->domain)->pdev_list)) )
-        on_each_cpu(wbinvd_ipi, NULL, 1, 1);
-}
-
-static void svm_vmexit_do_invalidate_cache(struct cpu_user_regs *regs)
-{
-    enum instruction_index list[] = { INSTR_INVD, INSTR_WBINVD };
-    int inst_len;
 
     svm_wbinvd_intercept();
 
-    inst_len = __get_instruction_length_from_list(
-        current, list, ARRAY_SIZE(list), NULL, NULL);
     __update_guest_eip(regs, inst_len);
 }
 
@@ -1204,7 +1210,8 @@ asmlinkage void svm_vmexit_handler(struc
         if ( !v->domain->debugger_attached )
             goto exit_and_crash;
         /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
-        inst_len = __get_instruction_length(v, INSTR_INT3, NULL);
+        if ( (inst_len = __get_instruction_length(v, INSTR_INT3)) == 0 )
+            break;
         __update_guest_eip(regs, inst_len);
         domain_pause_for_debugger();
         break;
@@ -1281,8 +1288,7 @@ asmlinkage void svm_vmexit_handler(struc
         break;
 
     case VMEXIT_VMMCALL:
-        inst_len = __get_instruction_length(v, INSTR_VMCALL, NULL);
-        if ( inst_len == 0 )
+        if ( (inst_len = __get_instruction_length(v, INSTR_VMCALL)) == 0 )
             break;
         HVMTRACE_1D(VMMCALL, v, regs->eax);
         rc = hvm_do_hypercall(regs);
diff -r e6f20d5ed5fe -r 01aa7c088e98 xen/include/asm-x86/hvm/svm/emulate.h
--- a/xen/include/asm-x86/hvm/svm/emulate.h     Tue May 06 11:05:00 2008 +0100
+++ b/xen/include/asm-x86/hvm/svm/emulate.h     Tue May 06 13:32:18 2008 +0100
@@ -34,15 +34,12 @@ enum instruction_index {
 };
 
 int __get_instruction_length_from_list(
-    struct vcpu *v,
-    enum instruction_index *list, unsigned int list_count, 
-    u8 *guest_eip_buf, enum instruction_index *match);
+    struct vcpu *v, enum instruction_index *list, unsigned int list_count);
 
-static inline int __get_instruction_length(struct vcpu *v, 
-        enum instruction_index instr, u8 *guest_eip_buf)
+static inline int __get_instruction_length(
+    struct vcpu *v, enum instruction_index instr)
 {
-    return __get_instruction_length_from_list(
-        v, &instr, 1, guest_eip_buf, NULL);
+    return __get_instruction_length_from_list(v, &instr, 1);
 }
 
 #endif /* __ASM_X86_HVM_SVM_EMULATE_H__ */

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog


 


Rackspace

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