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

[Xen-changelog] [xen-unstable] [HVM] Inject #PF when mmio instruction fetch fails



# HG changeset patch
# User Tim Deegan <Tim.Deegan@xxxxxxxxxxxxx>
# Date 1185878247 -3600
# Node ID 66055f773d19c8026708ecd7e6d5c488ad3a5766
# Parent  66147ca8f9c45a89676a8b7dd42f1ea590e840f1
[HVM] Inject #PF when mmio instruction fetch fails
instead of crashing the guest.  This can happen if one vcpu pages out
another vcpu's kernel text page while the other is performing an mmio op.
Signed-off-by: Tim Deegan <Tim.Deegan@xxxxxxxxxxxxx>
---
 xen/arch/x86/hvm/instrlen.c   |  113 ++++++++++++++++++------------------------
 xen/arch/x86/hvm/platform.c   |   14 +----
 xen/arch/x86/hvm/vmx/vmx.c    |    2 
 xen/include/asm-x86/hvm/hvm.h |    3 -
 4 files changed, 59 insertions(+), 73 deletions(-)

diff -r 66147ca8f9c4 -r 66055f773d19 xen/arch/x86/hvm/instrlen.c
--- a/xen/arch/x86/hvm/instrlen.c       Tue Jul 31 10:11:47 2007 +0100
+++ b/xen/arch/x86/hvm/instrlen.c       Tue Jul 31 11:37:27 2007 +0100
@@ -7,14 +7,6 @@
  *
  * Essentially a very, very stripped version of Keir Fraser's work in
  * x86_emulate.c.  Used for MMIO.
- */
-
-/*
- * TODO: The way in which we use hvm_instruction_length is very inefficient as
- * it now stands. It will be worthwhile to return the actual instruction buffer
- * along with the instruction length since one of the reasons we are getting
- * the instruction length is to know how many instruction bytes we need to
- * fetch.
  */
 
 #include <xen/config.h>
@@ -194,31 +186,51 @@ static uint8_t twobyte_table[256] = {
 /* 
  * insn_fetch - fetch the next byte from instruction stream
  */
-#define insn_fetch()                                                    \
-({ uint8_t _x;                                                          \
-   if ( length >= 15 )                                                  \
-       return -1;                                                       \
-   if ( inst_copy_from_guest(&_x, pc, 1) != 1 ) {                       \
-       gdprintk(XENLOG_WARNING,                                         \
-                "Cannot read from address %lx (eip %lx, mode %d)\n",    \
-                pc, org_pc, address_bytes);                             \
-       return -1;                                                       \
-   }                                                                    \
-   pc += 1;                                                             \
-   length += 1;                                                         \
-   _x;                                                                  \
+#define insn_fetch()                                                      \
+({ uint8_t _x;                                                            \
+   if ( length >= 15 )                                                    \
+       return -1;                                                         \
+   if ( inst_copy_from_guest(&_x, pc, 1) != 1 ) {                         \
+       unsigned long err;                                                 \
+       struct segment_register cs;                                        \
+       gdprintk(XENLOG_WARNING,                                           \
+                "Cannot read from address %lx (eip %lx, mode %d)\n",      \
+                pc, org_pc, address_bytes);                               \
+       err = 0; /* Must be not-present: we don't enforce reserved bits */ \
+       if ( hvm_nx_enabled(current) )                                     \
+           err |= PFEC_insn_fetch;                                        \
+       hvm_get_segment_register(current, x86_seg_cs, &cs);                \
+       if ( cs.attr.fields.dpl != 0 )                                     \
+           err |= PFEC_user_mode;                                         \
+       hvm_inject_exception(TRAP_page_fault, err, pc);                    \
+       return -1;                                                         \
+   }                                                                      \
+   if ( buf )                                                             \
+       buf[length] = _x;                                                  \
+   length += 1;                                                           \
+   pc += 1;                                                               \
+   _x;                                                                    \
 })
 
+#define insn_skip(_n) do {                      \
+    int _i;                                     \
+    for ( _i = 0; _i < (_n); _i++) {            \
+        (void) insn_fetch();                    \
+    }                                           \
+} while (0)
+
 /**
- * hvm_instruction_length - returns the current instructions length
+ * hvm_instruction_fetch - read the current instruction and return its length
  *
  * @org_pc: guest instruction pointer
- * @mode: guest operating mode
+ * @address_bytes: guest address width
+ * @buf: (optional) buffer to load actual instruction bytes into
  *
- * EXTERNAL this routine calculates the length of the current instruction
- * pointed to by org_pc.  The guest state is _not_ changed by this routine.
+ * Doesn't increment the guest's instruction pointer, but may
+ * issue faults to the guest.  Returns -1 on failure.
  */
-int hvm_instruction_length(unsigned long org_pc, int address_bytes)
+int hvm_instruction_fetch(unsigned long org_pc, int address_bytes,
+                          unsigned char *buf)
 {
     uint8_t b, d, twobyte = 0, rex_prefix = 0, modrm_reg = 0;
     unsigned int op_default, op_bytes, ad_default, ad_bytes, tmp;
@@ -317,18 +329,13 @@ done_prefixes:
             {
             case 0:
                 if ( modrm_rm == 6 ) 
-                {
-                    length += 2;
-                    pc += 2; /* skip disp16 */
-                }
+                    insn_skip(2); /* skip disp16 */
                 break;
             case 1:
-                length += 1;
-                pc += 1; /* skip disp8 */
+                insn_skip(1); /* skip disp8 */
                 break;
             case 2:
-                length += 2;
-                pc += 2; /* skip disp16 */
+                insn_skip(2); /* skip disp16 */
                 break;
             }
         }
@@ -340,33 +347,19 @@ done_prefixes:
             case 0:
                 if ( (modrm_rm == 4) && 
                      ((insn_fetch() & 7) == 5) )
-                {
-                    length += 4;
-                    pc += 4; /* skip disp32 specified by SIB.base */
-                }
+                    insn_skip(4); /* skip disp32 specified by SIB.base */
                 else if ( modrm_rm == 5 )
-                {
-                    length += 4;
-                    pc += 4; /* skip disp32 */
-                }
+                    insn_skip(4); /* skip disp32 */
                 break;
             case 1:
                 if ( modrm_rm == 4 )
-                {
-                    length += 1;
-                    pc += 1;
-                }
-                length += 1;
-                pc += 1; /* skip disp8 */
+                    insn_skip(1);
+                insn_skip(1); /* skip disp8 */
                 break;
             case 2:
                 if ( modrm_rm == 4 )
-                {
-                    length += 1;
-                    pc += 1;
-                }
-                length += 4;
-                pc += 4; /* skip disp32 */
+                    insn_skip(1);
+                insn_skip(4); /* skip disp32 */
                 break;
             }
         }
@@ -387,12 +380,10 @@ done_prefixes:
         tmp = (d & ByteOp) ? 1 : op_bytes;
         if ( tmp == 8 ) tmp = 4;
         /* NB. Immediates are sign-extended as necessary. */
-        length += tmp;
-        pc += tmp;
+        insn_skip(tmp);
         break;
     case SrcImmByte:
-        length += 1;
-        pc += 1;
+        insn_skip(1);
         break;
     }
 
@@ -402,8 +393,7 @@ done_prefixes:
     switch ( b )
     {
     case 0xa0 ... 0xa3: /* mov */
-        length += ad_bytes;
-        pc += ad_bytes; /* skip src/dst displacement */
+        insn_skip(ad_bytes); /* skip src/dst displacement */
         break;
     case 0xf6 ... 0xf7: /* Grp3 */
         switch ( modrm_reg )
@@ -412,8 +402,7 @@ done_prefixes:
             /* Special case in Grp3: test has an immediate source operand. */
             tmp = (d & ByteOp) ? 1 : op_bytes;
             if ( tmp == 8 ) tmp = 4;
-            length += tmp;
-            pc += tmp;
+            insn_skip(tmp);
             break;
         }
         break;
diff -r 66147ca8f9c4 -r 66055f773d19 xen/arch/x86/hvm/platform.c
--- a/xen/arch/x86/hvm/platform.c       Tue Jul 31 10:11:47 2007 +0100
+++ b/xen/arch/x86/hvm/platform.c       Tue Jul 31 11:37:27 2007 +0100
@@ -1041,17 +1041,13 @@ void handle_mmio(unsigned long gpa)
         /* real or vm86 modes */
         address_bytes = 2;
     inst_addr = hvm_get_segment_base(v, x86_seg_cs) + regs->eip;
-    inst_len = hvm_instruction_length(inst_addr, address_bytes);
+    memset(inst, 0, MAX_INST_LEN);
+    inst_len = hvm_instruction_fetch(inst_addr, address_bytes, inst);
     if ( inst_len <= 0 )
     {
-        printk("handle_mmio: failed to get instruction length\n");
-        domain_crash_synchronous();
-    }
-
-    memset(inst, 0, MAX_INST_LEN);
-    if ( inst_copy_from_guest(inst, inst_addr, inst_len) != inst_len ) {
-        printk("handle_mmio: failed to copy instruction\n");
-        domain_crash_synchronous();
+        gdprintk(XENLOG_DEBUG, "handle_mmio: failed to get instruction\n");
+        /* hvm_instruction_fetch() will have injected a #PF; get out now */
+        return;
     }
 
     if ( mmio_decode(address_bytes, inst, mmio_op, &ad_size,
diff -r 66147ca8f9c4 -r 66055f773d19 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c        Tue Jul 31 10:11:47 2007 +0100
+++ b/xen/arch/x86/hvm/vmx/vmx.c        Tue Jul 31 11:37:27 2007 +0100
@@ -777,7 +777,7 @@ int vmx_vmcs_restore(struct vcpu *v, str
             else
                 addrbytes = 2;
 
-            ilen = hvm_instruction_length(c->rip, addrbytes);
+            ilen = hvm_instruction_fetch(c->rip, addrbytes, NULL);
             __vmwrite(VM_ENTRY_INSTRUCTION_LEN, ilen);
         }
 
diff -r 66147ca8f9c4 -r 66055f773d19 xen/include/asm-x86/hvm/hvm.h
--- a/xen/include/asm-x86/hvm/hvm.h     Tue Jul 31 10:11:47 2007 +0100
+++ b/xen/include/asm-x86/hvm/hvm.h     Tue Jul 31 11:37:27 2007 +0100
@@ -229,7 +229,8 @@ hvm_guest_x86_mode(struct vcpu *v)
     return hvm_funcs.guest_x86_mode(v);
 }
 
-int hvm_instruction_length(unsigned long pc, int address_bytes);
+int hvm_instruction_fetch(unsigned long pc, int address_bytes,
+                          unsigned char *buf);
 
 static inline void
 hvm_update_host_cr3(struct vcpu *v)

_______________________________________________
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®.