From 639077fb69aec8112e5427210a83d0fb192969f0 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Mon, 19 Mar 2012 15:16:48 -0500 Subject: kgdb: x86: Return all segment registers also in 64-bit mode Even if the content is always 0, gdb expects us to return also ds, es, fs, and gs while in x86-64 mode. Do this to avoid ugly errors on "info registers". [jason.wessel@windriver.com: adjust NUMREGBYTES for two new regs] Signed-off-by: Jan Kiszka Signed-off-by: Jason Wessel --- arch/x86/include/asm/kgdb.h | 10 +++++++--- arch/x86/kernel/kgdb.c | 6 ++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/arch/x86/include/asm/kgdb.h b/arch/x86/include/asm/kgdb.h index 77e95f54570a..332f98c9111f 100644 --- a/arch/x86/include/asm/kgdb.h +++ b/arch/x86/include/asm/kgdb.h @@ -64,11 +64,15 @@ enum regnames { GDB_PS, /* 17 */ GDB_CS, /* 18 */ GDB_SS, /* 19 */ + GDB_DS, /* 20 */ + GDB_ES, /* 21 */ + GDB_FS, /* 22 */ + GDB_GS, /* 23 */ }; #define GDB_ORIG_AX 57 -#define DBG_MAX_REG_NUM 20 -/* 17 64 bit regs and 3 32 bit regs */ -#define NUMREGBYTES ((17 * 8) + (3 * 4)) +#define DBG_MAX_REG_NUM 24 +/* 17 64 bit regs and 5 32 bit regs */ +#define NUMREGBYTES ((17 * 8) + (5 * 4)) #endif /* ! CONFIG_X86_32 */ static inline void arch_kgdb_breakpoint(void) diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c index faba5771acad..fdc37b3d0ce3 100644 --- a/arch/x86/kernel/kgdb.c +++ b/arch/x86/kernel/kgdb.c @@ -67,8 +67,6 @@ struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = { "ss", 4, offsetof(struct pt_regs, ss) }, { "ds", 4, offsetof(struct pt_regs, ds) }, { "es", 4, offsetof(struct pt_regs, es) }, - { "fs", 4, -1 }, - { "gs", 4, -1 }, #else { "ax", 8, offsetof(struct pt_regs, ax) }, { "bx", 8, offsetof(struct pt_regs, bx) }, @@ -90,7 +88,11 @@ struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = { "flags", 4, offsetof(struct pt_regs, flags) }, { "cs", 4, offsetof(struct pt_regs, cs) }, { "ss", 4, offsetof(struct pt_regs, ss) }, + { "ds", 4, -1 }, + { "es", 4, -1 }, #endif + { "fs", 4, -1 }, + { "gs", 4, -1 }, }; int dbg_set_reg(int regno, void *mem, struct pt_regs *regs) -- cgit v1.2.3 From 9fbe465efc76044dd87afe764db5464ae61aeabc Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Fri, 16 Mar 2012 13:17:13 +0100 Subject: kgdb: Respect that flush op is optional Not all kgdb I/O drivers implement a flush operation. Adjust gdbstub_exit accordingly. Signed-off-by: Jan Kiszka Signed-off-by: Jason Wessel --- kernel/debug/gdbstub.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c index c22d8c28ad84..5a155742ae96 100644 --- a/kernel/debug/gdbstub.c +++ b/kernel/debug/gdbstub.c @@ -1129,5 +1129,6 @@ void gdbstub_exit(int status) dbg_io_ops->write_char(hex_asc_lo(checksum)); /* make sure the output is flushed, lest the bootloader clobber it */ - dbg_io_ops->flush(); + if (dbg_io_ops->flush) + dbg_io_ops->flush(); } -- cgit v1.2.3 From 2366e047840e33928803c0442176fb3991423da8 Mon Sep 17 00:00:00 2001 From: Jason Wessel Date: Fri, 16 Mar 2012 14:20:41 -0500 Subject: kgdb,debug-core,gdbstub: Hook the reboot notifier for debugger detach The gdbstub and kdb should get detached if the system is rebooting. Calling gdbstub_exit() will set the proper debug core state and send a message to any debugger that is connected to correctly detach. An attached debugger will receive the exit code from include/linux/reboot.h based on SYS_HALT, SYS_REBOOT, etc... Reported-by: Jan Kiszka Signed-off-by: Jason Wessel --- kernel/debug/debug_core.c | 17 +++++++++++++++++ kernel/debug/gdbstub.c | 7 +++++++ 2 files changed, 24 insertions(+) diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index 0d7c08784efb..3c1ad4e03543 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -784,6 +785,20 @@ void __init dbg_late_init(void) kdb_init(KDB_INIT_FULL); } +static int +dbg_notify_reboot(struct notifier_block *this, unsigned long code, void *x) +{ + if (!dbg_kdb_mode) + gdbstub_exit(code); + return NOTIFY_DONE; +} + +static struct notifier_block dbg_reboot_notifier = { + .notifier_call = dbg_notify_reboot, + .next = NULL, + .priority = INT_MAX, +}; + static void kgdb_register_callbacks(void) { if (!kgdb_io_module_registered) { @@ -791,6 +806,7 @@ static void kgdb_register_callbacks(void) kgdb_arch_init(); if (!dbg_is_early) kgdb_arch_late(); + register_reboot_notifier(&dbg_reboot_notifier); atomic_notifier_chain_register(&panic_notifier_list, &kgdb_panic_event_nb); #ifdef CONFIG_MAGIC_SYSRQ @@ -812,6 +828,7 @@ static void kgdb_unregister_callbacks(void) */ if (kgdb_io_module_registered) { kgdb_io_module_registered = 0; + unregister_reboot_notifier(&dbg_reboot_notifier); atomic_notifier_chain_unregister(&panic_notifier_list, &kgdb_panic_event_nb); kgdb_arch_exit(); diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c index 5a155742ae96..ce615e064482 100644 --- a/kernel/debug/gdbstub.c +++ b/kernel/debug/gdbstub.c @@ -1111,6 +1111,13 @@ void gdbstub_exit(int status) unsigned char checksum, ch, buffer[3]; int loop; + if (!kgdb_connected) + return; + kgdb_connected = 0; + + if (!dbg_io_ops || dbg_kdb_mode) + return; + buffer[0] = 'W'; buffer[1] = hex_asc_hi(status); buffer[2] = hex_asc_lo(status); -- cgit v1.2.3 From 8f30d411767351656ea62c9e7612120f9b870b59 Mon Sep 17 00:00:00 2001 From: Andrei Warkentin Date: Tue, 28 Feb 2012 06:55:05 -0600 Subject: KDB: Fix usability issues relating to the 'enter' key. This fixes the following problems: 1) Typematic-repeat of 'enter' gives warning message and leaks make/break if KDB exits. Repeats look something like 0x1c 0x1c .... 0x9c 2) Use of 'keypad enter' gives warning message and leaks the ENTER break/make code out if KDB exits. KP ENTER repeats look someting like 0xe0 0x1c 0xe0 0x1c ... 0xe0 0x9c. 3) Lag on the order of seconds between "break" and "make" when expecting the enter "break" code. Seen under virtualized environments such as VMware ESX. The existing special enter handler tries to glob the enter break code, but this fails if the other (KP) enter was used, or if there was a key repeat. It also fails if you mashed some keys along with enter, and you ended up with a non-enter make or non-enter break code coming after the enter make code. So first, we modify the handler to handle these cases. But performing these actions on every enter is annoying since now you can't hold ENTER down to scroll d messages in KDB. Since this special behaviour is only necessary to handle the exiting KDB ('g' + ENTER) without leaking scancodes to the OS. This cleanup needs to get executed anytime the kdb_main loop exits. Tested on QEMU. Set a bp on atkbd.c to verify no scan code was leaked. Cc: Andrei Warkentin [jason.wessel@windriver.com: move cleanup calls to kdb_main.c] Signed-off-by: Andrei Warkentin Signed-off-by: Jason Wessel --- kernel/debug/kdb/kdb_keyboard.c | 95 +++++++++++++++++++++++++++++++---------- kernel/debug/kdb/kdb_main.c | 3 ++ kernel/debug/kdb/kdb_private.h | 7 +++ 3 files changed, 83 insertions(+), 22 deletions(-) diff --git a/kernel/debug/kdb/kdb_keyboard.c b/kernel/debug/kdb/kdb_keyboard.c index 4bca634975c0..118527aa60ea 100644 --- a/kernel/debug/kdb/kdb_keyboard.c +++ b/kernel/debug/kdb/kdb_keyboard.c @@ -25,6 +25,7 @@ #define KBD_STAT_MOUSE_OBF 0x20 /* Mouse output buffer full */ static int kbd_exists; +static int kbd_last_ret; /* * Check if the keyboard controller has a keypress for us. @@ -90,8 +91,11 @@ int kdb_get_kbd_char(void) return -1; } - if ((scancode & 0x80) != 0) + if ((scancode & 0x80) != 0) { + if (scancode == 0x9c) + kbd_last_ret = 0; return -1; + } scancode &= 0x7f; @@ -178,35 +182,82 @@ int kdb_get_kbd_char(void) return -1; /* ignore unprintables */ } - if ((scancode & 0x7f) == 0x1c) { - /* - * enter key. All done. Absorb the release scancode. - */ + if (scancode == 0x1c) { + kbd_last_ret = 1; + return 13; + } + + return keychar & 0xff; +} +EXPORT_SYMBOL_GPL(kdb_get_kbd_char); + +/* + * Best effort cleanup of ENTER break codes on leaving KDB. Called on + * exiting KDB, when we know we processed an ENTER or KP ENTER scan + * code. + */ +void kdb_kbd_cleanup_state(void) +{ + int scancode, scanstatus; + + /* + * Nothing to clean up, since either + * ENTER was never pressed, or has already + * gotten cleaned up. + */ + if (!kbd_last_ret) + return; + + kbd_last_ret = 0; + /* + * Enter key. Need to absorb the break code here, lest it gets + * leaked out if we exit KDB as the result of processing 'g'. + * + * This has several interesting implications: + * + Need to handle KP ENTER, which has break code 0xe0 0x9c. + * + Need to handle repeat ENTER and repeat KP ENTER. Repeats + * only get a break code at the end of the repeated + * sequence. This means we can't propagate the repeated key + * press, and must swallow it away. + * + Need to handle possible PS/2 mouse input. + * + Need to handle mashed keys. + */ + + while (1) { while ((inb(KBD_STATUS_REG) & KBD_STAT_OBF) == 0) - ; + cpu_relax(); /* - * Fetch the scancode + * Fetch the scancode. */ scancode = inb(KBD_DATA_REG); scanstatus = inb(KBD_STATUS_REG); - while (scanstatus & KBD_STAT_MOUSE_OBF) { - scancode = inb(KBD_DATA_REG); - scanstatus = inb(KBD_STATUS_REG); - } + /* + * Skip mouse input. + */ + if (scanstatus & KBD_STAT_MOUSE_OBF) + continue; - if (scancode != 0x9c) { - /* - * Wasn't an enter-release, why not? - */ - kdb_printf("kdb: expected enter got 0x%x status 0x%x\n", - scancode, scanstatus); - } + /* + * If we see 0xe0, this is either a break code for KP + * ENTER, or a repeat make for KP ENTER. Either way, + * since the second byte is equivalent to an ENTER, + * skip the 0xe0 and try again. + * + * If we see 0x1c, this must be a repeat ENTER or KP + * ENTER (and we swallowed 0xe0 before). Try again. + * + * We can also see make and break codes for other keys + * mashed before or after pressing ENTER. Thus, if we + * see anything other than 0x9c, we have to try again. + * + * Note, if you held some key as ENTER was depressed, + * that break code would get leaked out. + */ + if (scancode != 0x9c) + continue; - return 13; + return; } - - return keychar & 0xff; } -EXPORT_SYMBOL_GPL(kdb_get_kbd_char); diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index e2ae7349437f..67b847dfa2bb 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -1400,6 +1400,9 @@ int kdb_main_loop(kdb_reason_t reason, kdb_reason_t reason2, int error, if (KDB_STATE(DOING_SS)) KDB_STATE_CLEAR(SSBPT); + /* Clean up any keyboard devices before leaving */ + kdb_kbd_cleanup_state(); + return result; } diff --git a/kernel/debug/kdb/kdb_private.h b/kernel/debug/kdb/kdb_private.h index e381d105b40b..47c4e56e513b 100644 --- a/kernel/debug/kdb/kdb_private.h +++ b/kernel/debug/kdb/kdb_private.h @@ -246,6 +246,13 @@ extern void debug_kusage(void); extern void kdb_set_current_task(struct task_struct *); extern struct task_struct *kdb_current_task; + +#ifdef CONFIG_KDB_KEYBOARD +extern void kdb_kbd_cleanup_state(void); +#else /* ! CONFIG_KDB_KEYBOARD */ +#define kdb_kbd_cleanup_state() +#endif /* ! CONFIG_KDB_KEYBOARD */ + #ifdef CONFIG_MODULES extern struct list_head *kdb_modules; #endif /* CONFIG_MODULES */ -- cgit v1.2.3 From bec4d62ead8096e433d624d9339893f50badd992 Mon Sep 17 00:00:00 2001 From: Jason Wessel Date: Mon, 19 Mar 2012 19:35:55 -0500 Subject: kgdb,debug_core: add the ability to control the reboot notifier Sometimes it is desirable to stop the kernel debugger before allowing a system to reboot either with kdb or kgdb. This patch adds the ability to turn the reboot notifier on and off or enter the debugger and stop kernel execution before rebooting. It is possible to change the setting after booting the kernel with the following: echo 1 > /sys/module/debug_core/parameters/kgdbreboot It is also possible to change this setting using kdb / kgdb to manipulate the variable directly. Using KDB: mm kgdbreboot 1 Using gdb: set kgdbreboot=1 Reported-by: Jan Kiszka Signed-off-by: Jason Wessel --- Documentation/DocBook/kgdb.tmpl | 17 +++++++++++++++++ kernel/debug/debug_core.c | 16 ++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/Documentation/DocBook/kgdb.tmpl b/Documentation/DocBook/kgdb.tmpl index d71b57fcf116..4ee4ba3509fc 100644 --- a/Documentation/DocBook/kgdb.tmpl +++ b/Documentation/DocBook/kgdb.tmpl @@ -361,6 +361,23 @@ It is possible to use this option with kgdboc on a tty that is not a system console. + + + Run time parameter: kgdbreboot + The kgdbreboot feature allows you to change how the debugger + deals with the reboot notification. You have 3 choices for the + behavior. The default behavior is always set to 0. + + echo -1 > /sys/module/debug_core/parameters/kgdbreboot + Ignore the reboot notification entirely. + + echo 0 > /sys/module/debug_core/parameters/kgdbreboot + Send the detach message to any attached debugger client. + + echo 1 > /sys/module/debug_core/parameters/kgdbreboot + Enter the debugger on reboot notify. + + diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index 3c1ad4e03543..3f88a45e6f0a 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -76,6 +76,8 @@ static int exception_level; struct kgdb_io *dbg_io_ops; static DEFINE_SPINLOCK(kgdb_registration_lock); +/* Action for the reboot notifiter, a global allow kdb to change it */ +static int kgdbreboot; /* kgdb console driver is loaded */ static int kgdb_con_registered; /* determine if kgdb console output should be used */ @@ -97,6 +99,7 @@ static int __init opt_kgdb_con(char *str) early_param("kgdbcon", opt_kgdb_con); module_param(kgdb_use_con, int, 0644); +module_param(kgdbreboot, int, 0644); /* * Holds information about breakpoints in a kernel. These breakpoints are @@ -788,8 +791,21 @@ void __init dbg_late_init(void) static int dbg_notify_reboot(struct notifier_block *this, unsigned long code, void *x) { + /* + * Take the following action on reboot notify depending on value: + * 1 == Enter debugger + * 0 == [the default] detatch debug client + * -1 == Do nothing... and use this until the board resets + */ + switch (kgdbreboot) { + case 1: + kgdb_breakpoint(); + case -1: + goto done; + } if (!dbg_kdb_mode) gdbstub_exit(code); +done: return NOTIFY_DONE; } -- cgit v1.2.3 From b8adde8ddec9ff62a21564fa8020b5463e70d4de Mon Sep 17 00:00:00 2001 From: Tim Bird Date: Wed, 21 Sep 2011 13:19:12 -0700 Subject: kdb: Avoid using dbg_io_ops until it is initialized This fixes a bug with setting a breakpoint during kdb initialization (from kdb_cmds). Any call to kdb_printf() before the initialization of the kgdboc serial console driver (which happens much later during bootup than kdb_init), results in kernel panic due to the use of dbg_io_ops before it is initialized. Signed-off-by: Tim Bird Signed-off-by: Jason Wessel --- kernel/debug/kdb/kdb_io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c index 4802eb5840e1..9b5f17da1c56 100644 --- a/kernel/debug/kdb/kdb_io.c +++ b/kernel/debug/kdb/kdb_io.c @@ -689,7 +689,7 @@ kdb_printit: if (!dbg_kdb_mode && kgdb_connected) { gdbstub_msg_write(kdb_buffer, retlen); } else { - if (!dbg_io_ops->is_console) { + if (dbg_io_ops && !dbg_io_ops->is_console) { len = strlen(kdb_buffer); cp = kdb_buffer; while (len--) { -- cgit v1.2.3 From 1ba0c1720eb0de2d0f3abf84c0b128d10af520d1 Mon Sep 17 00:00:00 2001 From: Jason Wessel Date: Wed, 21 Sep 2011 13:07:47 -0700 Subject: kdb: Add message about CONFIG_DEBUG_RODATA on failure to install breakpoint On x86, if CONFIG_DEBUG_RODATA is set, one cannot set breakpoints via KDB. Apparently this is a well-known problem, as at least one distribution now ships with both KDB enabled and CONFIG_DEBUG_RODATA=y for security reasons. This patch adds an printk message to the breakpoint failure case, in order to provide suggestions about how to use the debugger. Reported-by: Tim Bird Signed-off-by: Jason Wessel Acked-by: Tim Bird --- kernel/debug/kdb/kdb_bp.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c index 20059ef4459a..8418c2f8ec5d 100644 --- a/kernel/debug/kdb/kdb_bp.c +++ b/kernel/debug/kdb/kdb_bp.c @@ -153,6 +153,13 @@ static int _kdb_bp_install(struct pt_regs *regs, kdb_bp_t *bp) } else { kdb_printf("%s: failed to set breakpoint at 0x%lx\n", __func__, bp->bp_addr); +#ifdef CONFIG_DEBUG_RODATA + if (!bp->bp_type) { + kdb_printf("Software breakpoints are unavailable.\n" + " Change the kernel CONFIG_DEBUG_RODATA=n\n" + " OR use hw breaks: help bph\n"); + } +#endif return 1; } return 0; -- cgit v1.2.3