[GIT PULL] kgdb regression fixes for 2.6.35-rc5

[GIT PULL] kgdb regression fixes for 2.6.35-rc5

[Jason Wessel]

Linus, please pull the for_linus branch for 2.6.35-rc5.  

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git for_linus

The following patch set covers cleaning up problems that have been
found since the merge of kdb to the 2.6.35-rc1 kernel.

Thanks,
Jason.


---
Jason Wessel (4):
      repair gdbstub to match the gdbserial protocol specification
      Fix merge regression from external kdb to upstream kdb
      debug_core,kdb: fix kgdb_connected bit set in the wrong place
      sysrq,kdb: Use __handle_sysrq() for kdb's sysrq function

Martin Hicks (1):
      kdb: break out of kdb_ll() when command is terminated

 drivers/char/sysrq.c        |    2 +-
 include/linux/sysrq.h       |    1 +
 kernel/debug/debug_core.c   |    2 +-
 kernel/debug/gdbstub.c      |    9 +++------
 kernel/debug/kdb/kdb_main.c |    7 +++++--
 5 files changed, 11 insertions(+), 10 deletions(-)

[PATCH 1/5] kdb: break out of kdb_ll() when command is terminated

[Jason Wessel]

From: Martin Hicks <mort@sgi.com>

Without this patch the "ll" linked-list traversal command won't
terminate when you hit q/Q.

Signed-off-by: Martin Hicks <mort@sgi.com>
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 kernel/debug/kdb/kdb_main.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 184cd82..a7fe2e9 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2291,6 +2291,9 @@ static int kdb_ll(int argc, const char **argv)
 	while (va) {
 		char buf[80];
 
+		if (KDB_FLAG(CMD_INTERRUPT))
+			return 0;
+
 		sprintf(buf, "%s " kdb_machreg_fmt "\n", command, va);
 		diag = kdb_parse(buf);
 		if (diag)
-- 
1.6.3.3

[PATCH 2/5] repair gdbstub to match the gdbserial protocol specification

[Jason Wessel]

The gdbserial protocol handler should return an empty packet instead
of an error string when ever it responds to a command it does not
implement.

The problem cases come from a debugger client sending
qTBuffer, qTStatus, qSearch, qSupported.

The incorrect response from the gdbstub leads the debugger clients to
not function correctly.  Recent versions of gdb will not detach correctly as a result of this behavior.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Signed-off-by: Dongdong Deng <dongdong.deng@windriver.com>
---
 kernel/debug/gdbstub.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
index 4b17b32..e8fd686 100644
--- a/kernel/debug/gdbstub.c
+++ b/kernel/debug/gdbstub.c
@@ -621,10 +621,8 @@ static void gdb_cmd_query(struct kgdb_state *ks)
 	switch (remcom_in_buffer[1]) {
 	case 's':
 	case 'f':
-		if (memcmp(remcom_in_buffer + 2, "ThreadInfo", 10)) {
-			error_packet(remcom_out_buffer, -EINVAL);
+		if (memcmp(remcom_in_buffer + 2, "ThreadInfo", 10))
 			break;
-		}
 
 		i = 0;
 		remcom_out_buffer[0] = 'm';
@@ -665,10 +663,9 @@ static void gdb_cmd_query(struct kgdb_state *ks)
 		pack_threadid(remcom_out_buffer + 2, thref);
 		break;
 	case 'T':
-		if (memcmp(remcom_in_buffer + 1, "ThreadExtraInfo,", 16)) {
-			error_packet(remcom_out_buffer, -EINVAL);
+		if (memcmp(remcom_in_buffer + 1, "ThreadExtraInfo,", 16))
 			break;
-		}
+
 		ks->threadid = 0;
 		ptr = remcom_in_buffer + 17;
 		kgdb_hex2long(&ptr, &ks->threadid);
-- 
1.6.3.3

[PATCH 3/5] Fix merge regression from external kdb to upstream kdb

[Jason Wessel]

In the process of merging kdb to the mainline, the kdb lsmod command
stopped printing the base load address of kernel modules.  This is
needed for using kdb in conjunction with external tools such as gdb.

Simply restore the functionality by adding a kdb_printf for the base
load address of the kernel modules.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 kernel/debug/kdb/kdb_main.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index a7fe2e9..7e9bfd5 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1883,6 +1883,7 @@ static int kdb_lsmod(int argc, const char **argv)
 			kdb_printf(" (Loading)");
 		else
 			kdb_printf(" (Live)");
+		kdb_printf(" 0x%p", mod->module_core);
 
 #ifdef CONFIG_MODULE_UNLOAD
 		{
-- 
1.6.3.3

[PATCH 4/5] debug_core,kdb: fix kgdb_connected bit set in the wrong place

[Jason Wessel]

Immediately following an exit from the kdb shell the kgdb_connected
variable should be set to zero, unless there are breakpoints planted.
If the kgdb_connected variable is not zeroed out with kdb, it is
impossible to turn off kdb.

This patch is merely a work around for now, the real fix will check
for the breakpoints.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 kernel/debug/debug_core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 5cb7cd1..8bc5eef 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -605,13 +605,13 @@ cpu_master_loop:
 		if (dbg_kdb_mode) {
 			kgdb_connected = 1;
 			error = kdb_stub(ks);
+			kgdb_connected = 0;
 		} else {
 			error = gdb_serial_stub(ks);
 		}
 
 		if (error == DBG_PASS_EVENT) {
 			dbg_kdb_mode = !dbg_kdb_mode;
-			kgdb_connected = 0;
 		} else if (error == DBG_SWITCH_CPU_EVENT) {
 			dbg_cpu_switch(cpu, dbg_switch_cpu);
 			goto cpu_loop;
-- 
1.6.3.3

[PATCH 5/5] sysrq,kdb: Use __handle_sysrq() for kdb's sysrq function

[Jason Wessel]

The kdb code should not toggle the sysrq state in case an end user
wants to try and resume the normal kernel execution.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/char/sysrq.c        |    2 +-
 include/linux/sysrq.h       |    1 +
 kernel/debug/kdb/kdb_main.c |    3 +--
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
index 5d64e3a..878ac0c 100644
--- a/drivers/char/sysrq.c
+++ b/drivers/char/sysrq.c
@@ -493,7 +493,7 @@ static void __sysrq_put_key_op(int key, struct sysrq_key_op *op_p)
                 sysrq_key_table[i] = op_p;
 }
 
-static void __handle_sysrq(int key, struct tty_struct *tty, int check_mask)
+void __handle_sysrq(int key, struct tty_struct *tty, int check_mask)
 {
 	struct sysrq_key_op *op_p;
 	int orig_log_level;
diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
index 4496322..609e8ca 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -45,6 +45,7 @@ struct sysrq_key_op {
  */
 
 void handle_sysrq(int key, struct tty_struct *tty);
+void __handle_sysrq(int key, struct tty_struct *tty, int check_mask);
 int register_sysrq_key(int key, struct sysrq_key_op *op);
 int unregister_sysrq_key(int key, struct sysrq_key_op *op);
 struct sysrq_key_op *__sysrq_get_key_op(int key);
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 7e9bfd5..ebe4a28 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1820,9 +1820,8 @@ static int kdb_sr(int argc, const char **argv)
 {
 	if (argc != 1)
 		return KDB_ARGCOUNT;
-	sysrq_toggle_support(1);
 	kdb_trap_printk++;
-	handle_sysrq(*argv[1], NULL);
+	__handle_sysrq(*argv[1], NULL, 0);
 	kdb_trap_printk--;
 
 	return 0;
-- 
1.6.3.3

[GIT PULL] kgdb regression fixes for 2.6.35-rc5

[Jason Wessel]

Linus, please pull the for_linus branch for 2.6.35-rc6.  

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git for_linus

The kgdb validation tests found a regression where HW breakpoints
stopped working with kdb / kgdb.  The problem was bisected and after
talking with Frederic Weisbecker a patch was generated to repair the
regression.

Thanks,
Jason.


---
Jason Wessel (1):
      x86,kgdb: Fix hw breakpoint regression

 arch/x86/kernel/kgdb.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

Re: [GIT PULL] kgdb regression fixes for 2.6.35-rc5

[Frederic Weisbecker]

On Wed, Jul 28, 2010 at 03:39:49PM -0500, Jason Wessel wrote:
> Linus, please pull the for_linus branch for 2.6.35-rc6.  
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git for_linus
> 
> The kgdb validation tests found a regression where HW breakpoints
> stopped working with kdb / kgdb.  The problem was bisected and after
> talking with Frederic Weisbecker a patch was generated to repair the
> regression.
> 
> Thanks,
> Jason.
> 
> 
> ---
> Jason Wessel (1):
>       x86,kgdb: Fix hw breakpoint regression
> 
>  arch/x86/kernel/kgdb.c |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)



I'm sorry I spot it a bit late.
There is a little issue in this patch, irqs won't
be restored:

@@ -588,9 +588,10 @@ int kgdb_ll_trap(int cmd, const char *str,
        if (!kgdb_io_module_registered)
                return NOTIFY_DONE;
 
+       local_irq_save(flags);
        return __kgdb_notify(&args, cmd);
+       local_irq_restore(flags);
 }

Re: [GIT PULL] kgdb regression fixes for 2.6.35-rc5

[Linus Torvalds]

On Wed, Jul 28, 2010 at 2:17 PM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
>
> I'm sorry I spot it a bit late.
> There is a little issue in this patch, irqs won't
> be restored:
>
> @@ -588,9 +588,10 @@ int kgdb_ll_trap(int cmd, const char *str,
>        if (!kgdb_io_module_registered)
>                return NOTIFY_DONE;
>
> +       local_irq_save(flags);
>        return __kgdb_notify(&args, cmd);
> +       local_irq_restore(flags);
>  }

Yeah, that's obviously crap. I also wonder why __kgdb_notify messes up
the flags to begin with.

             Linus

Re: [GIT PULL] kgdb regression fixes for 2.6.35-rc5

[Jason Wessel]

On 07/28/2010 04:26 PM, Linus Torvalds wrote:
> On Wed, Jul 28, 2010 at 2:17 PM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
>> I'm sorry I spot it a bit late.
>> There is a little issue in this patch, irqs won't
>> be restored:
>>
>> @@ -588,9 +588,10 @@ int kgdb_ll_trap(int cmd, const char *str,
>>        if (!kgdb_io_module_registered)
>>                return NOTIFY_DONE;
>>
>> +       local_irq_save(flags);
>>        return __kgdb_notify(&args, cmd);
>> +       local_irq_restore(flags);
>>  }
> 
> Yeah, that's obviously crap. I also wonder why __kgdb_notify messes up
> the flags to begin with.
> 

This should not be needed as the irqs should already be off while in the overflow handler.

Sorry for the noise on the one.   I updated the for_linus branch with the final version.


 1 files changed, 7 insertions(+), 2 deletions(-)


--
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -572,7 +572,6 @@ static int __kgdb_notify(struct die_args *args, unsigned long cmd)
 	return NOTIFY_STOP;
 }
 
-#ifdef CONFIG_KGDB_LOW_LEVEL_TRAP
 int kgdb_ll_trap(int cmd, const char *str,
 		 struct pt_regs *regs, long err, int trap, int sig)
 {
@@ -590,7 +589,6 @@ int kgdb_ll_trap(int cmd, const char *str,
 
 	return __kgdb_notify(&args, cmd);
 }
-#endif /* CONFIG_KGDB_LOW_LEVEL_TRAP */
 
 static int
 kgdb_notify(struct notifier_block *self, unsigned long cmd, void *ptr)
@@ -625,6 +623,12 @@ int kgdb_arch_init(void)
 	return register_die_notifier(&kgdb_notifier);
 }
 
+static void kgdb_hw_overflow_handler(struct perf_event *event, int nmi,
+		struct perf_sample_data *data, struct pt_regs *regs)
+{
+	kgdb_ll_trap(DIE_DEBUG, "debug", regs, 0, 0, SIGTRAP);
+}
+
 void kgdb_arch_late(void)
 {
 	int i, cpu;
@@ -655,6 +659,7 @@ void kgdb_arch_late(void)
 		for_each_online_cpu(cpu) {
 			pevent = per_cpu_ptr(breakinfo[i].pev, cpu);
 			pevent[0]->hw.sample_period = 1;
+			pevent[0]->overflow_handler = kgdb_hw_overflow_handler;
 			if (pevent[0]->destroy != NULL) {
 				pevent[0]->destroy = NULL;
 				release_bp_slot(*pevent);

Re: [GIT PULL] kgdb regression fixes for 2.6.35-rc5

[Frederic Weisbecker]

On Wed, Jul 28, 2010 at 07:14:42PM -0500, Jason Wessel wrote:
> On 07/28/2010 04:26 PM, Linus Torvalds wrote:
> > On Wed, Jul 28, 2010 at 2:17 PM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >> I'm sorry I spot it a bit late.
> >> There is a little issue in this patch, irqs won't
> >> be restored:
> >>
> >> @@ -588,9 +588,10 @@ int kgdb_ll_trap(int cmd, const char *str,
> >>        if (!kgdb_io_module_registered)
> >>                return NOTIFY_DONE;
> >>
> >> +       local_irq_save(flags);
> >>        return __kgdb_notify(&args, cmd);
> >> +       local_irq_restore(flags);
> >>  }
> > 
> > Yeah, that's obviously crap. I also wonder why __kgdb_notify messes up
> > the flags to begin with.
> > 
> 
> This should not be needed as the irqs should already be off while in the overflow handler.



Interrupts are not disabled in the debug exception handler.

I'm not sure what would be the state of dr6 if a nested debug exception
would happen while do_debug() is interrupted.

Perhaps the previous dr6 value is dropped. Intel manuals don't seem to say anything
about that.

Anyway that's a separate issue. The current patch fixes a regression and
it looks good.

Thanks.