@@ -2541,7 +2541,7 @@ __initcall(init_vcpu_kick_softirq);
void domain_pause_for_debugger(void)
{
-#ifdef CONFIG_CRASH_DEBUG
+#ifdef CONFIG_GDBSX
struct vcpu *curr = current;
struct domain *d = curr->domain;
@@ -858,13 +858,20 @@ static void do_trap(struct cpu_user_regs *regs)
if ( regs->error_code & X86_XEC_EXT )
goto hardware_trap;
- if ( debugger_trap_entry(trapnr, regs) )
- return;
-
ASSERT(trapnr < 32);
if ( guest_mode(regs) )
{
+ struct vcpu *curr = current;
+ if ( (trapnr == TRAP_debug || trapnr == TRAP_int3) &&
+ guest_kernel_mode(curr, regs) &&
+ curr->domain->debugger_attached )
+ {
+ if ( trapnr != TRAP_debug )
+ curr->arch.gdbsx_vcpu_event = trapnr;
+ domain_pause_for_debugger();
+ return;
+ }
pv_inject_hw_exception(trapnr,
(TRAP_HAVE_EC & (1u << trapnr))
? regs->error_code : X86_EVENT_NO_EC);
@@ -1094,9 +1101,6 @@ void do_invalid_op(struct cpu_user_regs *regs)
int id = -1, lineno;
const struct virtual_region *region;
- if ( debugger_trap_entry(TRAP_invalid_op, regs) )
- return;
-
if ( likely(guest_mode(regs)) )
{
if ( pv_emulate_invalid_op(regs) )
@@ -1201,8 +1205,7 @@ void do_invalid_op(struct cpu_user_regs *regs)
void do_int3(struct cpu_user_regs *regs)
{
- if ( debugger_trap_entry(TRAP_int3, regs) )
- return;
+ struct vcpu *curr = current;
if ( !guest_mode(regs) )
{
@@ -1216,6 +1219,13 @@ void do_int3(struct cpu_user_regs *regs)
return;
}
+ if ( guest_kernel_mode(curr, regs) && curr->domain->debugger_attached )
+ {
+ curr->arch.gdbsx_vcpu_event = TRAP_int3;
+ domain_pause_for_debugger();
+ return;
+ }
+
pv_inject_hw_exception(TRAP_int3, X86_EVENT_NO_EC);
}
@@ -1492,9 +1502,6 @@ void do_page_fault(struct cpu_user_regs *regs)
/* fixup_page_fault() might change regs->error_code, so cache it here. */
error_code = regs->error_code;
- if ( debugger_trap_entry(TRAP_page_fault, regs) )
- return;
-
perfc_incr(page_faults);
/* Any shadow stack access fault is a bug in Xen. */
@@ -1593,9 +1600,6 @@ void do_general_protection(struct cpu_user_regs *regs)
struct vcpu *v = current;
#endif
- if ( debugger_trap_entry(TRAP_gp_fault, regs) )
- return;
-
if ( regs->error_code & X86_XEC_EXT )
goto hardware_gp;
@@ -1888,9 +1892,6 @@ void do_debug(struct cpu_user_regs *regs)
/* Stash dr6 as early as possible. */
dr6 = read_debugreg(6);
- if ( debugger_trap_entry(TRAP_debug, regs) )
- return;
-
/*
* At the time of writing (March 2018), on the subject of %dr6:
*
@@ -1995,6 +1996,12 @@ void do_debug(struct cpu_user_regs *regs)
return;
}
+ if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached )
+ {
+ domain_pause_for_debugger();
+ return;
+ }
+
/* Save debug status register where guest OS can peek at it */
v->arch.dr6 |= (dr6 & ~X86_DR6_DEFAULT);
v->arch.dr6 &= (dr6 | ~X86_DR6_DEFAULT);
@@ -2014,9 +2021,6 @@ void do_entry_CP(struct cpu_user_regs *regs)
const char *err = "??";
unsigned int ec = regs->error_code;
- if ( debugger_trap_entry(TRAP_debug, regs) )
- return;
-
/* Decode ec if possible */
if ( ec < ARRAY_SIZE(errors) && errors[ec][0] )
err = errors[ec];
@@ -2028,6 +2032,12 @@ void do_entry_CP(struct cpu_user_regs *regs)
*/
if ( guest_mode(regs) )
{
+ struct vcpu *curr = current;
+ if ( guest_kernel_mode(curr, regs) && curr->domain->debugger_attached )
+ {
+ domain_pause_for_debugger();
+ return;
+ }
gprintk(XENLOG_ERR, "Hit #CP[%04x] in guest context %04x:%p\n",
ec, regs->cs, _p(regs->rip));
ASSERT_UNREACHABLE();
@@ -5,19 +5,12 @@
*
* Each debugger should define two functions here:
*
- * 1. debugger_trap_entry():
- * Called at start of any synchronous fault or trap, before any other work
- * is done. The idea is that if your debugger deliberately caused the trap
- * (e.g. to implement breakpoints or data watchpoints) then you can take
- * appropriate action and return a non-zero value to cause early exit from
- * the trap function.
- *
- * 2. debugger_trap_fatal():
+ * 1. debugger_trap_fatal():
* Called when Xen is about to give up and crash. Typically you will use this
* hook to drop into a debug session. It can also be used to hook off
* deliberately caused traps (which you then handle and return non-zero).
*
- * 3. debugger_trap_immediate():
+ * 2. debugger_trap_immediate():
* Called if we want to drop into a debugger now. This is essentially the
* same as debugger_trap_fatal, except that we use the current register state
* rather than the state which was in effect when we took the trap.
@@ -49,31 +42,6 @@ static inline bool debugger_trap_fatal(
/* Int3 is a trivial way to gather cpu_user_regs context. */
#define debugger_trap_immediate() __asm__ __volatile__ ( "int3" );
-static inline bool debugger_trap_entry(
- unsigned int vector, struct cpu_user_regs *regs)
-{
- /*
- * This function is called before any checks are made. Amongst other
- * things, be aware that during early boot, current is not a safe pointer
- * to follow.
- */
- struct vcpu *v = current;
-
- if ( vector != TRAP_int3 && vector != TRAP_debug )
- return false;
-
- if ( guest_mode(regs) && guest_kernel_mode(v, regs) &&
- v->domain->debugger_attached )
- {
- if ( vector != TRAP_debug ) /* domain pause is good enough */
- current->arch.gdbsx_vcpu_event = vector;
- domain_pause_for_debugger();
- return true;
- }
-
- return false;
-}
-
#else
static inline bool debugger_trap_fatal(
@@ -84,12 +52,6 @@ static inline bool debugger_trap_fatal(
#define debugger_trap_immediate() ((void)0)
-static inline bool debugger_trap_entry(
- unsigned int vector, struct cpu_user_regs *regs)
-{
- return false;
-}
-
#endif
#ifdef CONFIG_GDBSX
The functions debugger_trap_fatal(), debugger_trap_immediate(), and debugger_trap_entry() are generic hook functions for debugger support. In practice, debugger_trap_fatal() and debugger_trap_immediate() are only used in the debugging of Xen itself and debugger_trap_entry() is only used in the debugging of guests. That is, debugger_trap_entry() is part of gdbsx functionality and not the Xen gdstub. This is evidenced by debugger_trap_fatal()'s usage of domain_pause_for_debugger(). Because of this, debugger_trap_entry() many not belong alongside the Xen debug functions. This commit fixes this by expanding inline debugger_trap_entry() into its usage sites in x86/traps.c and stubbing out domain_pause_for_debugger() when !CONFIG_GDBSX. Placing what was debugger_trap_entry() under the scope of gdbsx instead of gdbstub. The function calls that caused an effective no-op and early exit out of debugger_trap_entry() are removed completely (when the trapnr is not int3/debug). This commit is one of a series geared towards removing the unnecessary requirement that all architectures to implement <asm/debugger.h>. Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com> --- Changes in v4: - Reword commit message for accuracy (make weaker claims) - Fix "if { return } else if { return }" anti-pattern xen/arch/x86/domain.c | 2 +- xen/arch/x86/traps.c | 50 ++++++++++++++++++++-------------- xen/include/asm-x86/debugger.h | 42 ++-------------------------- 3 files changed, 33 insertions(+), 61 deletions(-)