[git pull] kgdb 2.6.30-rc5 regression fixes

[git pull] kgdb 2.6.30-rc5 regression fixes

[Jason Wessel]

Linus, please pull the kgdb git tree fixes for 2.6.30-rc5

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

These are 3 kgdb regressions that have had fixes for some time.  They
are all well tested so it would be worth it to get them merged.

The piece that touches the sysrq change was already reviewed by the
owners of the code and includes the appropriate ack's.

Thanks,
Jason.

Short log follows:

Frank Rowand (1):
      kgdb: gdb documentation fix

Jason Wessel (2):
      sysrq, intel_fb: fix sysrq g collision
      kgdb,i386: use address that SP register points to in the exception frame

 Documentation/DocBook/kgdb.tmpl |    2 +-
 arch/x86/kernel/kgdb.c          |    3 ++-
 drivers/char/sysrq.c            |    4 ++--
 drivers/gpu/drm/i915/intel_fb.c |    4 ++--
 kernel/kgdb.c                   |    4 ++--
 5 files changed, 9 insertions(+), 8 deletions(-)

[PATCH 1/3] sysrq, intel_fb: fix sysrq g collision

[Jason Wessel]

Commit 79e539453b34e35f39299a899d263b0a1f1670bd introduced a
regression where you cannot use sysrq 'g' to enter kgdb.  The solution
is to move the intel fb sysrq over to V for video instead of G for
graphics.  The SMP VOYAGER code to register for the sysrq-v is not
anywhere to be found in the mainline kernel, so the comments in the
code were cleaned up as well.

This patch also cleans up the sysrq definitions for kgdb to make it
generic for the kernel debugger, such that the sysrq 'g' can be used
in the future to enter a gdbstub or another kernel debugger.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Acked-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/char/sysrq.c            |    4 ++--
 drivers/gpu/drm/i915/intel_fb.c |    4 ++--
 kernel/kgdb.c                   |    4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
index b0a6a3e..d6a807f 100644
--- a/drivers/char/sysrq.c
+++ b/drivers/char/sysrq.c
@@ -406,7 +406,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
 	&sysrq_showlocks_op,		/* d */
 	&sysrq_term_op,			/* e */
 	&sysrq_moom_op,			/* f */
-	/* g: May be registered by ppc for kgdb */
+	/* g: May be registered for the kernel debugger */
 	NULL,				/* g */
 	NULL,				/* h - reserved for help */
 	&sysrq_kill_op,			/* i */
@@ -431,7 +431,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
 	&sysrq_sync_op,			/* s */
 	&sysrq_showstate_op,		/* t */
 	&sysrq_mountro_op,		/* u */
-	/* v: May be registered at init time by SMP VOYAGER */
+	/* v: May be registered for frame buffer console restore */
 	NULL,				/* v */
 	&sysrq_showstate_blocked_op,	/* w */
 	/* x: May be registered on ppc/powerpc for xmon */
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 3e094be..e4652dc 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -864,7 +864,7 @@ static void intelfb_sysrq(int dummy1, struct tty_struct *dummy3)
 
 static struct sysrq_key_op sysrq_intelfb_restore_op = {
         .handler = intelfb_sysrq,
-        .help_msg = "force-fb(G)",
+        .help_msg = "force-fb(V)",
         .action_msg = "Restore framebuffer console",
 };
 
@@ -898,7 +898,7 @@ int intelfb_probe(struct drm_device *dev)
 		ret = intelfb_single_fb_probe(dev);
 	}
 
-	register_sysrq_key('g', &sysrq_intelfb_restore_op);
+	register_sysrq_key('v', &sysrq_intelfb_restore_op);
 
 	return ret;
 }
diff --git a/kernel/kgdb.c b/kernel/kgdb.c
index e4dcfb2..9147a31 100644
--- a/kernel/kgdb.c
+++ b/kernel/kgdb.c
@@ -1583,8 +1583,8 @@ static void sysrq_handle_gdb(int key, struct tty_struct *tty)
 
 static struct sysrq_key_op sysrq_gdb_op = {
 	.handler	= sysrq_handle_gdb,
-	.help_msg	= "Gdb",
-	.action_msg	= "GDB",
+	.help_msg	= "debug(G)",
+	.action_msg	= "DEBUG",
 };
 #endif
 
-- 
1.6.3.1.9.g95405b

[PATCH 2/3] kgdb,i386: use address that SP register points to in the exception frame

[Jason Wessel]

The treatment of the SP register is different on x86_64 and i386.
This is a regression fix that lived outside the mainline kernel from
2.6.27 to now.  The regression was a result of the original merge
consolidation of the i386 and x86_64 archs to x86.

The incorrectly reported SP on i386 prevented stack tracebacks from
working correctly in gdb.

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

diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index eedfaeb..b1f4dff 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -88,6 +88,7 @@ void pt_regs_to_gdb_regs(unsigned long *gdb_regs, struct pt_regs *regs)
 	gdb_regs[GDB_SS]	= __KERNEL_DS;
 	gdb_regs[GDB_FS]	= 0xFFFF;
 	gdb_regs[GDB_GS]	= 0xFFFF;
+	gdb_regs[GDB_SP]	= (int)&regs->sp;
 #else
 	gdb_regs[GDB_R8]	= regs->r8;
 	gdb_regs[GDB_R9]	= regs->r9;
@@ -100,8 +101,8 @@ void pt_regs_to_gdb_regs(unsigned long *gdb_regs, struct pt_regs *regs)
 	gdb_regs32[GDB_PS]	= regs->flags;
 	gdb_regs32[GDB_CS]	= regs->cs;
 	gdb_regs32[GDB_SS]	= regs->ss;
-#endif
 	gdb_regs[GDB_SP]	= regs->sp;
+#endif
 }
 
 /**
-- 
1.6.3.1.9.g95405b

[PATCH 3/3] kgdb: gdb documentation fix

[Jason Wessel]

From: Frank Rowand <frank.rowand@am.sony.com>

gdb command "set remote debug 1" is not valid, change to correct command.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 Documentation/DocBook/kgdb.tmpl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/DocBook/kgdb.tmpl b/Documentation/DocBook/kgdb.tmpl
index 372dec2..5cff41a 100644
--- a/Documentation/DocBook/kgdb.tmpl
+++ b/Documentation/DocBook/kgdb.tmpl
@@ -281,7 +281,7 @@
     seriously wrong while debugging, it will most often be the case
     that you want to enable gdb to be verbose about its target
     communications.  You do this prior to issuing the <constant>target
-    remote</constant> command by typing in: <constant>set remote debug 1</constant>
+    remote</constant> command by typing in: <constant>set debug remote 1</constant>
     </para>
   </chapter>
   <chapter id="KGDBTestSuite">
-- 
1.6.3.1.9.g95405b

Re: [PATCH 2/3] kgdb,i386: use address that SP register points to in the exception frame

[Linus Torvalds]

On Fri, 15 May 2009, Jason Wessel wrote:
>
> The treatment of the SP register is different on x86_64 and i386.
> This is a regression fix that lived outside the mainline kernel from
> 2.6.27 to now.  The regression was a result of the original merge
> consolidation of the i386 and x86_64 archs to x86.
> 
> The incorrectly reported SP on i386 prevented stack tracebacks from
> working correctly in gdb.

Is this only ever used for kernel register state?

Because in the _general_ case, the code should likely be something like

	if (user_mode_vm(regs)) {
		gdb_regs[GDB_SS] = regs->ss;
		gdb_regs[GDB_SP] = regs->sp;
	} else {
		gdb_regs[GDB_SS] = __KERNEL_DS;
		gdb_regs[GDB_SP] = (unsigned long)&regs->sp
	}

if the 'regs' contents can ever point to user mode state.

		Linus

Re: [PATCH 2/3] kgdb,i386: use address that SP register points to in the exception frame

[Jason Wessel]

[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]

Linus Torvalds wrote:
> Is this only ever used for kernel register state?
>
> Because in the _general_ case, the code should likely be something like
>
> 	if (user_mode_vm(regs)) {
> 		gdb_regs[GDB_SS] = regs->ss;
> 		gdb_regs[GDB_SP] = regs->sp;
> 	} else {
> 		gdb_regs[GDB_SS] = __KERNEL_DS;
> 		gdb_regs[GDB_SP] = (unsigned long)&regs->sp
> 	}
>   

You have discovered a long standing corner case.  The only way you can
end up with user_mode_vm() being true is the death by NMI watch dog or
from the IPI to bring the non master kgdb cpus into debugger. By default
the kgdb exception handler is not going to end up with that condition,
because there is a check against regs in the kgdb notify handler for the
non NMI/IPI events.

Thank you for the suggestion.   I went ahead and tested it out to
confirm the behavior, as well as to run the standard set of kgdb
regression tests.

The corner case is fixed in the attached patch, and I updated for_linus
branch with just this patch, if you would prefer to pull it.

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

Thanks,
Jason.

[-- Attachment #2: 0001-kgdb-i386-Fix-corner-case-access-to-sp-with-NMI-watc.patch --]
[-- Type: text/x-diff, Size: 1268 bytes --]

>From 0fff698b7a60d8f534dcc0d1ef26efb579938d09 Mon Sep 17 00:00:00 2001
From: Jason Wessel <jason.wessel@windriver.com>
Date: Fri, 15 May 2009 11:39:08 -0500
Subject: [PATCH 1/1] kgdb,i386: Fix corner case access to sp with NMI watch dog exception

It is possible for the user_mode_vm(regs) check to return true for a
non master kgdb cpu or when the master kgdb cpu handles the NMI watch
dog exception.

The solution is simply to select the correct stack pointer location
based on the check to user_mode_vm(regs).

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>

---
 arch/x86/kernel/kgdb.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -85,10 +85,15 @@ void pt_regs_to_gdb_regs(unsigned long *
 	gdb_regs[GDB_DS]	= regs->ds;
 	gdb_regs[GDB_ES]	= regs->es;
 	gdb_regs[GDB_CS]	= regs->cs;
-	gdb_regs[GDB_SS]	= __KERNEL_DS;
 	gdb_regs[GDB_FS]	= 0xFFFF;
 	gdb_regs[GDB_GS]	= 0xFFFF;
-	gdb_regs[GDB_SP]	= (int)&regs->sp;
+	if (user_mode_vm(regs)) {
+		gdb_regs[GDB_SS] = regs->ss;
+		gdb_regs[GDB_SP] = regs->sp;
+	} else {
+		gdb_regs[GDB_SS] = __KERNEL_DS;
+		gdb_regs[GDB_SP] = (unsigned long)&regs->sp;
+	}
 #else
 	gdb_regs[GDB_R8]	= regs->r8;
 	gdb_regs[GDB_R9]	= regs->r9;