avoid debugger_trap when we don't expect

RFC PATCH In review Akio Takebe 2 participants 5 messages Jan 30, 2008 – Jan 31, 2008

lore

Share & tools

lore

Akio Takebe

author

2008-01-30 05:03 UTC #0 lore raw

RFC PATCH 1/1 avoid debugger_trap when we don't expect

Hi,

This patch avoid debugger_trap when we don't setup a debugger.

kexec_crash() is called after debugger_trap_immediate(). Because we want to use the safe path at kexecing, we should avoid the debugger path.
And the trap is a needless work.

Signed-off-by Akio Takebe <[email protected]>

Best Regards,

Akio Takebe

# HG changeset patch
# User [email protected]
# Date 1201701338 -32400
# Node ID c60150a6af125c5c5a4685765da03a0a3e393b78 # Parent b79802517a75a23abb88eda900ff802810a6add6 This patch avoids debugger_trap when we don't expect it. kexec_crash() is called after debugger_trap_immediate(). Because we want to use the safe path at kexecing, we should avoid the debugger path.
And the trap is a needless work.

xen/common/gdbstub.c +2 -2

diff -r b79802517a75 -r c60150a6af12 xen/common/gdbstub.c
@@ -516,13 +516,13 @@ process_command(struct cpu_user_regs *re
     return resume;
 }
 
-static struct gdb_context
+struct gdb_context
 __gdb_ctx = {
     .serhnd  = -1,
     .running = ATOMIC_INIT(1),
     .signum  = 1
 };
-static struct gdb_context *gdb_ctx = &__gdb_ctx;
+struct gdb_context *gdb_ctx = &__gdb_ctx;
 
 static void
 gdbstub_console_puts(const char *str)

xen/include/asm-ia64/debugger.h +5 -1

diff -r b79802517a75 -r c60150a6af12 xen/include/asm-ia64/debugger.h
@@ -67,7 +67,11 @@ static inline int debugger_trap_fatal(
 
 #define ____debugger_trap_immediate(b) __asm__ __volatile__ ("break.m "#b"\n")
 #define __debugger_trap_immediate(b) ____debugger_trap_immediate(b)
-#define debugger_trap_immediate() __debugger_trap_immediate(CDB_BREAK_NUM)
+#define debugger_trap_immediate()					\
+do {									\
+    if ( gdb_ctx->serhnd >= 0 )						\
+        __debugger_trap_immediate(CDB_BREAK_NUM);			\
+} while (0)
 
 //XXX temporal work around
 #ifndef CONFIG_SMP

xen/include/asm-powerpc/debugger.h +5 -1

diff -r b79802517a75 -r c60150a6af12 xen/include/asm-powerpc/debugger.h
@@ -23,6 +23,9 @@
 #define _ASM_DEBUGGER_H_
 
 #include <public/xen.h>
+#ifdef CRASH_DEBUG
+#include <xen/gdbstub.h>
+#endif
 
 extern void show_backtrace_regs(struct cpu_user_regs *);
 extern void show_backtrace(ulong sp, ulong lr, ulong pc);

@@ -56,7 +59,8 @@ static inline void debugger_trap_immedia
 {
     dump_all_execution_state();
 #ifdef CRASH_DEBUG
-    __builtin_trap();
+    if ( gdb_ctx->serhnd >= 0 )
+        __builtin_trap();
 #endif
 }

xen/include/asm-x86/debugger.h +5 -1

diff -r b79802517a75 -r c60150a6af12 xen/include/asm-x86/debugger.h
@@ -51,7 +51,11 @@ static inline int debugger_trap_fatal(
 }
 
 /* Int3 is a trivial way to gather cpu_user_regs context. */
-#define debugger_trap_immediate() __asm__ __volatile__ ( "int3" );
+#define debugger_trap_immediate()					\
+do {									\
+    if ( gdb_ctx->serhnd >= 0 )						\
+        __asm__ __volatile__ ( "int3" );				\
+} while (0)
 
 #else

xen/include/xen/gdbstub.h +1 -0

diff -r b79802517a75 -r c60150a6af12 xen/include/xen/gdbstub.h
@@ -47,6 +47,7 @@ struct gdb_context {
     unsigned long       out_offset;
     u8                  out_csum;
 };
+extern struct gdb_context *gdb_ctx;
 
 /* interface to arch specific routines */
 void gdb_write_to_packet(

_______________________________________________
Xen-ia64-devel mailing list
[email protected]
http://lists.xensource.com/xen-ia64-devel

Keir Fraser

2008-01-30 08:27 UTC #1 lore raw

This doesn't need to be fixed on x86 -- the int3 handler will return silently if the debugger is not configured. It would be nice if the ia64 handler would do the same. If that is not possible then change only ia64 code and if you need to be able to probe gdbstub configuration then add a public function to do that rather than grope at internal state. The same goes for powerpc, *if* it is broken in the first place.

-- Keir

Quoted 18 lines · Akio Takebe

Hi,
This patch avoid debugger_trap when we don't setup a debugger.
kexec_crash() is called after debugger_trap_immediate(). Because we want to use the safe path at kexecing, we should avoid the debugger path.
And the trap is a needless work.
Signed-off-by Akio Takebe <[email protected]>
Best Regards,
Akio Takebe
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Akio Takebe

author

2008-01-30 08:48 UTC #2 lore raw

Hi, Keir

Quoted 7 lines

This doesn't need to be fixed on x86 -- the int3 handler will return silently if the debugger is not configured. It would be nice if the ia64 handler would do the same. If that is not possible then change only ia64 code and if you need to be able to probe gdbstub configuration then add a public function to do that rather than grope at internal state. The same goes for powerpc, *if* it is broken in the first place.

I know the int3 hanlder return well on x86 if it is not configured. But the route may be not safe. The debugger_trap is called after hypervisor panic and dom0 panic. So I want to avoid the needless route for kexec/kdump.

Best Regards,

Akio Takebe

Keir Fraser

2008-01-30 09:26 UTC #3 lore raw

Quoted 10 lines · Akio Takebe

Quoted 7 lines
This doesn't need to be fixed on x86 -- the int3 handler will return silently if the debugger is not configured. It would be nice if the ia64 handler would do the same. If that is not possible then change only ia64 code and if you need to be able to probe gdbstub configuration then add a public function to do that rather than grope at internal state. The same goes for powerpc, *if* it is broken in the first place.
I know the int3 hanlder return well on x86 if it is not configured. But the route may be not safe. The debugger_trap is called after hypervisor panic and dom0 panic. So I want to avoid the needless route for kexec/kdump.

Oh no, then all bets are off. :-) We could be screwed and fail to kexec no matter what we do.

-- Keir

Akio Takebe

author

2008-01-31 00:56 UTC #4 lore raw

Hi, Keir

Quoted 9 lines · Akio Takebe

Quoted 8 lines
Quoted 8 lines
This doesn't need to be fixed on x86 -- the int3 handler will return silently if the debugger is not configured. It would be nice if the ia64 handler would do the same. If that is not possible then change only ia64 code and if you need to be able to probe gdbstub configuration then add a public function to do that rather than grope at internal state. The same goes for powerpc, *if* it is broken in the first place.

Quoted 7 lines

Quoted 3 lines
I know the int3 hanlder return well on x86 if it is not configured. But the route may be not safe. The debugger_trap is called after hypervisor panic and dom0 panic. So I want to avoid the needless route for kexec/kdump.
Oh no, then all bets are off. :-) We could be screwed and fail to kexec no matter what we do.

OK. I just suggested a safer way than current code. Recently we found a bug around the trap handler on ia64, so I worry about such a thing.
I still want to avoid the needless code on ia64. so I'll talk in xen-ia64 ML.

Best Regards,

Akio Takebe