All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Xen 3.2: Fix guest boot on AMD K8 machine
@ 2009-10-01  9:34 Christoph Egger
  0 siblings, 0 replies; only message in thread
From: Christoph Egger @ 2009-10-01  9:34 UTC (permalink / raw)
  To: xen-devel

[-- Attachment #1: Type: text/plain, Size: 750 bytes --]


Hi!

A bug has been introduced in Xen 3.2.2 (and still reproducable with Xen 3.2.3)
which causes the guest to freeze at boot and xen floods the console with this 
messages:

(XEN) platform.c:1049:d6 handle_mmio: failed to get instruction
(XEN) instrlen.c:252:d6 Cannot read from address 802eb001 (eip 802eb001, mode 
2)

The problem is reproducable on AMD K8 machines.
Attached patch fixes this.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen32_guestfreeze.diff --]
[-- Type: text/x-diff, Size: 9429 bytes --]

diff -r 784805591fa2 -r f70475e8396d xen/arch/x86/hvm/svm/emulate.c
--- a/xen/arch/x86/hvm/svm/emulate.c	Tue May 13 15:54:31 2008 +0100
+++ b/xen/arch/x86/hvm/svm/emulate.c	Tue May 13 15:34:33 2008 +0100
@@ -29,6 +29,9 @@
 #include <asm/hvm/svm/emulate.h>
 
 
+extern int inst_copy_from_guest(unsigned char *buf, unsigned long guest_eip,
+        int inst_len);
+
 #define REX_PREFIX_BASE 0x40
 #define REX_X           0x02
 #define REX_W           0x08
@@ -409,8 +412,8 @@ static const u8 *opc_bytes[INSTR_MAX_COU
  * Intel has a vmcs entry to give the instruction length. AMD doesn't.  So we
  * have to do a little bit of work to find out... 
  *
- * The caller may supply a buffer of at least MAX_INST_LEN bytes, which
- * the instruction will be read into.
+ * The caller can either pass a NULL pointer to the guest_eip_buf, or a pointer
+ * to enough bytes to satisfy the instruction including prefix bytes.
  */
 int __get_instruction_length_from_list(struct vcpu *v,
         enum instruction_index *list, unsigned int list_count, 
@@ -422,40 +425,21 @@ int __get_instruction_length_from_list(s
     unsigned int j;
     int found = 0;
     enum instruction_index instr = 0;
-    unsigned long fetch_addr;
-    int fetch_len;
+    u8 buffer[MAX_INST_LEN];
     u8 *buf;
     const u8 *opcode = NULL;
-    u8 temp_buffer[MAX_INST_LEN];
 
-    /* Use the stack if the caller didn't give us a buffer */
-    buf = ( guest_eip_buf ) ? guest_eip_buf : temp_buffer;
-
-#define FETCH(_buf, _addr, _len) do                                           \
-    {                                                                         \
-        switch ( hvm_fetch_from_guest_virt((_buf), (_addr), (_len)) )         \
-        {                                                                     \
-        case HVMCOPY_okay:                                                    \
-            break;                                                            \
-        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:                                          \
-            /* Not OK: fetches from non-RAM pages are not supportable. */     \
-            gdprintk(XENLOG_ERR, "Bad instruction fetch at %#lx (%#lx)\n",    \
-                     (unsigned long) guest_cpu_user_regs()->eip, fetch_addr); \
-            hvm_inject_exception(TRAP_gp_fault, 0, 0);                        \
-            return 0;                                                         \
-        }                                                                     \
-    } while (0)
-
-    /* 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 = PAGE_SIZE - (fetch_addr & ~PAGE_MASK) ;
-    if ( fetch_len > MAX_INST_LEN ) 
-        fetch_len = MAX_INST_LEN;
-    FETCH(buf, fetch_addr, fetch_len);
+    if (guest_eip_buf)
+    {
+        buf = guest_eip_buf;
+    }
+    else
+    {
+        if ( inst_copy_from_guest(buffer, svm_rip2pointer(v), MAX_INST_LEN)
+             != MAX_INST_LEN )
+            return 0;
+        buf = buffer;
+    }
 
     for (j = 0; j < list_count; j++)
     {
@@ -466,16 +450,7 @@ int __get_instruction_length_from_list(s
         while (inst_len < MAX_INST_LEN && 
                 is_prefix(buf[inst_len]) && 
                 !is_prefix(opcode[1]))
-        {
             inst_len++;
-            if ( inst_len >= fetch_len ) 
-            { 
-                FETCH(buf + fetch_len, 
-                      fetch_addr + fetch_len, 
-                      MAX_INST_LEN - fetch_len);
-                fetch_len = MAX_INST_LEN;
-            }
-        }
 
         ASSERT(opcode[0] <= 15);    /* Make sure the table is correct. */
         found = 1;
@@ -486,14 +461,6 @@ int __get_instruction_length_from_list(s
             if (i == opcode[0]-1 && opcode[i+1] == 0)
                 break;
 
-            if ( inst_len + i >= fetch_len ) 
-            { 
-                FETCH(buf + fetch_len, 
-                      fetch_addr + fetch_len, 
-                      MAX_INST_LEN - fetch_len);
-                fetch_len = MAX_INST_LEN;
-            }
-
             if (buf[inst_len+i] != opcode[i+1])
             {
                 found = 0;
@@ -520,11 +487,7 @@ int __get_instruction_length_from_list(s
 
     printk("%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;
-
-#undef FETCH
-
 }
 
 /*
diff -r 784805591fa2 -r f70475e8396d xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c	Tue May 13 15:54:31 2008 +0100
+++ b/xen/arch/x86/hvm/svm/svm.c	Tue May 13 15:34:33 2008 +0100
@@ -78,10 +78,7 @@ static void inline __update_guest_eip(
 {
     struct vcpu *curr = current;
 
-    if ( unlikely(inst_len == 0) )
-        return;
-
-    if ( unlikely(inst_len > 15) )
+    if ( unlikely((inst_len == 0) || (inst_len > 15)) )
     {
         gdprintk(XENLOG_ERR, "Bad instruction length %u\n", inst_len);
         domain_crash(curr->domain);
@@ -1134,28 +1131,18 @@ static void svm_dr_access(struct vcpu *v
 
 static int svm_get_prefix_info(struct vcpu *v, unsigned int dir, 
                                 svm_segment_register_t **seg, 
-                                unsigned int *asize,
-                                unsigned int isize)
+                                unsigned int *asize)
 {
     struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
     unsigned char inst[MAX_INST_LEN];
     int i;
 
     memset(inst, 0, MAX_INST_LEN);
-    
-    switch ( hvm_fetch_from_guest_virt(inst, svm_rip2pointer(v), isize) )
+    if (inst_copy_from_guest(inst, svm_rip2pointer(v), sizeof(inst)) 
+        != MAX_INST_LEN) 
     {
-        case HVMCOPY_okay:
-            break;
-        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:
-            gdprintk(XENLOG_ERR, "Bad prefix fetch at %#lx (%#lx)\n",
-                     (unsigned long) guest_cpu_user_regs()->eip,
-                     svm_rip2pointer(v));
-            domain_crash(v->domain);
-            return 0;
+        gdprintk(XENLOG_ERR, "get guest instruction failed\n");
+        return 0;
     }
 
     for (i = 0; i < MAX_INST_LEN; i++)
@@ -1245,8 +1232,12 @@ static int svm_get_io_address(
      * to figure out what it is...
      */
     isize = vmcb->exitinfo2 - regs->eip;
-    if ( isize > ((info.fields.rep) ? 2 : 1) ) 
-        if ( !svm_get_prefix_info(v, info.fields.type, &seg, &asize, isize) )
+
+    if (info.fields.rep)
+        isize --;
+
+    if (isize > 1) 
+        if ( !svm_get_prefix_info(v, info.fields.type, &seg, &asize) )
             return 0;
 
     if (info.fields.type == IOREQ_WRITE)
@@ -1602,24 +1593,30 @@ static void svm_cr_access(
     enum instruction_index list_b[] = {INSTR_MOVCR2, INSTR_SMSW};
     enum instruction_index match;
 
+    if ( inst_copy_from_guest(buffer, svm_rip2pointer(v), sizeof(buffer))
+         != sizeof buffer )
+        /* #PF will have been delivered if appropriate. */
+        return;
 
+    /* get index to first actual instruction byte - as we will need to know 
+       where the prefix lives later on */
+    index = skip_prefix_bytes(buffer, sizeof(buffer));
+    
     if ( type == TYPE_MOV_TO_CR )
     {
         inst_len = __get_instruction_length_from_list(
-            v, list_a, ARRAY_SIZE(list_a), buffer, &match);
+            v, list_a, ARRAY_SIZE(list_a), &buffer[index], &match);
     }
     else /* type == TYPE_MOV_FROM_CR */
     {
         inst_len = __get_instruction_length_from_list(
-            v, list_b, ARRAY_SIZE(list_b), buffer, &match);
+            v, list_b, ARRAY_SIZE(list_b), &buffer[index], &match);
     }
 
     if ( inst_len == 0 )
         return;
 
-    /* get index to first actual instruction byte - as we will need to know 
-       where the prefix lives later on */
-    index = skip_prefix_bytes(buffer, sizeof(buffer));
+    inst_len += index;
 
     /* Check for REX prefix - it's ALWAYS the last byte of any prefix bytes */
     if (index > 0 && (buffer[index-1] & 0xF0) == 0x40)
@@ -1944,6 +1941,16 @@ void svm_handle_invlpg(const short invlp
     unsigned long g_vaddr;
     int inst_len;
 
+    /* 
+     * Unknown how many bytes the invlpg instruction will take.  Use the
+     * maximum instruction length here
+     */
+    if ( inst_copy_from_guest(opcode, svm_rip2pointer(v), length) < length )
+    {
+        gdprintk(XENLOG_ERR, "Error reading memory %d bytes\n", length);
+        return;
+    }
+
     if ( invlpga )
     {
         inst_len = __get_instruction_length(v, INSTR_INVLPGA, opcode);
@@ -1958,8 +1965,8 @@ void svm_handle_invlpg(const short invlp
     else
     {
         /* What about multiple prefix codes? */
+        prefix = (is_prefix(opcode[0]) ? opcode[0] : 0);
         inst_len = __get_instruction_length(v, INSTR_INVLPG, opcode);
-        prefix = (is_prefix(opcode[0]) ? opcode[0] : 0);
         if ( inst_len <= 0 )
         {
             gdprintk(XENLOG_ERR, "Error getting invlpg instr len\n");

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2009-10-01  9:34 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-01  9:34 [PATCH] Xen 3.2: Fix guest boot on AMD K8 machine Christoph Egger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.