WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-changelog

[Xen-changelog] [xen-3.2-testing] x86 hvm: Fix guest boot on AMD K8 mach

To: xen-changelog@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-changelog] [xen-3.2-testing] x86 hvm: Fix guest boot on AMD K8 machine
From: "Xen patchbot-3.2-testing" <patchbot-3.2-testing@xxxxxxxxxxxxxxxxxxx>
Date: Thu, 01 Oct 2009 04:35:25 -0700
Delivery-date: Thu, 01 Oct 2009 04:37:03 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-changelog-request@lists.xensource.com?subject=help>
List-id: BK change log <xen-changelog.lists.xensource.com>
List-post: <mailto:xen-changelog@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=unsubscribe>
Reply-to: xen-devel@xxxxxxxxxxxxxxxxxxx
Sender: xen-changelog-bounces@xxxxxxxxxxxxxxxxxxx
# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1254396780 -3600
# Node ID 671e9863095cdb2fbb33ac1fe63bf05b4df677a9
# Parent  6fc6ca3c393aa3e9af0d0edba51263f8c952ea7b
x86 hvm: Fix guest boot on AMD K8 machine

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 message:
(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 reproducible on AMD K8 machines.

Signed-off-by: Christoph Egger <Christoph.Egger@xxxxxxx>
---
 xen/arch/x86/hvm/svm/emulate.c |   73 +++++++++----------------------------
 xen/arch/x86/hvm/svm/svm.c     |   79 ++++++++++++++++++++++-------------------
 2 files changed, 61 insertions(+), 91 deletions(-)

diff -r 6fc6ca3c393a -r 671e9863095c xen/arch/x86/hvm/svm/emulate.c
--- a/xen/arch/x86/hvm/svm/emulate.c    Mon Jan 05 11:11:53 2009 +0000
+++ b/xen/arch/x86/hvm/svm/emulate.c    Thu Oct 01 12:33:00 2009 +0100
@@ -28,6 +28,9 @@
 #include <asm/hvm/svm/vmcb.h>
 #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
@@ -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;
@@ -485,14 +460,6 @@ int __get_instruction_length_from_list(s
             /* 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 ) 
-            { 
-                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])
             {
@@ -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 6fc6ca3c393a -r 671e9863095c xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c        Mon Jan 05 11:11:53 2009 +0000
+++ b/xen/arch/x86/hvm/svm/svm.c        Thu Oct 01 12:33:00 2009 +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);
@@ -1148,28 +1145,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) )
-    {
-        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;
+    if (inst_copy_from_guest(inst, svm_rip2pointer(v), sizeof(inst)) 
+        != MAX_INST_LEN) 
+    {
+        gdprintk(XENLOG_ERR, "get guest instruction failed\n");
+        return 0;
     }
 
     for (i = 0; i < MAX_INST_LEN; i++)
@@ -1259,8 +1246,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)
@@ -1616,24 +1607,30 @@ static void svm_cr_access(
     enum instruction_index list_b[] = {INSTR_MOVCR2, INSTR_SMSW};
     enum instruction_index match;
 
-
-    if ( type == TYPE_MOV_TO_CR )
-    {
-        inst_len = __get_instruction_length_from_list(
-            v, list_a, ARRAY_SIZE(list_a), buffer, &match);
-    }
-    else /* type == TYPE_MOV_FROM_CR */
-    {
-        inst_len = __get_instruction_length_from_list(
-            v, list_b, ARRAY_SIZE(list_b), buffer, &match);
-    }
-
-    if ( inst_len == 0 )
+    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[index], &match);
+    }
+    else /* type == TYPE_MOV_FROM_CR */
+    {
+        inst_len = __get_instruction_length_from_list(
+            v, list_b, ARRAY_SIZE(list_b), &buffer[index], &match);
+    }
+
+    if ( inst_len == 0 )
+        return;
+
+    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)
@@ -1955,6 +1952,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);
@@ -1969,8 +1976,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");

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

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-changelog] [xen-3.2-testing] x86 hvm: Fix guest boot on AMD K8 machine, Xen patchbot-3.2-testing <=