Clean up SVM instruction-fetch code some more

diff -r 483d006cc607 -r c20b8931b598 xen/arch/x86/hvm/svm/emulate.c
@@ -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)
 {

@@ -101,29 +89,47 @@ 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_ERR, "Bad instruction fetch at %#lx (%#lx)\n",
+                 (unsigned long) guest_cpu_user_regs()->eip, addr);
+        domain_crash(v->domain);
+        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;
-
-    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;
-    }
+    unsigned long fetch_addr;
+    int fetch_len;
+
+    /* 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;
+    if ( !fetch(v, buf, fetch_addr, fetch_len) )
+        return 0;
 
     for ( j = 0; j < list_count; j++ )
     {

@@ -134,17 +140,36 @@ 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 ) 
+            { 
+                if ( !fetch(v, buf + fetch_len,
+                            fetch_addr + fetch_len,
+                            MAX_INST_LEN - fetch_len) )
+                    return 0;
+                fetch_len = MAX_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;

@@ -158,13 +183,12 @@ int __get_instruction_length_from_list(s
 
     printk("%s: Mismatch between expected and actual instruction bytes: "
             "eip = %lx\n",  __func__, (unsigned long)vmcb->rip);
+    domain_crash(v->domain);
     return 0;
 
  done:
     inst_len += opcode[0];
     ASSERT(inst_len <= MAX_INST_LEN);
-    if ( match )
-        *match = instr;
     return inst_len;
 }
 

diff -r 483d006cc607 -r c20b8931b598 xen/arch/x86/hvm/svm/svm.c
@@ -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,7 +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);
+    inst_len = __get_instruction_length(current, INSTR_CPUID);
     if ( inst_len == 0 ) 
         return;
 

@@ -1084,12 +1087,12 @@ static void svm_do_msr_access(struct cpu
     if ( vmcb->exitinfo1 == 0 )
     {
         rc = hvm_msr_read_intercept(regs);
-        inst_len = __get_instruction_length(v, INSTR_RDMSR, NULL);
+        inst_len = __get_instruction_length(v, INSTR_RDMSR);
     }
     else
     {
         rc = hvm_msr_write_intercept(regs);
-        inst_len = __get_instruction_length(v, INSTR_WRMSR, NULL);
+        inst_len = __get_instruction_length(v, INSTR_WRMSR);
     }
 
     if ( rc == X86EMUL_OKAY )

@@ -1103,7 +1106,7 @@ static void svm_vmexit_do_hlt(struct vmc
     struct hvm_intack intack = hvm_vcpu_has_pending_irq(curr);
     unsigned int inst_len;
 
-    inst_len = __get_instruction_length(curr, INSTR_HLT, NULL);
+    inst_len = __get_instruction_length(curr, INSTR_HLT);
     if ( inst_len == 0 )
         return;
     __update_guest_eip(regs, inst_len);

@@ -1140,7 +1143,7 @@ static void svm_vmexit_do_invalidate_cac
     svm_wbinvd_intercept();
 
     inst_len = __get_instruction_length_from_list(
-        current, list, ARRAY_SIZE(list), NULL, NULL);
+        current, list, ARRAY_SIZE(list));
     __update_guest_eip(regs, inst_len);
 }
 

@@ -1216,7 +1219,7 @@ 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);
+        inst_len = __get_instruction_length(v, INSTR_INT3);
         __update_guest_eip(regs, inst_len);
         domain_pause_for_debugger();
         break;

@@ -1293,7 +1296,7 @@ asmlinkage void svm_vmexit_handler(struc
         break;
 
     case VMEXIT_VMMCALL:
-        inst_len = __get_instruction_length(v, INSTR_VMCALL, NULL);
+        inst_len = __get_instruction_length(v, INSTR_VMCALL);
         if ( inst_len == 0 )
             break;
         HVMTRACE_1D(VMMCALL, v, regs->eax);

diff -r 483d006cc607 -r c20b8931b598 xen/include/asm-x86/hvm/svm/emulate.h
@@ -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)
+                                           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__ */