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

[PATCH 3/6] x86: don't assume #DB is always caused by singlestep if EFLAGS.TF is set



Today, when a HVM (or PVH) guest triggers a hardware breakpoint while
EFLAGS.TF is set, Xen incorrectly assumes that this is a single stepping
exception and sets DR_STEP in dr6 in addition to DR_TRAP<n>.

This causes problems with Linux HW breakpoint handler, which ignores
DR_TRAP<n> bits when DR_STEP is set.  This prevents user-mode debuggers
from recognizing hardware breakpoints if EFLAGS.TF is set.

Fix this by not setting DR_STEP in {vmx,svm}_inject_event, unless the
emulator explicitly signals the single-stepping mode via the newly added
"singlestep" boolean field of struct x86_event.

Fixes: 8b831f4189 ("x86: single step after instruction emulation")
Signed-off-by: Jinoh Kang <jinoh.kang.kr@xxxxxxxxx>
---
 xen/arch/x86/hvm/emulate.c             |  3 ++-
 xen/arch/x86/hvm/svm/svm.c             |  6 +++---
 xen/arch/x86/hvm/vmx/vmx.c             |  6 +++---
 xen/arch/x86/include/asm/hvm/hvm.h     | 12 ++++++++++++
 xen/arch/x86/mm/shadow/multi.c         |  5 +++--
 xen/arch/x86/x86_emulate/x86_emulate.h |  4 +++-
 6 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 9b6e4c8bc61b..5ad372466e1d 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -26,6 +26,7 @@
 #include <asm/hvm/support.h>
 #include <asm/iocap.h>
 #include <asm/vm_event.h>
+#include <asm/debugreg.h>
 
 struct hvmemul_cache
 {
@@ -2673,7 +2674,7 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt 
*hvmemul_ctxt,
     }
 
     if ( hvmemul_ctxt->ctxt.retire.singlestep )
-        hvm_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC);
+        hvm_inject_debug_exception(DR_STEP);
 
     new_intr_shadow = hvmemul_ctxt->intr_shadow;
 
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index d5e8cb0722ca..f25d6a53f092 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -96,7 +96,7 @@ void __update_guest_eip(struct cpu_user_regs *regs, unsigned 
int inst_len)
     curr->arch.hvm.svm.vmcb->int_stat.intr_shadow = 0;
 
     if ( regs->eflags & X86_EFLAGS_TF )
-        hvm_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC);
+        hvm_inject_debug_exception(DR_STEP);
 }
 
 static void cf_check svm_cpu_down(void)
@@ -1328,10 +1328,10 @@ static void cf_check svm_inject_event(const struct 
x86_event *event)
     switch ( _event.vector | -(_event.type == X86_EVENTTYPE_SW_INTERRUPT) )
     {
     case X86_EXC_DB:
-        if ( regs->eflags & X86_EFLAGS_TF )
+        if ( event->extra )
         {
             __restore_debug_registers(vmcb, curr);
-            vmcb_set_dr6(vmcb, vmcb_get_dr6(vmcb) | DR_STEP);
+            vmcb_set_dr6(vmcb, vmcb_get_dr6(vmcb) | event->extra);
         }
         /* fall through */
     case X86_EXC_BP:
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 8823ca13e55d..1795b9479cf9 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2022,10 +2022,10 @@ static void cf_check vmx_inject_event(const struct 
x86_event *event)
     switch ( _event.vector | -(_event.type == X86_EVENTTYPE_SW_INTERRUPT) )
     {
     case X86_EXC_DB:
-        if ( guest_cpu_user_regs()->eflags & X86_EFLAGS_TF )
+        if ( event->extra )
         {
             __restore_debug_registers(curr);
-            write_debugreg(6, read_debugreg(6) | DR_STEP);
+            write_debugreg(6, read_debugreg(6) | event->extra);
         }
         if ( !nestedhvm_vcpu_in_guestmode(curr) ||
              !nvmx_intercepts_exception(curr, X86_EXC_DB, _event.error_code) )
@@ -3068,7 +3068,7 @@ void update_guest_eip(void)
     }
 
     if ( regs->eflags & X86_EFLAGS_TF )
-        hvm_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC);
+        hvm_inject_debug_exception(DR_STEP);
 }
 
 static void cf_check vmx_fpu_dirty_intercept(void)
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h 
b/xen/arch/x86/include/asm/hvm/hvm.h
index f3f6310ab684..6a0b9e3ff01e 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -538,6 +538,18 @@ static inline void hvm_inject_page_fault(int errcode, 
unsigned long cr2)
     hvm_inject_event(&event);
 }
 
+static inline void hvm_inject_debug_exception(unsigned long pending_dbg)
+{
+    struct x86_event event = {
+        .vector = X86_EXC_DB,
+        .type = X86_EVENTTYPE_HW_EXCEPTION,
+        .error_code = X86_EVENT_NO_EC,
+        .extra = pending_dbg,
+    };
+
+    hvm_inject_event(&event);
+}
+
 static inline bool hvm_event_pending(const struct vcpu *v)
 {
     return alternative_call(hvm_funcs.event_pending, v);
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index cf74fdf5dda6..365af5169750 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -24,6 +24,7 @@
 #include <asm/hvm/cacheattr.h>
 #include <asm/mtrr.h>
 #include <asm/guest_pt.h>
+#include <asm/debugreg.h>
 #include <public/sched.h>
 #include "private.h"
 #include "types.h"
@@ -2788,7 +2789,7 @@ static int cf_check sh_page_fault(
 #endif
 
     if ( emul_ctxt.ctxt.retire.singlestep )
-        hvm_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC);
+        hvm_inject_debug_exception(DR_STEP);
 
 #if GUEST_PAGING_LEVELS == 3 /* PAE guest */
     /*
@@ -2829,7 +2830,7 @@ static int cf_check sh_page_fault(
                 TRACE_SHADOW_PATH_FLAG(TRCE_SFLAG_EMULATION_LAST_FAILED);
 
                 if ( emul_ctxt.ctxt.retire.singlestep )
-                    hvm_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC);
+                    hvm_inject_debug_exception(DR_STEP);
 
                 break; /* Don't emulate again if we failed! */
             }
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h 
b/xen/arch/x86/x86_emulate/x86_emulate.h
index bad957f9bcb2..868a64ab20e6 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -78,7 +78,9 @@ struct x86_event {
     uint8_t       type;         /* X86_EVENTTYPE_* */
     uint8_t       insn_len;     /* Instruction length */
     int32_t       error_code;   /* X86_EVENT_NO_EC if n/a */
-    unsigned long extra;        /* CR2 if X86_EXC_PF h/w exception */
+
+    /* Type-specific extra data (%cr2 for #PF, pending_dbg for #DB) */
+    unsigned long extra;
 };
 
 /*
-- 
2.41.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.