From f92509371ec06227a7e29778f395776d31b0deab Mon Sep 17 00:00:00 2001 From: "grzegorz.chwesewicz@chilan.com" Date: Thu, 24 Apr 2008 16:57:22 -0500 Subject: [PATCH 1/5] kgdb: minor documentation fixes Two minor fixes to the kgdb documentation. Signed-off-by: Grzegorz Chwesewicz, Chilan Signed-off-by: Jason Wessel --- Documentation/DocBook/kgdb.tmpl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Documentation/DocBook/kgdb.tmpl b/Documentation/DocBook/kgdb.tmpl index 97618bed4d65..028a8444d95e 100644 --- a/Documentation/DocBook/kgdb.tmpl +++ b/Documentation/DocBook/kgdb.tmpl @@ -72,7 +72,7 @@ kgdb is a source level debugger for linux kernel. It is used along with gdb to debug a linux kernel. The expectation is that gdb can be used to "break in" to the kernel to inspect memory, variables - and look through a cal stack information similar to what an + and look through call stack information similar to what an application developer would use gdb for. It is possible to place breakpoints in kernel code and perform some limited execution stepping. @@ -93,8 +93,10 @@ Compiling a kernel - To enable CONFIG_KGDB, look under the "Kernel debugging" - and then select "KGDB: kernel debugging with remote gdb". + To enable CONFIG_KGDB you should first turn on + "Prompt for development and/or incomplete code/drivers" + (CONFIG_EXPERIMENTAL) in "General setup", then under the + "Kernel debugging" select "KGDB: kernel debugging with remote gdb". Next you should choose one of more I/O drivers to interconnect debugging From 001fddf5fdcfe2c08ac9c4e5ca80c5e5698363bb Mon Sep 17 00:00:00 2001 From: Harvey Harrison Date: Thu, 24 Apr 2008 16:57:23 -0500 Subject: [PATCH 2/5] kgdb: trivial sparse fixes in kgdb test-suite Shadowed variable and integer as NULL pointer fixes: drivers/misc/kgdbts.c:877:6: warning: symbol 'sys_open_test' shadows an earlier one drivers/misc/kgdbts.c:537:27: originally declared here drivers/misc/kgdbts.c:378:22: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:386:22: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:468:30: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:472:15: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:502:30: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:506:30: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:509:30: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:523:20: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:527:20: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:530:15: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:541:21: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:545:21: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:548:15: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:559:30: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:563:15: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:573:16: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:574:19: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:578:15: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:588:16: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:589:19: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:593:15: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:602:16: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:604:15: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:925:3: warning: Using plain integer as NULL pointer drivers/misc/kgdbts.c:938:3: warning: Using plain integer as NULL pointer Signed-off-by: Harvey Harrison Signed-off-by: Jason Wessel --- drivers/misc/kgdbts.c | 60 +++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c index 30a1af857c7a..879fcbde32c2 100644 --- a/drivers/misc/kgdbts.c +++ b/drivers/misc/kgdbts.c @@ -375,7 +375,7 @@ static void emul_sstep_get(char *arg) break; case 1: /* set breakpoint */ - break_helper("Z0", 0, sstep_addr); + break_helper("Z0", NULL, sstep_addr); break; case 2: /* Continue */ @@ -383,7 +383,7 @@ static void emul_sstep_get(char *arg) break; case 3: /* Clear breakpoint */ - break_helper("z0", 0, sstep_addr); + break_helper("z0", NULL, sstep_addr); break; default: eprintk("kgdbts: ERROR failed sstep get emulation\n"); @@ -465,11 +465,11 @@ static struct test_struct sw_breakpoint_test[] = { { "?", "S0*" }, /* Clear break points */ { "kgdbts_break_test", "OK", sw_break, }, /* set sw breakpoint */ { "c", "T0*", }, /* Continue */ - { "g", "kgdbts_break_test", 0, check_and_rewind_pc }, + { "g", "kgdbts_break_test", NULL, check_and_rewind_pc }, { "write", "OK", write_regs }, { "kgdbts_break_test", "OK", sw_rem_break }, /*remove breakpoint */ { "D", "OK" }, /* Detach */ - { "D", "OK", 0, got_break }, /* If the test worked we made it here */ + { "D", "OK", NULL, got_break }, /* On success we made it here */ { "", "" }, }; @@ -499,14 +499,14 @@ static struct test_struct singlestep_break_test[] = { { "?", "S0*" }, /* Clear break points */ { "kgdbts_break_test", "OK", sw_break, }, /* set sw breakpoint */ { "c", "T0*", }, /* Continue */ - { "g", "kgdbts_break_test", 0, check_and_rewind_pc }, + { "g", "kgdbts_break_test", NULL, check_and_rewind_pc }, { "write", "OK", write_regs }, /* Write registers */ { "kgdbts_break_test", "OK", sw_rem_break }, /*remove breakpoint */ { "s", "T0*", emul_sstep_get, emul_sstep_put }, /* Single step */ - { "g", "kgdbts_break_test", 0, check_single_step }, + { "g", "kgdbts_break_test", NULL, check_single_step }, { "kgdbts_break_test", "OK", sw_break, }, /* set sw breakpoint */ { "c", "T0*", }, /* Continue */ - { "g", "kgdbts_break_test", 0, check_and_rewind_pc }, + { "g", "kgdbts_break_test", NULL, check_and_rewind_pc }, { "write", "OK", write_regs }, /* Write registers */ { "D", "OK" }, /* Remove all breakpoints and continues */ { "", "" }, @@ -520,14 +520,14 @@ static struct test_struct do_fork_test[] = { { "?", "S0*" }, /* Clear break points */ { "do_fork", "OK", sw_break, }, /* set sw breakpoint */ { "c", "T0*", }, /* Continue */ - { "g", "do_fork", 0, check_and_rewind_pc }, /* check location */ + { "g", "do_fork", NULL, check_and_rewind_pc }, /* check location */ { "write", "OK", write_regs }, /* Write registers */ { "do_fork", "OK", sw_rem_break }, /*remove breakpoint */ { "s", "T0*", emul_sstep_get, emul_sstep_put }, /* Single step */ - { "g", "do_fork", 0, check_single_step }, + { "g", "do_fork", NULL, check_single_step }, { "do_fork", "OK", sw_break, }, /* set sw breakpoint */ { "7", "T0*", skip_back_repeat_test }, /* Loop based on repeat_test */ - { "D", "OK", 0, final_ack_set }, /* detach and unregister I/O */ + { "D", "OK", NULL, final_ack_set }, /* detach and unregister I/O */ { "", "" }, }; @@ -538,14 +538,14 @@ static struct test_struct sys_open_test[] = { { "?", "S0*" }, /* Clear break points */ { "sys_open", "OK", sw_break, }, /* set sw breakpoint */ { "c", "T0*", }, /* Continue */ - { "g", "sys_open", 0, check_and_rewind_pc }, /* check location */ + { "g", "sys_open", NULL, check_and_rewind_pc }, /* check location */ { "write", "OK", write_regs }, /* Write registers */ { "sys_open", "OK", sw_rem_break }, /*remove breakpoint */ { "s", "T0*", emul_sstep_get, emul_sstep_put }, /* Single step */ - { "g", "sys_open", 0, check_single_step }, + { "g", "sys_open", NULL, check_single_step }, { "sys_open", "OK", sw_break, }, /* set sw breakpoint */ { "7", "T0*", skip_back_repeat_test }, /* Loop based on repeat_test */ - { "D", "OK", 0, final_ack_set }, /* detach and unregister I/O */ + { "D", "OK", NULL, final_ack_set }, /* detach and unregister I/O */ { "", "" }, }; @@ -556,11 +556,11 @@ static struct test_struct hw_breakpoint_test[] = { { "?", "S0*" }, /* Clear break points */ { "kgdbts_break_test", "OK", hw_break, }, /* set hw breakpoint */ { "c", "T0*", }, /* Continue */ - { "g", "kgdbts_break_test", 0, check_and_rewind_pc }, + { "g", "kgdbts_break_test", NULL, check_and_rewind_pc }, { "write", "OK", write_regs }, { "kgdbts_break_test", "OK", hw_rem_break }, /*remove breakpoint */ { "D", "OK" }, /* Detach */ - { "D", "OK", 0, got_break }, /* If the test worked we made it here */ + { "D", "OK", NULL, got_break }, /* On success we made it here */ { "", "" }, }; @@ -570,12 +570,12 @@ static struct test_struct hw_breakpoint_test[] = { static struct test_struct hw_write_break_test[] = { { "?", "S0*" }, /* Clear break points */ { "hw_break_val", "OK", hw_write_break, }, /* set hw breakpoint */ - { "c", "T0*", 0, got_break }, /* Continue */ - { "g", "silent", 0, check_and_rewind_pc }, + { "c", "T0*", NULL, got_break }, /* Continue */ + { "g", "silent", NULL, check_and_rewind_pc }, { "write", "OK", write_regs }, { "hw_break_val", "OK", hw_rem_write_break }, /*remove breakpoint */ { "D", "OK" }, /* Detach */ - { "D", "OK", 0, got_break }, /* If the test worked we made it here */ + { "D", "OK", NULL, got_break }, /* On success we made it here */ { "", "" }, }; @@ -585,12 +585,12 @@ static struct test_struct hw_write_break_test[] = { static struct test_struct hw_access_break_test[] = { { "?", "S0*" }, /* Clear break points */ { "hw_break_val", "OK", hw_access_break, }, /* set hw breakpoint */ - { "c", "T0*", 0, got_break }, /* Continue */ - { "g", "silent", 0, check_and_rewind_pc }, + { "c", "T0*", NULL, got_break }, /* Continue */ + { "g", "silent", NULL, check_and_rewind_pc }, { "write", "OK", write_regs }, { "hw_break_val", "OK", hw_rem_access_break }, /*remove breakpoint */ { "D", "OK" }, /* Detach */ - { "D", "OK", 0, got_break }, /* If the test worked we made it here */ + { "D", "OK", NULL, got_break }, /* On success we made it here */ { "", "" }, }; @@ -599,9 +599,9 @@ static struct test_struct hw_access_break_test[] = { */ static struct test_struct nmi_sleep_test[] = { { "?", "S0*" }, /* Clear break points */ - { "c", "T0*", 0, got_break }, /* Continue */ + { "c", "T0*", NULL, got_break }, /* Continue */ { "D", "OK" }, /* Detach */ - { "D", "OK", 0, got_break }, /* If the test worked we made it here */ + { "D", "OK", NULL, got_break }, /* On success we made it here */ { "", "" }, }; @@ -874,15 +874,15 @@ static void kgdbts_run_tests(void) { char *ptr; int fork_test = 0; - int sys_open_test = 0; + int do_sys_open_test = 0; int nmi_sleep = 0; ptr = strstr(config, "F"); if (ptr) - fork_test = simple_strtol(ptr+1, NULL, 10); + fork_test = simple_strtol(ptr + 1, NULL, 10); ptr = strstr(config, "S"); if (ptr) - sys_open_test = simple_strtol(ptr+1, NULL, 10); + do_sys_open_test = simple_strtol(ptr + 1, NULL, 10); ptr = strstr(config, "N"); if (ptr) nmi_sleep = simple_strtol(ptr+1, NULL, 10); @@ -922,7 +922,7 @@ static void kgdbts_run_tests(void) repeat_test = fork_test; printk(KERN_INFO "kgdbts:RUN do_fork for %i breakpoints\n", repeat_test); - kthread_run(kgdbts_unreg_thread, 0, "kgdbts_unreg"); + kthread_run(kgdbts_unreg_thread, NULL, "kgdbts_unreg"); run_do_fork_test(); return; } @@ -931,11 +931,11 @@ static void kgdbts_run_tests(void) * executed because a kernel thread will be spawned at the very * end to unregister the debug hooks. */ - if (sys_open_test) { - repeat_test = sys_open_test; + if (do_sys_open_test) { + repeat_test = do_sys_open_test; printk(KERN_INFO "kgdbts:RUN sys_open for %i breakpoints\n", repeat_test); - kthread_run(kgdbts_unreg_thread, 0, "kgdbts_unreg"); + kthread_run(kgdbts_unreg_thread, NULL, "kgdbts_unreg"); run_sys_open_test(); return; } From 7cfcd985d36031459cc64e3843ea36a4d801097d Mon Sep 17 00:00:00 2001 From: Jason Wessel Date: Thu, 24 Apr 2008 16:57:23 -0500 Subject: [PATCH 3/5] kgdb: 1000 loops for the single step test in kgdbts The single step test is not terribly costly and it should be able to pass at 1000 loops successfully in under 1 second. A non-kgdb timing regression was found using this test, but it did not occur frequently because by default the test was only executed a single time. This patch changes the default for the single step test to 1000 iterations and allows for individual configuration of the single step test to further exercise the kgdb subsystem when needed. Signed-off-by: Jason Wessel --- drivers/misc/kgdbts.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c index 879fcbde32c2..fa394104339c 100644 --- a/drivers/misc/kgdbts.c +++ b/drivers/misc/kgdbts.c @@ -47,6 +47,7 @@ * to test the HW NMI watchdog * F## = Break at do_fork for ## iterations * S## = Break at sys_open for ## iterations + * I## = Run the single step test ## iterations * * NOTE: that the do_fork and sys_open tests are mutually exclusive. * @@ -875,7 +876,9 @@ static void kgdbts_run_tests(void) char *ptr; int fork_test = 0; int do_sys_open_test = 0; + int sstep_test = 1000; int nmi_sleep = 0; + int i; ptr = strstr(config, "F"); if (ptr) @@ -886,6 +889,9 @@ static void kgdbts_run_tests(void) ptr = strstr(config, "N"); if (ptr) nmi_sleep = simple_strtol(ptr+1, NULL, 10); + ptr = strstr(config, "I"); + if (ptr) + sstep_test = simple_strtol(ptr+1, NULL, 10); /* required internal KGDB tests */ v1printk("kgdbts:RUN plant and detach test\n"); @@ -894,8 +900,13 @@ static void kgdbts_run_tests(void) run_breakpoint_test(0); v1printk("kgdbts:RUN bad memory access test\n"); run_bad_read_test(); - v1printk("kgdbts:RUN singlestep breakpoint test\n"); - run_singlestep_break_test(); + v1printk("kgdbts:RUN singlestep test %i iterations\n", sstep_test); + for (i = 0; i < sstep_test; i++) { + run_singlestep_break_test(); + if (i % 100 == 0) + v1printk("kgdbts:RUN singlestep [%i/%i]\n", + i, sstep_test); + } /* ===Optional tests=== */ From 688b744d8bc84dc5cc646e97509113dc5e8818ed Mon Sep 17 00:00:00 2001 From: Harvey Harrison Date: Thu, 24 Apr 2008 16:57:23 -0500 Subject: [PATCH 4/5] kgdb: fix signedness mixmatches, add statics, add declaration to header Noticed by sparse: arch/x86/kernel/kgdb.c:556:15: warning: symbol 'kgdb_arch_pc' was not declared. Should it be static? kernel/kgdb.c:149:8: warning: symbol 'kgdb_do_roundup' was not declared. Should it be static? kernel/kgdb.c:193:22: warning: symbol 'kgdb_arch_pc' was not declared. Should it be static? kernel/kgdb.c:712:5: warning: symbol 'remove_all_break' was not declared. Should it be static? Related to kgdb_hex2long: arch/x86/kernel/kgdb.c:371:28: warning: incorrect type in argument 2 (different signedness) arch/x86/kernel/kgdb.c:371:28: expected long *long_val arch/x86/kernel/kgdb.c:371:28: got unsigned long * kernel/kgdb.c:469:27: warning: incorrect type in argument 2 (different signedness) kernel/kgdb.c:469:27: expected long *long_val kernel/kgdb.c:469:27: got unsigned long * kernel/kgdb.c:470:27: warning: incorrect type in argument 2 (different signedness) kernel/kgdb.c:470:27: expected long *long_val kernel/kgdb.c:470:27: got unsigned long * kernel/kgdb.c:894:27: warning: incorrect type in argument 2 (different signedness) kernel/kgdb.c:894:27: expected long *long_val kernel/kgdb.c:894:27: got unsigned long * kernel/kgdb.c:895:27: warning: incorrect type in argument 2 (different signedness) kernel/kgdb.c:895:27: expected long *long_val kernel/kgdb.c:895:27: got unsigned long * kernel/kgdb.c:1127:28: warning: incorrect type in argument 2 (different signedness) kernel/kgdb.c:1127:28: expected long *long_val kernel/kgdb.c:1127:28: got unsigned long * kernel/kgdb.c:1132:25: warning: incorrect type in argument 2 (different signedness) kernel/kgdb.c:1132:25: expected long *long_val kernel/kgdb.c:1132:25: got unsigned long * Signed-off-by: Harvey Harrison Signed-off-by: Jason Wessel --- include/linux/kgdb.h | 4 +++- kernel/kgdb.c | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index 9757b1a6d9dc..6adcc297e354 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -261,10 +261,12 @@ struct kgdb_io { extern struct kgdb_arch arch_kgdb_ops; +extern unsigned long __weak kgdb_arch_pc(int exception, struct pt_regs *regs); + extern int kgdb_register_io_module(struct kgdb_io *local_kgdb_io_ops); extern void kgdb_unregister_io_module(struct kgdb_io *local_kgdb_io_ops); -extern int kgdb_hex2long(char **ptr, long *long_val); +extern int kgdb_hex2long(char **ptr, unsigned long *long_val); extern int kgdb_mem2hex(char *mem, char *buf, int count); extern int kgdb_hex2mem(char *buf, char *mem, int count); diff --git a/kernel/kgdb.c b/kernel/kgdb.c index 1bd0ec1c80b2..39e31a036f5b 100644 --- a/kernel/kgdb.c +++ b/kernel/kgdb.c @@ -61,7 +61,7 @@ struct kgdb_state { int err_code; int cpu; int pass_exception; - long threadid; + unsigned long threadid; long kgdb_usethreadid; struct pt_regs *linux_regs; }; @@ -146,7 +146,7 @@ atomic_t kgdb_cpu_doing_single_step = ATOMIC_INIT(-1); * the other CPUs might interfere with your debugging context, so * use this with care: */ -int kgdb_do_roundup = 1; +static int kgdb_do_roundup = 1; static int __init opt_nokgdbroundup(char *str) { @@ -438,7 +438,7 @@ int kgdb_hex2mem(char *buf, char *mem, int count) * While we find nice hex chars, build a long_val. * Return number of chars processed. */ -int kgdb_hex2long(char **ptr, long *long_val) +int kgdb_hex2long(char **ptr, unsigned long *long_val) { int hex_val; int num = 0; @@ -709,7 +709,7 @@ int kgdb_isremovedbreak(unsigned long addr) return 0; } -int remove_all_break(void) +static int remove_all_break(void) { unsigned long addr; int error; From e024cbd257efc2788b7d21b9353e966267485c87 Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Fri, 25 Apr 2008 18:35:29 +0200 Subject: [PATCH 5/5] kgdb: kconfig fix xconfig/menuconfig element Kconfig.kgdb: fix menuconfig element Signed-off-by: Jan Engelhardt Signed-off-by: Jason Wessel --- lib/Kconfig.kgdb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb index f2e01ac5ab09..a5d4b1dac2a5 100644 --- a/lib/Kconfig.kgdb +++ b/lib/Kconfig.kgdb @@ -1,4 +1,10 @@ +config HAVE_ARCH_KGDB_SHADOW_INFO + bool + +config HAVE_ARCH_KGDB + bool + menuconfig KGDB bool "KGDB: kernel debugging with remote gdb" select FRAME_POINTER @@ -10,15 +16,10 @@ menuconfig KGDB at http://kgdb.sourceforge.net as well as in DocBook form in Documentation/DocBook/. If unsure, say N. -config HAVE_ARCH_KGDB_SHADOW_INFO - bool - -config HAVE_ARCH_KGDB - bool +if KGDB config KGDB_SERIAL_CONSOLE tristate "KGDB: use kgdb over the serial console" - depends on KGDB select CONSOLE_POLL select MAGIC_SYSRQ default y @@ -28,7 +29,6 @@ config KGDB_SERIAL_CONSOLE config KGDB_TESTS bool "KGDB: internal test suite" - depends on KGDB default n help This is a kgdb I/O module specifically designed to test @@ -56,3 +56,5 @@ config KGDB_TESTS_BOOT_STRING boot. See the drivers/misc/kgdbts.c for detailed information about other strings you could use beyond the default of V1F100. + +endif # KGDB