|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH] x86/debug: Avoid crashing in early boot because of debugger_trap_entry()
debugger_trap_entry() is not safe to use during early boot, as it follows
current before it is necesserily safe to do so. Futhermore it does this
unconditionally, despite most callsites turning into no-ops because of the
vector test.
Inline debugger_trap_entry() into the two callers where doesn't become a
no-op, and reposition the guest_kernel_mode() test to be after the
guest_mode() test, avoiding the reference to current if the exception occured
from hypervisor context. This makes the exception handlers safe in early
boot. The repositioning also has bugfix in the TRAP_debug case, where dr6
will be correct in the debuggers view of state.
This causes the deletion of DEBUGGER_trap_entry(), which is a good thing as
hiding a return statement in a function-like macro is very antisocial
programming.
While cleaning this area up, drop the DEBUGGER_trap_fatal() wrapper as well,
making the code flow more apparent, and switch debugger_trap_fatal()'s return
type to boolean to match its semantics.
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
---
xen/arch/x86/traps.c | 54 ++++++++++++++++++++++++++++--------------
xen/include/asm-x86/debugger.h | 29 +++--------------------
2 files changed, 39 insertions(+), 44 deletions(-)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 767d0b0..933435d 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -736,7 +736,9 @@ void do_reserved_trap(struct cpu_user_regs *regs)
{
unsigned int trapnr = regs->entry_vector;
- DEBUGGER_trap_fatal(trapnr, regs);
+ if ( debugger_trap_fatal(trapnr, regs) )
+ return;
+
show_execution_state(regs);
panic("FATAL RESERVED TRAP %#x: %s", trapnr, trapstr(trapnr));
}
@@ -750,8 +752,6 @@ static void do_trap(struct cpu_user_regs *regs, int
use_error_code)
if ( regs->error_code & X86_XEC_EXT )
goto hardware_trap;
- DEBUGGER_trap_entry(trapnr, regs);
-
if ( guest_mode(regs) )
{
do_guest_trap(trapnr, regs, use_error_code);
@@ -777,7 +777,8 @@ static void do_trap(struct cpu_user_regs *regs, int
use_error_code)
}
hardware_trap:
- DEBUGGER_trap_fatal(trapnr, regs);
+ if ( debugger_trap_fatal(trapnr, regs) )
+ return;
show_execution_state(regs);
panic("FATAL TRAP: vector = %d (%s)\n"
@@ -1307,8 +1308,6 @@ void do_invalid_op(struct cpu_user_regs *regs)
int id = -1, lineno;
const struct virtual_region *region;
- DEBUGGER_trap_entry(TRAP_invalid_op, regs);
-
if ( likely(guest_mode(regs)) )
{
if ( !emulate_invalid_rdtscp(regs) &&
@@ -1377,7 +1376,10 @@ void do_invalid_op(struct cpu_user_regs *regs)
case BUGFRAME_bug:
printk("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
- DEBUGGER_trap_fatal(TRAP_invalid_op, regs);
+
+ if ( debugger_trap_fatal(TRAP_invalid_op, regs) )
+ return;
+
show_execution_state(regs);
panic("Xen BUG at %s%s:%d", prefix, filename, lineno);
@@ -1389,7 +1391,10 @@ void do_invalid_op(struct cpu_user_regs *regs)
printk("Assertion '%s' failed at %s%s:%d\n",
predicate, prefix, filename, lineno);
- DEBUGGER_trap_fatal(TRAP_invalid_op, regs);
+
+ if ( debugger_trap_fatal(TRAP_invalid_op, regs) )
+ return;
+
show_execution_state(regs);
panic("Assertion '%s' failed at %s%s:%d",
predicate, prefix, filename, lineno);
@@ -1402,14 +1407,17 @@ void do_invalid_op(struct cpu_user_regs *regs)
regs->eip = fixup;
return;
}
- DEBUGGER_trap_fatal(TRAP_invalid_op, regs);
+
+ if ( debugger_trap_fatal(TRAP_invalid_op, regs) )
+ return;
+
show_execution_state(regs);
panic("FATAL TRAP: vector = %d (invalid opcode)", TRAP_invalid_op);
}
void do_int3(struct cpu_user_regs *regs)
{
- DEBUGGER_trap_entry(TRAP_int3, regs);
+ struct vcpu *curr = current;
if ( !guest_mode(regs) )
{
@@ -1417,6 +1425,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;
+ }
+
do_guest_trap(TRAP_int3, regs, 0);
}
@@ -1744,8 +1759,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;
- DEBUGGER_trap_entry(TRAP_page_fault, regs);
-
perfc_incr(page_faults);
if ( unlikely(fixup_page_fault(addr, regs) != 0) )
@@ -1774,7 +1787,8 @@ void do_page_fault(struct cpu_user_regs *regs)
return;
}
- DEBUGGER_trap_fatal(TRAP_page_fault, regs);
+ if ( debugger_trap_fatal(TRAP_page_fault, regs) )
+ return;
show_execution_state(regs);
show_page_walk(addr);
@@ -3471,8 +3485,6 @@ void do_general_protection(struct cpu_user_regs *regs)
struct vcpu *v = current;
unsigned long fixup;
- DEBUGGER_trap_entry(TRAP_gp_fault, regs);
-
if ( regs->error_code & X86_XEC_EXT )
goto hardware_gp;
@@ -3543,7 +3555,8 @@ void do_general_protection(struct cpu_user_regs *regs)
}
hardware_gp:
- DEBUGGER_trap_fatal(TRAP_gp_fault, regs);
+ if ( debugger_trap_fatal(TRAP_gp_fault, regs) )
+ return;
show_execution_state(regs);
panic("GENERAL PROTECTION FAULT\n[error_code=%04x]", regs->error_code);
@@ -3778,8 +3791,6 @@ void do_debug(struct cpu_user_regs *regs)
{
struct vcpu *v = current;
- DEBUGGER_trap_entry(TRAP_debug, regs);
-
if ( !guest_mode(regs) )
{
if ( regs->eflags & X86_EFLAGS_TF )
@@ -3814,6 +3825,13 @@ void do_debug(struct cpu_user_regs *regs)
/* Save debug status register where guest OS can peek at it */
v->arch.debugreg[6] = read_debugreg(6);
+ if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached )
+ {
+ v->arch.gdbsx_vcpu_event = TRAP_debug;
+ domain_pause_for_debugger();
+ return;
+ }
+
ler_enable();
do_guest_trap(TRAP_debug, regs, 0);
return;
diff --git a/xen/include/asm-x86/debugger.h b/xen/include/asm-x86/debugger.h
index 271cbaa..0f95fe9 100644
--- a/xen/include/asm-x86/debugger.h
+++ b/xen/include/asm-x86/debugger.h
@@ -33,17 +33,11 @@
#include <asm/regs.h>
#include <asm/processor.h>
-/* The main trap handlers use these helper macros which include early bail. */
-#define DEBUGGER_trap_entry(_v, _r) \
- if ( debugger_trap_entry(_v, _r) ) return;
-#define DEBUGGER_trap_fatal(_v, _r) \
- if ( debugger_trap_fatal(_v, _r) ) return;
-
#ifdef CONFIG_CRASH_DEBUG
#include <xen/gdbstub.h>
-static inline int debugger_trap_fatal(
+static inline bool debugger_trap_fatal(
unsigned int vector, struct cpu_user_regs *regs)
{
int rc = __trap_to_gdb(regs, vector);
@@ -55,33 +49,16 @@ static inline int debugger_trap_fatal(
#else
-static inline int debugger_trap_fatal(
+static inline bool debugger_trap_fatal(
unsigned int vector, struct cpu_user_regs *regs)
{
- return 0;
+ return false;
}
#define debugger_trap_immediate() ((void)0)
#endif
-static inline int debugger_trap_entry(
- unsigned int vector, struct cpu_user_regs *regs)
-{
- struct vcpu *v = current;
-
- if ( guest_kernel_mode(v, regs) && v->domain->debugger_attached &&
- ((vector == TRAP_int3) || (vector == TRAP_debug)) )
- {
- if ( vector != TRAP_debug ) /* domain pause is good enough */
- current->arch.gdbsx_vcpu_event = vector;
- domain_pause_for_debugger();
- return 1;
- }
-
- return 0;
-}
-
unsigned int dbg_rw_mem(void * __user addr, void * __user buf,
unsigned int len, domid_t domid, bool_t toaddr,
uint64_t pgd3);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |