svm: Fix __update_guest_eip() to clear interrupt shadow.

Get rid of assertions about return value of get_instruction_length()
-- instead test in __update_guest_eip() and crash the domain.
Cache value of 'current' in svm_do_hlt().

The mismanagement of the interrupt shadow was found by Christoph
Egger of AMD.

Signed-off-by: Keir Fraser <keir@xensource.com>
xen-unstable changeset:   16398:bc6aaa44e296c0d905daf57ebe268b32faa58376
xen-unstable date:        Tue Nov 20 15:05:36 2007 +0000

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 9967d7c43957ebd5d2d931681fb0b14461be0e0b..3493dcd405adde04fb99c55a19c094130877c537 100644 (file)
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -69,6 +69,22 @@ static void *root_vmcb[NR_CPUS] __read_mostly;
 /* hardware assisted paging bits */
 extern int opt_hap_enabled;
 
+static void inline __update_guest_eip(
+    struct vmcb_struct *vmcb, unsigned int inst_len)
+{
+    if ( unlikely((inst_len == 0) || (inst_len > 15)) )
+    {
+        gdprintk(XENLOG_ERR, "Bad instruction length %u\n", inst_len);
+        domain_crash(current->domain);
+        return;
+    }
+
+    vmcb->rip += inst_len;
+    vmcb->rflags &= ~X86_EFLAGS_RF;
+
+    current->arch.hvm_svm.vmcb->interrupt_shadow = 0;
+}
+ 
 static void svm_inject_exception(struct vcpu *v, int trap, 
                                         int ev, int error_code)
 {
@@ -1171,7 +1187,6 @@ static void svm_vmexit_do_cpuid(struct vmcb_struct *vmcb,
                 ((uint64_t)eax << 32) | ebx, ((uint64_t)ecx << 32) | edx);
 
     inst_len = __get_instruction_length(v, INSTR_CPUID, NULL);
-    ASSERT(inst_len > 0);
     __update_guest_eip(vmcb, inst_len);
 }
 
@@ -2001,8 +2016,6 @@ static int svm_cr_access(struct vcpu *v, unsigned int cr, unsigned int type,
             v, list_b, ARR_SIZE(list_b), &buffer[index], &match);
     }
 
-    ASSERT(inst_len > 0);
-
     inst_len += index;
 
     /* Check for REX prefix - it's ALWAYS the last byte of any prefix bytes */
@@ -2100,8 +2113,6 @@ static int svm_cr_access(struct vcpu *v, unsigned int cr, unsigned int type,
         BUG();
     }
 
-    ASSERT(inst_len);
-
     __update_guest_eip(vmcb, inst_len);
     
     return result;
@@ -2224,7 +2235,11 @@ static void svm_do_msr_access(
 
 static void svm_vmexit_do_hlt(struct vmcb_struct *vmcb)
 {
-    __update_guest_eip(vmcb, 1);
+    struct vcpu *curr = current;
+    unsigned int inst_len;
+
+    inst_len = __get_instruction_length(curr, INSTR_HLT, NULL);
+    __update_guest_eip(vmcb, inst_len);
 
     /* Check for interrupt not handled or new interrupt. */
     if ( (vmcb->rflags & X86_EFLAGS_IF) &&
@@ -2233,7 +2248,7 @@ static void svm_vmexit_do_hlt(struct vmcb_struct *vmcb)
         return;
     }
 
-    HVMTRACE_1D(HLT, current, /*int pending=*/ 0);
+    HVMTRACE_1D(HLT, curr, /*int pending=*/ 0);
     hvm_hlt(vmcb->rflags);
 }
 
@@ -2261,17 +2276,15 @@ void svm_handle_invlpg(const short invlpga, struct cpu_user_regs *regs)
      * 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)
+    if ( inst_copy_from_guest(opcode, svm_rip2pointer(v), length) < length )
     {
         gdprintk(XENLOG_ERR, "Error reading memory %d bytes\n", length);
-        domain_crash(v->domain);
-        return;
+        goto crash;
     }
 
     if (invlpga)
     {
         inst_len = __get_instruction_length(v, INSTR_INVLPGA, opcode);
-        ASSERT(inst_len > 0);
         __update_guest_eip(vmcb, inst_len);
 
         /* 
@@ -2285,7 +2298,11 @@ void svm_handle_invlpg(const short invlpga, struct cpu_user_regs *regs)
         /* What about multiple prefix codes? */
         prefix = (is_prefix(opcode[0])?opcode[0]:0);
         inst_len = __get_instruction_length(v, INSTR_INVLPG, opcode);
-        ASSERT(inst_len > 0);
+        if ( inst_len <= 0 )
+        {
+            gdprintk(XENLOG_ERR, "Error getting invlpg instr len\n");
+            goto crash;
+        }
 
         inst_len--;
         length -= inst_len;
@@ -2307,6 +2324,10 @@ void svm_handle_invlpg(const short invlpga, struct cpu_user_regs *regs)
     paging_invlpg(v, g_vaddr);
     /* signal invplg to ASID handler */
     svm_asid_g_invlpg (v, g_vaddr);
+    return;
+
+ crash:
+    domain_crash(v->domain);
 }
 
 
@@ -2518,7 +2539,6 @@ asmlinkage void svm_vmexit_handler(struct cpu_user_regs *regs)
 
     case VMEXIT_VMMCALL:
         inst_len = __get_instruction_length(v, INSTR_VMCALL, NULL);
-        ASSERT(inst_len > 0);
         HVMTRACE_1D(VMMCALL, v, regs->eax);
         rc = hvm_do_hypercall(regs);
         if ( rc != HVM_HCALL_preempted )

diff --git a/xen/include/asm-x86/hvm/svm/emulate.h b/xen/include/asm-x86/hvm/svm/emulate.h
index 20d67855bd748484d42d2b30cfa2a091f98cf573..dfc66bcaeafbeeff205d18c24cf9c9b91d9abed7 100644 (file)
--- a/xen/include/asm-x86/hvm/svm/emulate.h
+++ b/xen/include/asm-x86/hvm/svm/emulate.h
@@ -132,16 +132,6 @@ static inline int skip_prefix_bytes(u8 *buf, size_t size)
     return index;
 }
 
-
-
-static void inline __update_guest_eip(
-    struct vmcb_struct *vmcb, int inst_len) 
-{
-    ASSERT(inst_len > 0);
-    vmcb->rip += inst_len;
-    vmcb->rflags &= ~X86_EFLAGS_RF;
-}
-
 #endif /* __ASM_X86_HVM_SVM_EMULATE_H__ */
 
 /*