[PATCH 0/4] kgdb: Small usability improvements for x86

[PATCH 0/4] kgdb: Small usability improvements for x86

[Jan Kiszka]

This cleans up the "info register" result on x86 and adds gdb detach on
reboot/shutdown for this target arch. See patches for details.

CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: x86@kernel.org

Jan Kiszka (4):
  kgdb: x86: Return all segment registers also in 64-bit mode
  kgdb: Make gdbstub_exit a nop unless gdb is attached
  kgdb: Respect that flush op is optional
  kgdb: x86: Detach gdb if machine shuts down or reboots

 arch/x86/include/asm/kgdb.h |    6 +++++-
 arch/x86/kernel/kgdb.c      |    6 ++++--
 arch/x86/kernel/reboot.c    |    6 ++++++
 include/linux/kgdb.h        |    1 +
 kernel/debug/gdbstub.c      |    6 +++++-
 5 files changed, 21 insertions(+), 4 deletions(-)

-- 
1.7.3.4

[PATCH 1/4] kgdb: x86: Return all segment registers also in 64-bit mode

[Jan Kiszka]

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".

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/x86/include/asm/kgdb.h |    6 +++++-
 arch/x86/kernel/kgdb.c      |    6 ++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/kgdb.h b/arch/x86/include/asm/kgdb.h
index 77e95f5..e857f1a 100644
--- a/arch/x86/include/asm/kgdb.h
+++ b/arch/x86/include/asm/kgdb.h
@@ -64,9 +64,13 @@ 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
+#define DBG_MAX_REG_NUM		24
 /* 17 64 bit regs and 3 32 bit regs */
 #define NUMREGBYTES		((17 * 8) + (3 * 4))
 #endif /* ! CONFIG_X86_32 */
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index faba577..fdc37b3 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)
-- 
1.7.3.4

[PATCH 2/4] kgdb: Make gdbstub_exit a nop unless gdb is attached

[Jan Kiszka]

This allows to call gdbstub_exit without worrying if
 - CONFIG_KGDB is enabled
 - if an kgdb I/O driver is loaded
 - if a gdb frontend is currently attached

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 include/linux/kgdb.h   |    1 +
 kernel/debug/gdbstub.c |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index fa39183..16410e2 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -306,6 +306,7 @@ extern atomic_t			kgdb_active;
 extern bool dbg_is_early;
 extern void __init dbg_late_init(void);
 #else /* ! CONFIG_KGDB */
+static inline void gdbstub_exit(int status) { }
 #define in_dbg_master() (0)
 #define dbg_late_init()
 #endif /* ! CONFIG_KGDB */
diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
index c22d8c2..5d7ed0a 100644
--- a/kernel/debug/gdbstub.c
+++ b/kernel/debug/gdbstub.c
@@ -1111,6 +1111,9 @@ void gdbstub_exit(int status)
 	unsigned char checksum, ch, buffer[3];
 	int loop;
 
+	if (!dbg_io_ops || !kgdb_connected)
+		return;
+
 	buffer[0] = 'W';
 	buffer[1] = hex_asc_hi(status);
 	buffer[2] = hex_asc_lo(status);
-- 
1.7.3.4

[PATCH 3/4] kgdb: Respect that flush op is optional

[Jan Kiszka]

Not all kgdb I/O drivers implement a flush operation. Adjust
gdbstub_exit accordingly.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kernel/debug/gdbstub.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
index 5d7ed0a..c174ea3 100644
--- a/kernel/debug/gdbstub.c
+++ b/kernel/debug/gdbstub.c
@@ -1132,5 +1132,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();
 }
-- 
1.7.3.4

[PATCH 4/4] kgdb: x86: Detach gdb if machine shuts down or reboots

[Jan Kiszka]

Hook into machine restart/power-off/halt handlers and call gdbstub_exit
so that a attached gdb frontend is properly informed. If kgdb is
disabled or no frontend attached, gdbstub_exit will do nothing.

CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: x86@kernel.org
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/x86/kernel/reboot.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index d840e69..926ac17 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -7,6 +7,7 @@
 #include <linux/sched.h>
 #include <linux/tboot.h>
 #include <linux/delay.h>
+#include <linux/kgdb.h>
 #include <acpi/reboot.h>
 #include <asm/io.h>
 #include <asm/apic.h>
@@ -683,6 +684,7 @@ void native_machine_shutdown(void)
 static void __machine_emergency_restart(int emergency)
 {
 	reboot_emergency = emergency;
+	gdbstub_exit(1);
 	machine_ops.emergency_restart();
 }
 
@@ -730,6 +732,7 @@ struct machine_ops machine_ops = {
 
 void machine_power_off(void)
 {
+	gdbstub_exit(0);
 	machine_ops.power_off();
 }
 
@@ -745,17 +748,20 @@ void machine_emergency_restart(void)
 
 void machine_restart(char *cmd)
 {
+	gdbstub_exit(0);
 	machine_ops.restart(cmd);
 }
 
 void machine_halt(void)
 {
+	gdbstub_exit(0);
 	machine_ops.halt();
 }
 
 #ifdef CONFIG_KEXEC
 void machine_crash_shutdown(struct pt_regs *regs)
 {
+	gdbstub_exit(1);
 	machine_ops.crash_shutdown(regs);
 }
 #endif
-- 
1.7.3.4

Re: [PATCH 0/4] kgdb: Small usability improvements for x86

[Jan Kiszka]

On 2012-03-16 12:40, Jan Kiszka wrote:
> This cleans up the "info register" result on x86 and adds gdb detach on
> reboot/shutdown for this target arch. See patches for details.
> 
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: x86@kernel.org
> 
> Jan Kiszka (4):
>   kgdb: x86: Return all segment registers also in 64-bit mode
>   kgdb: Make gdbstub_exit a nop unless gdb is attached
>   kgdb: Respect that flush op is optional
>   kgdb: x86: Detach gdb if machine shuts down or reboots
> 
>  arch/x86/include/asm/kgdb.h |    6 +++++-
>  arch/x86/kernel/kgdb.c      |    6 ++++--
>  arch/x86/kernel/reboot.c    |    6 ++++++
>  include/linux/kgdb.h        |    1 +
>  kernel/debug/gdbstub.c      |    6 +++++-
>  5 files changed, 21 insertions(+), 4 deletions(-)
> 

Sorry, wrong CC. Should have gone to LKML instead of the kvm list. I
will repost.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux