linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger
@ 2012-09-13 15:01 Anton Vorontsov
  2012-09-13 15:03 ` [PATCH 01/11] kernel/debug: Mask KGDB NMI upon entry Anton Vorontsov
                   ` (12 more replies)
  0 siblings, 13 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 15:01 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

Hi all,

Here comes the lucky v7:

- Per Alan Cox's suggestion added hangup method and removed a small
  leftover;
- Per Colin Cross' suggestion moved IRQ quiescing logic into
  poll_get_char routine. IIUC, Alan is less unhappy about it.  As a
  result, clear_irq() callback dropped.

These patches can be found in the following repo (based on tty-next):

	git://git.infradead.org/users/cbou/linux-nmi-kdb.git master

Old changelogs and rationale for these patches can be found here:

	v1-v5, rationale: http://lkml.org/lkml/2012/9/10/2
	v6: http://lkml.org/lkml/2012/9/10/2

Thanks,

--
 arch/arm/Kconfig                    |  19 ++
 arch/arm/common/vic.c               |  28 ++
 arch/arm/include/asm/hardware/vic.h |   2 +
 arch/arm/include/asm/kgdb.h         |   8 +
 arch/arm/kernel/Makefile            |   1 +
 arch/arm/kernel/entry-armv.S        | 167 +-----------
 arch/arm/kernel/entry-header.S      | 170 +++++++++++++
 arch/arm/kernel/kgdb_fiq.c          |  99 ++++++++
 arch/arm/kernel/kgdb_fiq_entry.S    |  87 +++++++
 arch/arm/mach-versatile/Makefile    |   1 +
 arch/arm/mach-versatile/kgdb_fiq.c  |  31 +++
 drivers/tty/serial/Kconfig          |  19 ++
 drivers/tty/serial/Makefile         |   1 +
 drivers/tty/serial/amba-pl011.c     |  73 +++++-
 drivers/tty/serial/kgdb_nmi.c       | 396 +++++++++++++++++++++++++++++
 drivers/tty/serial/kgdboc.c         |   6 +
 drivers/tty/serial/serial_core.c    |  17 ++
 include/linux/kdb.h                 |  29 ++-
 include/linux/kgdb.h                |  33 +++
 include/linux/serial_core.h         |   1 +
 kernel/debug/debug_core.c           |  36 ++-
 kernel/debug/kdb/kdb_main.c         |  29 +++
 22 files changed, 1060 insertions(+), 193 deletions(-)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 01/11] kernel/debug: Mask KGDB NMI upon entry
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
@ 2012-09-13 15:03 ` Anton Vorontsov
  2012-09-19 11:52   ` Jason Wessel
  2012-09-13 15:03 ` [PATCH 02/11] kdb: Implement disable_nmi command Anton Vorontsov
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 15:03 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

The new arch callback should manage NMIs that usually cause KGDB to
enter. That is, not all NMIs should be enabled/disabled, but only
those that issue kgdb_handle_exception().

We must mask it as serial-line interrupt can be used as an NMI, so
if the original KGDB-entry cause was say a breakpoint, then every
input to KDB console will cause KGDB to reenter, which we don't want.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 include/linux/kgdb.h      | 23 +++++++++++++++++++++++
 kernel/debug/debug_core.c | 36 +++++++++++++++++++++++++++++++++---
 2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index c4d2fc1..3b111a6 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -221,6 +221,29 @@ extern int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt);
  */
 extern void kgdb_arch_late(void);
 
+/**
+ *	kgdb_arch_enable_nmi - Enable or disable KGDB-entry NMI
+ *	@on: Flag to either enable or disable an NMI
+ *
+ *	This is an architecture-specific "back-end" for kgdb_enable_nmi(). The
+ *	call does not count disable/enable requests, do not use it directly.
+ */
+extern void kgdb_arch_enable_nmi(bool on);
+
+/**
+ *	kgdb_enable_nmi - Enable or disable KGDB-entry NMI
+ *	@on: Flag to either enable or disable an NMI
+ *
+ *	This function manages NMIs that usually cause KGDB to enter. That is,
+ *	not all NMIs should be enabled or disabled, but only those that issue
+ *	kgdb_handle_exception().
+ *
+ *	The call counts disable requests, and thus allows to nest disables.
+ *	But trying to enable already enabled NMI is an error. The call returns
+ *	1 if NMI has been actually enabled after the call, and a value <= 0 if
+ *	it is still disabled.
+ */
+extern int kgdb_enable_nmi(bool on);
 
 /**
  * struct kgdb_arch - Describe architecture specific values.
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 0557f24..b621d1e 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -214,6 +214,30 @@ int __weak kgdb_skipexception(int exception, struct pt_regs *regs)
 	return 0;
 }
 
+void __weak kgdb_arch_enable_nmi(bool on)
+{
+}
+
+int kgdb_enable_nmi(bool on)
+{
+	static atomic_t cnt;
+	int ret;
+
+	ret = atomic_add_return(on ? 1 : -1, &cnt);
+	if (ret > 1 && on) {
+		/*
+		 * There should be only one instance that calls this function
+		 * in "enable, disable" order. All other users must call
+		 * disable first, then enable. If not, something is wrong.
+		 */
+		WARN_ON(1);
+		return 1;
+	}
+
+	kgdb_arch_enable_nmi(ret > 0);
+	return ret;
+}
+
 /*
  * Some architectures need cache flushes when we set/clear a
  * breakpoint:
@@ -672,6 +696,9 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
 {
 	struct kgdb_state kgdb_var;
 	struct kgdb_state *ks = &kgdb_var;
+	int ret = 0;
+
+	kgdb_enable_nmi(0);
 
 	ks->cpu			= raw_smp_processor_id();
 	ks->ex_vector		= evector;
@@ -681,11 +708,14 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
 	ks->linux_regs		= regs;
 
 	if (kgdb_reenter_check(ks))
-		return 0; /* Ouch, double exception ! */
+		goto out; /* Ouch, double exception ! */
 	if (kgdb_info[ks->cpu].enter_kgdb != 0)
-		return 0;
+		goto out;
 
-	return kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
+	ret = kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
+out:
+	kgdb_enable_nmi(1);
+	return ret;
 }
 
 int kgdb_nmicallback(int cpu, void *regs)
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 02/11] kdb: Implement disable_nmi command
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
  2012-09-13 15:03 ` [PATCH 01/11] kernel/debug: Mask KGDB NMI upon entry Anton Vorontsov
@ 2012-09-13 15:03 ` Anton Vorontsov
  2012-09-19 11:52   ` Jason Wessel
  2012-09-13 15:03 ` [PATCH 03/11] kdb: Turn KGDB_KDB=n stubs into static inlines Anton Vorontsov
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 15:03 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

This command disables NMI-entry. If NMI source has been previously shared
with a serial console ("debug port"), this effectively releases the port
from KDB exclusive use, and makes the console available for normal use.

Of course, NMI can be reenabled, enable_nmi modparam is used for that:

	echo 1 > /sys/module/kdb/parameters/enable_nmi

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 kernel/debug/kdb/kdb_main.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 31df170..9fadff1 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -21,6 +21,7 @@
 #include <linux/smp.h>
 #include <linux/utsname.h>
 #include <linux/vmalloc.h>
+#include <linux/atomic.h>
 #include <linux/module.h>
 #include <linux/mm.h>
 #include <linux/init.h>
@@ -2107,6 +2108,32 @@ static int kdb_dmesg(int argc, const char **argv)
 	return 0;
 }
 #endif /* CONFIG_PRINTK */
+
+/* Make sure we balance enable/disable calls, must disable first. */
+static atomic_t kdb_nmi_disabled;
+
+static int kdb_disable_nmi(int argc, const char *argv[])
+{
+	if (atomic_read(&kdb_nmi_disabled))
+		return 0;
+	atomic_set(&kdb_nmi_disabled, 1);
+	kgdb_enable_nmi(0);
+	return 0;
+}
+
+static int kdb_param_enable_nmi(const char *val, const struct kernel_param *kp)
+{
+	if (!atomic_add_unless(&kdb_nmi_disabled, -1, 0))
+		return -EINVAL;
+	kgdb_enable_nmi(1);
+	return 0;
+}
+
+static const struct kernel_param_ops kdb_param_ops_enable_nmi = {
+	.set = kdb_param_enable_nmi,
+};
+module_param_cb(enable_nmi, &kdb_param_ops_enable_nmi, NULL, 0600);
+
 /*
  * kdb_cpu - This function implements the 'cpu' command.
  *	cpu	[<cpunum>]
@@ -2851,6 +2878,8 @@ static void __init kdb_inittab(void)
 	kdb_register_repeat("dmesg", kdb_dmesg, "[lines]",
 	  "Display syslog buffer", 0, KDB_REPEAT_NONE);
 #endif
+	kdb_register_repeat("disable_nmi", kdb_disable_nmi, "",
+	  "Disable NMI entry to KDB", 0, KDB_REPEAT_NONE);
 	kdb_register_repeat("defcmd", kdb_defcmd, "name \"usage\" \"help\"",
 	  "Define a set of commands, down to endefcmd", 0, KDB_REPEAT_NONE);
 	kdb_register_repeat("kill", kdb_kill, "<-signal> <pid>",
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 03/11] kdb: Turn KGDB_KDB=n stubs into static inlines
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
  2012-09-13 15:03 ` [PATCH 01/11] kernel/debug: Mask KGDB NMI upon entry Anton Vorontsov
  2012-09-13 15:03 ` [PATCH 02/11] kdb: Implement disable_nmi command Anton Vorontsov
@ 2012-09-13 15:03 ` Anton Vorontsov
  2012-09-13 15:03 ` [PATCH 04/11] tty/serial/core: Introduce poll_init callback Anton Vorontsov
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 15:03 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

This makes the stubs actually usable, since e.g. 'foo = kdb_register();'
leads to build errors in !KGDB_KDB case. Plus, with static inlines we
do type checking.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 include/linux/kdb.h | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 42d9e86..7f6fe6e 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -13,6 +13,14 @@
  * Copyright (C) 2009 Jason Wessel <jason.wessel@windriver.com>
  */
 
+typedef enum {
+	KDB_REPEAT_NONE = 0,	/* Do not repeat this command */
+	KDB_REPEAT_NO_ARGS,	/* Repeat the command without arguments */
+	KDB_REPEAT_WITH_ARGS,	/* Repeat the command including its arguments */
+} kdb_repeat_t;
+
+typedef int (*kdb_func_t)(int, const char **);
+
 #ifdef	CONFIG_KGDB_KDB
 #include <linux/init.h>
 #include <linux/sched.h>
@@ -32,14 +40,6 @@ extern atomic_t kdb_event;
 
 #define KDB_MAXARGS    16 /* Maximum number of arguments to a function  */
 
-typedef enum {
-	KDB_REPEAT_NONE = 0,	/* Do not repeat this command */
-	KDB_REPEAT_NO_ARGS,	/* Repeat the command without arguments */
-	KDB_REPEAT_WITH_ARGS,	/* Repeat the command including its arguments */
-} kdb_repeat_t;
-
-typedef int (*kdb_func_t)(int, const char **);
-
 /* KDB return codes from a command or internal kdb function */
 #define KDB_NOTFOUND	(-1)
 #define KDB_ARGCOUNT	(-2)
@@ -149,11 +149,14 @@ extern int kdb_register_repeat(char *, kdb_func_t, char *, char *,
 			       short, kdb_repeat_t);
 extern int kdb_unregister(char *);
 #else /* ! CONFIG_KGDB_KDB */
-#define kdb_printf(...)
-#define kdb_init(x)
-#define kdb_register(...)
-#define kdb_register_repeat(...)
-#define kdb_uregister(x)
+static inline __printf(1, 2) int kdb_printf(const char *fmt, ...) { return 0; }
+static inline void kdb_init(int level) {}
+static inline int kdb_register(char *cmd, kdb_func_t func, char *usage,
+			       char *help, short minlen) { return 0; }
+static inline int kdb_register_repeat(char *cmd, kdb_func_t func, char *usage,
+				      char *help, short minlen,
+				      kdb_repeat_t repeat) { return 0; }
+static inline int kdb_unregister(char *cmd) { return 0; }
 #endif	/* CONFIG_KGDB_KDB */
 enum {
 	KDB_NOT_INITIALIZED,
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 04/11] tty/serial/core: Introduce poll_init callback
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
                   ` (2 preceding siblings ...)
  2012-09-13 15:03 ` [PATCH 03/11] kdb: Turn KGDB_KDB=n stubs into static inlines Anton Vorontsov
@ 2012-09-13 15:03 ` Anton Vorontsov
  2012-09-13 15:03 ` [PATCH 05/11] tty/serial/amba-pl011: Implement " Anton Vorontsov
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 15:03 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

It was noticed that polling drivers (like KGDB) are not able to use
serial ports if the ports were not previously initialized via console.
I.e.  when booting with console=ttyAMA0 kgdboc=ttyAMA0, everything works
fine, but with console=ttyFOO kgdboc=ttyAMA0, the kgdboc doesn't work.

This is because we don't initialize the hardware. Calling ->startup() is
not an option, because drivers request interrupts there, and drivers
fail to handle situations when tty isn't opened with interrupts enabled.

So, we have to implement a new callback (actually, tty_ops already have
a similar callback), which does everything needed to initialize just the
hardware.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 drivers/tty/serial/serial_core.c | 17 +++++++++++++++++
 include/linux/serial_core.h      |  1 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 046279c..dcb2d5a 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2129,6 +2129,7 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
 	int bits = 8;
 	int parity = 'n';
 	int flow = 'n';
+	int ret;
 
 	if (!state || !state->uart_port)
 		return -1;
@@ -2137,6 +2138,22 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
 	if (!(port->ops->poll_get_char && port->ops->poll_put_char))
 		return -1;
 
+	if (port->ops->poll_init) {
+		struct tty_port *tport = &state->port;
+
+		ret = 0;
+		mutex_lock(&tport->mutex);
+		/*
+		 * We don't set ASYNCB_INITIALIZED as we only initialized the
+		 * hw, e.g. state->xmit is still uninitialized.
+		 */
+		if (!test_bit(ASYNCB_INITIALIZED, &tport->flags))
+			ret = port->ops->poll_init(port);
+		mutex_unlock(&tport->mutex);
+		if (ret)
+			return ret;
+	}
+
 	if (options) {
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
 		return uart_set_options(port, NULL, baud, parity, bits, flow);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 7cf0b68..822c887 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -274,6 +274,7 @@ struct uart_ops {
 	int		(*verify_port)(struct uart_port *, struct serial_struct *);
 	int		(*ioctl)(struct uart_port *, unsigned int, unsigned long);
 #ifdef CONFIG_CONSOLE_POLL
+	int		(*poll_init)(struct uart_port *);
 	void	(*poll_put_char)(struct uart_port *, unsigned char);
 	int		(*poll_get_char)(struct uart_port *);
 #endif
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 05/11] tty/serial/amba-pl011: Implement poll_init callback
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
                   ` (3 preceding siblings ...)
  2012-09-13 15:03 ` [PATCH 04/11] tty/serial/core: Introduce poll_init callback Anton Vorontsov
@ 2012-09-13 15:03 ` Anton Vorontsov
  2012-09-13 15:03 ` [PATCH 06/11] tty/serial/amba-pl011: Quiesce interrupts in poll_get_char Anton Vorontsov
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 15:03 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

The callback is used to initialize the hardware, nothing else should be
done, i.e. we should not request interrupts (but we can and do unmask
some of them, as they might be useful for NMI entry).

As a side-effect, the patch also fixes a division by zero[1] when booting
with kgdboc options specified (e.g. kgdboc=ttyAMA0,115200n8). The issue
happens because serial core calls set_termios callback, but the driver
doesn't know clock frequency, and thus cannot calculate proper baud rate
values.

[1]
WARNING: at drivers/tty/serial/serial_core.c:400 uart_get_baud_rate+0xe8/0x14c()
Modules linked in:
[<c0018e50>] (unwind_backtrace+0x0/0xf0) from [<c0020ae8>] (warn_slowpath_common+0x4c/0x64)
[<c0020ae8>] (warn_slowpath_common+0x4c/0x64) from [<c0020b1c>] (warn_slowpath_null+0x1c/0x24)
[<c0020b1c>] (warn_slowpath_null+0x1c/0x24) from [<c0185ed8>] (uart_get_baud_rate+0xe8/0x14c)
[<c0185ed8>] (uart_get_baud_rate+0xe8/0x14c) from [<c0187078>] (pl011_set_termios+0x48/0x278)
[<c0187078>] (pl011_set_termios+0x48/0x278) from [<c01850b0>] (uart_set_options+0xe8/0x114)
[<c01850b0>] (uart_set_options+0xe8/0x114) from [<c0185de4>] (uart_poll_init+0xd4/0xe0)
[<c0185de4>] (uart_poll_init+0xd4/0xe0) from [<c016da8c>] (tty_find_polling_driver+0x100/0x17c)
[<c016da8c>] (tty_find_polling_driver+0x100/0x17c) from [<c0188538>] (configure_kgdboc+0xc8/0x1b8)
[<c0188538>] (configure_kgdboc+0xc8/0x1b8) from [<c00088a4>] (do_one_initcall+0x30/0x168)
[<c00088a4>] (do_one_initcall+0x30/0x168) from [<c033784c>] (do_basic_setup+0x94/0xc8)
[<c033784c>] (do_basic_setup+0x94/0xc8) from [<c03378e0>] (kernel_init+0x60/0xf4)
[<c03378e0>] (kernel_init+0x60/0xf4) from [<c00144a0>] (kernel_thread_exit+0x0/0x8)
---[ end trace 7d41c9186f342c40 ]---
Division by zero in kernel.
[<c0018e50>] (unwind_backtrace+0x0/0xf0) from [<c014546c>] (Ldiv0+0x8/0x10)
[<c014546c>] (Ldiv0+0x8/0x10) from [<c0187098>] (pl011_set_termios+0x68/0x278)
[<c0187098>] (pl011_set_termios+0x68/0x278) from [<c01850b0>] (uart_set_options+0xe8/0x114)
[<c01850b0>] (uart_set_options+0xe8/0x114) from [<c0185de4>] (uart_poll_init+0xd4/0xe0)
[<c0185de4>] (uart_poll_init+0xd4/0xe0) from [<c016da8c>] (tty_find_polling_driver+0x100/0x17c)
[<c016da8c>] (tty_find_polling_driver+0x100/0x17c) from [<c0188538>] (configure_kgdboc+0xc8/0x1b8)
[<c0188538>] (configure_kgdboc+0xc8/0x1b8) from [<c00088a4>] (do_one_initcall+0x30/0x168)
[<c00088a4>] (do_one_initcall+0x30/0x168) from [<c033784c>] (do_basic_setup+0x94/0xc8)
[<c033784c>] (do_basic_setup+0x94/0xc8) from [<c03378e0>] (kernel_init+0x60/0xf4)
[<c03378e0>] (kernel_init+0x60/0xf4) from [<c00144a0>] (kernel_thread_exit+0x0/0x8)
Division by zero in kernel.
[<c0018e50>] (unwind_backtrace+0x0/0xf0) from [<c014546c>] (Ldiv0+0x8/0x10)
[<c014546c>] (Ldiv0+0x8/0x10) from [<c0183a98>] (uart_update_timeout+0x4c/0x5c)
[<c0183a98>] (uart_update_timeout+0x4c/0x5c) from [<c01870f8>] (pl011_set_termios+0xc8/0x278)
[<c01870f8>] (pl011_set_termios+0xc8/0x278) from [<c01850b0>] (uart_set_options+0xe8/0x114)
[<c01850b0>] (uart_set_options+0xe8/0x114) from [<c0185de4>] (uart_poll_init+0xd4/0xe0)
[<c0185de4>] (uart_poll_init+0xd4/0xe0) from [<c016da8c>] (tty_find_polling_driver+0x100/0x17c)
[<c016da8c>] (tty_find_polling_driver+0x100/0x17c) from [<c0188538>] (configure_kgdboc+0xc8/0x1b8)
[<c0188538>] (configure_kgdboc+0xc8/0x1b8) from [<c00088a4>] (do_one_initcall+0x30/0x168)
[<c00088a4>] (do_one_initcall+0x30/0x168) from [<c033784c>] (do_basic_setup+0x94/0xc8)
[<c033784c>] (do_basic_setup+0x94/0xc8) from [<c03378e0>] (kernel_init+0x60/0xf4)
[<c03378e0>] (kernel_init+0x60/0xf4) from [<c00144a0>] (kernel_thread_exit+0x0/0x8)

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 drivers/tty/serial/amba-pl011.c | 44 ++++++++++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index cede938..45137e4 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1309,10 +1309,9 @@ static void pl011_put_poll_char(struct uart_port *port,
 
 #endif /* CONFIG_CONSOLE_POLL */
 
-static int pl011_startup(struct uart_port *port)
+static int pl011_hwinit(struct uart_port *port)
 {
 	struct uart_amba_port *uap = (struct uart_amba_port *)port;
-	unsigned int cr;
 	int retval;
 
 	/* Optionaly enable pins to be muxed in and configured */
@@ -1337,6 +1336,37 @@ static int pl011_startup(struct uart_port *port)
 	       UART011_RTIS | UART011_RXIS, uap->port.membase + UART011_ICR);
 
 	/*
+	 * Save interrupts enable mask, and enable RX interrupts in case if
+	 * the interrupt is used for NMI entry.
+	 */
+	uap->im = readw(uap->port.membase + UART011_IMSC);
+	writew(UART011_RTIM | UART011_RXIM, uap->port.membase + UART011_IMSC);
+
+	if (uap->port.dev->platform_data) {
+		struct amba_pl011_data *plat;
+
+		plat = uap->port.dev->platform_data;
+		if (plat->init)
+			plat->init();
+	}
+	return 0;
+ out:
+	return retval;
+}
+
+static int pl011_startup(struct uart_port *port)
+{
+	struct uart_amba_port *uap = (struct uart_amba_port *)port;
+	unsigned int cr;
+	int retval;
+
+	retval = pl011_hwinit(port);
+	if (retval)
+		goto clk_dis;
+
+	writew(uap->im, uap->port.membase + UART011_IMSC);
+
+	/*
 	 * Allocate the IRQ
 	 */
 	retval = request_irq(uap->port.irq, pl011_int, 0, "uart-pl011", uap);
@@ -1395,19 +1425,10 @@ static int pl011_startup(struct uart_port *port)
 	writew(uap->im, uap->port.membase + UART011_IMSC);
 	spin_unlock_irq(&uap->port.lock);
 
-	if (uap->port.dev->platform_data) {
-		struct amba_pl011_data *plat;
-
-		plat = uap->port.dev->platform_data;
-		if (plat->init)
-			plat->init();
-	}
-
 	return 0;
 
  clk_dis:
 	clk_disable_unprepare(uap->clk);
- out:
 	return retval;
 }
 
@@ -1688,6 +1709,7 @@ static struct uart_ops amba_pl011_pops = {
 	.config_port	= pl011_config_port,
 	.verify_port	= pl011_verify_port,
 #ifdef CONFIG_CONSOLE_POLL
+	.poll_init     = pl011_hwinit,
 	.poll_get_char = pl011_get_poll_char,
 	.poll_put_char = pl011_put_poll_char,
 #endif
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 06/11] tty/serial/amba-pl011: Quiesce interrupts in poll_get_char
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
                   ` (4 preceding siblings ...)
  2012-09-13 15:03 ` [PATCH 05/11] tty/serial/amba-pl011: Implement " Anton Vorontsov
@ 2012-09-13 15:03 ` Anton Vorontsov
  2012-09-13 15:03 ` [PATCH 07/11] tty/serial: Add kgdb_nmi driver Anton Vorontsov
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 15:03 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

We need to quiesce interrupts in the poll_get_char routine, otherwise,
if used with KGDB NMI debugger, we'll keep reentering the NMI.

Quiescing interrupts is pretty straightforward, except for TXIM
interrupt. The interrupt has "ready to transmit" meaning, so it's
almost always raised, and the only way to silence it is to mask it. But
that's OK, ops->start_tx will unmask it.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 drivers/tty/serial/amba-pl011.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 45137e4..459f8ba 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1284,11 +1284,40 @@ static void pl011_break_ctl(struct uart_port *port, int break_state)
 }
 
 #ifdef CONFIG_CONSOLE_POLL
+
+static void pl011_quiesce_irqs(struct uart_port *port)
+{
+	struct uart_amba_port *uap = (struct uart_amba_port *)port;
+	unsigned char __iomem *regs = uap->port.membase;
+
+	writew(readw(regs + UART011_MIS), regs + UART011_ICR);
+	/*
+	 * There is no way to clear TXIM as this is "ready to transmit IRQ", so
+	 * we simply mask it. start_tx() will unmask it.
+	 *
+	 * Note we can race with start_tx(), and if the race happens, the
+	 * polling user might get another interrupt just after we clear it.
+	 * But it should be OK and can happen even w/o the race, e.g.
+	 * controller immediately got some new data and raised the IRQ.
+	 *
+	 * And whoever uses polling routines assumes that it manages the device
+	 * (including tx queue), so we're also fine with start_tx()'s caller
+	 * side.
+	 */
+	writew(readw(regs + UART011_IMSC) & ~UART011_TXIM, regs + UART011_IMSC);
+}
+
 static int pl011_get_poll_char(struct uart_port *port)
 {
 	struct uart_amba_port *uap = (struct uart_amba_port *)port;
 	unsigned int status;
 
+	/*
+	 * The caller might need IRQs lowered, e.g. if used with KDB NMI
+	 * debugger.
+	 */
+	pl011_quiesce_irqs(port);
+
 	status = readw(uap->port.membase + UART01x_FR);
 	if (status & UART01x_FR_RXFE)
 		return NO_POLL_CHAR;
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 07/11] tty/serial: Add kgdb_nmi driver
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
                   ` (5 preceding siblings ...)
  2012-09-13 15:03 ` [PATCH 06/11] tty/serial/amba-pl011: Quiesce interrupts in poll_get_char Anton Vorontsov
@ 2012-09-13 15:03 ` Anton Vorontsov
  2012-09-15 15:52   ` Francesco Lavra
  2012-09-19 11:52   ` [PATCH " Jason Wessel
  2012-09-13 15:03 ` [PATCH 08/11] ARM: Move some macros from entry-armv to entry-header Anton Vorontsov
                   ` (5 subsequent siblings)
  12 siblings, 2 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 15:03 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

This special driver makes it possible to temporary use NMI debugger port
as a normal console by issuing 'nmi_console' command (assuming that the
port is attached to KGDB).

Unlike KDB's disable_nmi command, with this driver you are always able
to go back to the debugger using KGDB escape sequence ($3#33).  This is
because this console driver processes the input in NMI context, and thus
is able to intercept the magic sequence.

Note that since the console interprets input and uses polling
communication methods, for things like PPP it is still better to fully
detach debugger port from the KGDB NMI (i.e. disable_nmi), and use raw
console.

Usually, to enter the debugger one have to type the magic sequence, so
initially the kernel will print the following prompt on the NMI debugger
console:

	Type $3#33 to enter the debugger>

For convenience, there is a kgdb_fiq.knock kernel command line option,
when set to 0, this turns the special command to just a return key
press, so the kernel will be printing this:

	Hit <return> to enter the debugger>

This is more convenient for long debugging sessions, although it makes
nmi_console feature somewhat useless.

And for the cases when NMI connected to a dedicated button, the knocking
can be disabled altogether by setting kgdb_fiq.knock to -1.

Suggested-by: Colin Cross <ccross@android.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 drivers/tty/serial/Kconfig    |  19 ++
 drivers/tty/serial/Makefile   |   1 +
 drivers/tty/serial/kgdb_nmi.c | 396 ++++++++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/kgdboc.c   |   6 +
 include/linux/kgdb.h          |  10 ++
 5 files changed, 432 insertions(+)
 create mode 100644 drivers/tty/serial/kgdb_nmi.c

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 26907cf..b22e45b 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -141,6 +141,25 @@ config SERIAL_ATMEL_TTYAT
 
 	  Say Y if you have an external 8250/16C550 UART.  If unsure, say N.
 
+config SERIAL_KGDB_NMI
+	bool "Serial console over KGDB NMI debugger port"
+	depends on KGDB_SERIAL_CONSOLE
+	help
+	  This special driver allows you to temporary use NMI debugger port
+	  as a normal console (assuming that the port is attached to KGDB).
+
+	  Unlike KDB's disable_nmi command, with this driver you are always
+	  able to go back to the debugger using KGDB escape sequence ($3#33).
+	  This is because this console driver processes the input in NMI
+	  context, and thus is able to intercept the magic sequence.
+
+	  Note that since the console interprets input and uses polling
+	  communication methods, for things like PPP you still must fully
+	  detach debugger port from the KGDB NMI (i.e. disable_nmi), and
+	  use raw console.
+
+	  If unsure, say N.
+
 config SERIAL_KS8695
 	bool "Micrel KS8695 (Centaur) serial port support"
 	depends on ARCH_KS8695
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index ce88667..4f694da 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_SERIAL_MSM_HS) += msm_serial_hs.o
 obj-$(CONFIG_SERIAL_NETX) += netx-serial.o
 obj-$(CONFIG_SERIAL_OF_PLATFORM) += of_serial.o
 obj-$(CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL) += nwpserial.o
+obj-$(CONFIG_SERIAL_KGDB_NMI) += kgdb_nmi.o
 obj-$(CONFIG_SERIAL_KS8695) += serial_ks8695.o
 obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
 obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o
diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c
new file mode 100644
index 0000000..57bf744
--- /dev/null
+++ b/drivers/tty/serial/kgdb_nmi.c
@@ -0,0 +1,396 @@
+/*
+ * KGDB NMI serial console
+ *
+ * Copyright 2010 Google, Inc.
+ *		  Arve Hjønnevåg <arve@android.com>
+ *		  Colin Cross <ccross@android.com>
+ * Copyright 2012 Linaro Ltd.
+ *		  Anton Vorontsov <anton.vorontsov@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/compiler.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/atomic.h>
+#include <linux/console.h>
+#include <linux/tty.h>
+#include <linux/tty_driver.h>
+#include <linux/tty_flip.h>
+#include <linux/interrupt.h>
+#include <linux/hrtimer.h>
+#include <linux/tick.h>
+#include <linux/kfifo.h>
+#include <linux/kgdb.h>
+#include <linux/kdb.h>
+
+static int kgdb_nmi_knock = 1;
+module_param_named(knock, kgdb_nmi_knock, int, 0600);
+MODULE_PARM_DESC(knock, "if set to 1 (default), the special '$3#33' command "
+			"must be used to enter the debugger; when set to 0, "
+			"hitting return key is enough to enter the debugger; "
+			"when set to -1, the debugger is entered immediately "
+			"upon NMI");
+
+static char *kgdb_nmi_magic = "$3#33";
+module_param_named(magic, kgdb_nmi_magic, charp, 0600);
+MODULE_PARM_DESC(magic, "magic sequence to enter NMI debugger (default $3#33)");
+
+static bool kgdb_nmi_tty_enabled;
+
+static void kgdb_nmi_console_write(struct console *co, const char *s, uint c)
+{
+	int i;
+
+	if (!kgdb_nmi_tty_enabled || atomic_read(&kgdb_active) >= 0)
+		return;
+
+	for (i = 0; i < c; i++)
+		dbg_io_ops->write_char(s[i]);
+}
+
+static struct tty_driver *kgdb_nmi_tty_driver;
+
+static struct tty_driver *kgdb_nmi_console_device(struct console *co, int *idx)
+{
+	*idx = co->index;
+	return kgdb_nmi_tty_driver;
+}
+
+static struct console kgdb_nmi_console = {
+	.name	= "ttyNMI",
+	.write	= kgdb_nmi_console_write,
+	.device	= kgdb_nmi_console_device,
+	.flags	= CON_PRINTBUFFER | CON_ANYTIME | CON_ENABLED,
+	.index	= -1,
+};
+
+/*
+ * This is usually the maximum rate on debug ports. We make fifo large enough
+ * to make copy-pasting to the terminal usable.
+ */
+#define KGDB_NMI_BAUD		115200
+#define KGDB_NMI_FIFO_SIZE	roundup_pow_of_two(KGDB_NMI_BAUD / 8 / HZ)
+
+struct kgdb_nmi_tty_priv {
+	struct tty_port port;
+	struct tasklet_struct tlet;
+	STRUCT_KFIFO(char, KGDB_NMI_FIFO_SIZE) fifo;
+};
+
+static struct kgdb_nmi_tty_priv *kgdb_nmi_port_to_priv(struct tty_port *port)
+{
+	return container_of(port, struct kgdb_nmi_tty_priv, port);
+}
+
+/*
+ * Our debugging console is polled in a tasklet, so we'll check for input
+ * every tick. In HZ-less mode, we should program the next tick.  We have
+ * to use the lowlevel stuff as no locks should be grabbed.
+ */
+#ifdef CONFIG_HIGH_RES_TIMERS
+static void kgdb_tty_poke(void)
+{
+	tick_program_event(ktime_get(), 0);
+}
+#else
+static inline void kgdb_tty_poke(void) {}
+#endif
+
+static struct tty_port *kgdb_nmi_port;
+
+static void kgdb_tty_recv(int ch)
+{
+	struct kgdb_nmi_tty_priv *priv;
+	char c = ch;
+
+	if (!kgdb_nmi_port || ch < 0)
+		return;
+	/*
+	 * Can't use port->tty->driver_data as tty might be not there. Tasklet
+	 * will check for tty and will get the ref, but here we don't have to
+	 * do that, and actually, we can't: we're in NMI context, no locks are
+	 * possible.
+	 */
+	priv = kgdb_nmi_port_to_priv(kgdb_nmi_port);
+	kfifo_in(&priv->fifo, &c, 1);
+	kgdb_tty_poke();
+}
+
+static int kgdb_nmi_poll_one_knock(void)
+{
+	static int n;
+	int c = -1;
+	const char *magic = kgdb_nmi_magic;
+	size_t m = strlen(magic);
+	bool printch = 0;
+
+	c = dbg_io_ops->read_char();
+	if (c == NO_POLL_CHAR)
+		return c;
+
+	if (!kgdb_nmi_knock && (c == '\r' || c == '\n')) {
+		return 1;
+	} else if (c == magic[n]) {
+		n = (n + 1) % m;
+		if (!n)
+			return 1;
+		printch = 1;
+	} else {
+		n = 0;
+	}
+
+	if (kgdb_nmi_tty_enabled) {
+		kgdb_tty_recv(c);
+		return 0;
+	}
+
+	if (printch) {
+		kdb_printf("%c", c);
+		return 0;
+	}
+
+	kdb_printf("\r%s %s to enter the debugger> %*s",
+		   kgdb_nmi_knock ? "Type" : "Hit",
+		   kgdb_nmi_knock ? magic  : "<return>", m, "");
+	while (m--)
+		kdb_printf("\b");
+	return 0;
+}
+
+/**
+ * kgdb_nmi_poll_knock - Check if it is time to enter the debugger
+ *
+ * "Serial ports are often noisy, especially when muxed over another port (we
+ * often use serial over the headset connector). Noise on the async command
+ * line just causes characters that are ignored, on a command line that blocked
+ * execution noise would be catastrophic." -- Colin Cross
+ *
+ * So, this function implements KGDB/KDB knocking on the serial line: we won't
+ * enter the debugger until we receive a known magic phrase (which is actually
+ * "$3#33", known as "escape to KDB" command. There is also a relaxed variant
+ * of knocking, i.e. just pressing the return key is enough to enter the
+ * debugger. And if knocking is disabled, the function always returns 1.
+ */
+bool kgdb_nmi_poll_knock(void)
+{
+	if (kgdb_nmi_knock < 0)
+		return 1;
+
+	while (1) {
+		int ret;
+
+		ret = kgdb_nmi_poll_one_knock();
+		if (ret == NO_POLL_CHAR)
+			return 0;
+		else if (ret == 1)
+			break;
+	}
+	return 1;
+}
+
+/*
+ * The tasklet is cheap, it does not cause wakeups when reschedules itself,
+ * instead it waits for the next tick.
+ */
+static void kgdb_nmi_tty_receiver(unsigned long data)
+{
+	struct kgdb_nmi_tty_priv *priv = (void *)data;
+	struct tty_struct *tty;
+	char ch;
+
+	tasklet_schedule(&priv->tlet);
+
+	if (likely(!kgdb_nmi_tty_enabled || !kfifo_len(&priv->fifo)))
+		return;
+
+	/* Port is there, but tty might be hung up, check. */
+	tty = tty_port_tty_get(kgdb_nmi_port);
+	if (!tty)
+		return;
+
+	while (kfifo_out(&priv->fifo, &ch, 1))
+		tty_insert_flip_char(priv->port.tty, ch, TTY_NORMAL);
+	tty_flip_buffer_push(priv->port.tty);
+
+	tty_kref_put(tty);
+}
+
+static int kgdb_nmi_tty_activate(struct tty_port *port, struct tty_struct *tty)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	kgdb_nmi_port = port;
+	tasklet_schedule(&priv->tlet);
+	return 0;
+}
+
+static void kgdb_nmi_tty_shutdown(struct tty_port *port)
+{
+	struct kgdb_nmi_tty_priv *priv = port->tty->driver_data;
+
+	tasklet_kill(&priv->tlet);
+	kgdb_nmi_port = NULL;
+}
+
+static const struct tty_port_operations kgdb_nmi_tty_port_ops = {
+	.activate	= kgdb_nmi_tty_activate,
+	.shutdown	= kgdb_nmi_tty_shutdown,
+};
+
+static int kgdb_nmi_tty_install(struct tty_driver *drv, struct tty_struct *tty)
+{
+	struct kgdb_nmi_tty_priv *priv;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	INIT_KFIFO(priv->fifo);
+	tasklet_init(&priv->tlet, kgdb_nmi_tty_receiver, (unsigned long)priv);
+	tty_port_init(&priv->port);
+	priv->port.ops = &kgdb_nmi_tty_port_ops;
+	tty->driver_data = priv;
+
+	ret = tty_port_install(&priv->port, drv, tty);
+	if (ret) {
+		pr_err("%s: can't nstall tty port: %d\n", __func__, ret);
+		goto err;
+	}
+	return 0;
+err:
+	kfree(priv);
+	return ret;
+}
+
+static void kgdb_nmi_tty_cleanup(struct tty_struct *tty)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	tty->driver_data = NULL;
+	kfree(priv);
+}
+
+static int kgdb_nmi_tty_open(struct tty_struct *tty, struct file *file)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	return tty_port_open(&priv->port, tty, file);
+}
+
+static void kgdb_nmi_tty_close(struct tty_struct *tty, struct file *file)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	tty_port_close(&priv->port, tty, file);
+}
+
+static void kgdb_nmi_tty_hangup(struct tty_struct *tty)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	tty_port_hangup(&priv->port);
+}
+
+static int kgdb_nmi_tty_write_room(struct tty_struct *tty)
+{
+	/* Actually, we can handle any amount as we use polled writes. */
+	return 2048;
+}
+
+static int kgdb_nmi_tty_write(struct tty_struct *tty, const unchar *buf, int c)
+{
+	int i;
+
+	for (i = 0; i < c; i++)
+		dbg_io_ops->write_char(buf[i]);
+	return c;
+}
+
+static const struct tty_operations kgdb_nmi_tty_ops = {
+	.open		= kgdb_nmi_tty_open,
+	.close		= kgdb_nmi_tty_close,
+	.install	= kgdb_nmi_tty_install,
+	.cleanup	= kgdb_nmi_tty_cleanup,
+	.hangup		= kgdb_nmi_tty_hangup,
+	.write_room	= kgdb_nmi_tty_write_room,
+	.write		= kgdb_nmi_tty_write,
+};
+
+static int kgdb_nmi_enable_console(int argc, const char *argv[])
+{
+	kgdb_nmi_tty_enabled = !(argc == 1 && !strcmp(argv[1], "off"));
+	return 0;
+}
+
+int kgdb_register_nmi_console(void)
+{
+	int ret;
+
+	kgdb_nmi_tty_driver = alloc_tty_driver(1);
+	if (!kgdb_nmi_tty_driver) {
+		pr_err("%s: cannot allocate tty\n", __func__);
+		return -ENOMEM;
+	}
+	kgdb_nmi_tty_driver->driver_name	= "ttyNMI";
+	kgdb_nmi_tty_driver->name		= "ttyNMI";
+	kgdb_nmi_tty_driver->num		= 1;
+	kgdb_nmi_tty_driver->type		= TTY_DRIVER_TYPE_SERIAL;
+	kgdb_nmi_tty_driver->subtype		= SERIAL_TYPE_NORMAL;
+	kgdb_nmi_tty_driver->flags		= TTY_DRIVER_REAL_RAW;
+	kgdb_nmi_tty_driver->init_termios	= tty_std_termios;
+	tty_termios_encode_baud_rate(&kgdb_nmi_tty_driver->init_termios,
+				     KGDB_NMI_BAUD, KGDB_NMI_BAUD);
+	tty_set_operations(kgdb_nmi_tty_driver, &kgdb_nmi_tty_ops);
+
+	ret = tty_register_driver(kgdb_nmi_tty_driver);
+	if (ret) {
+		pr_err("%s: can't register tty driver: %d\n", __func__, ret);
+		goto err_drv_reg;
+	}
+
+	ret = kdb_register("nmi_console", kgdb_nmi_enable_console, "[off]",
+			   "switch to Linux NMI console", 0);
+	if (ret) {
+		pr_err("%s: can't register kdb command: %d\n", __func__, ret);
+		goto err_kdb_reg;
+	}
+
+	register_console(&kgdb_nmi_console);
+	kgdb_enable_nmi(1);
+
+	return 0;
+err_kdb_reg:
+	tty_unregister_driver(kgdb_nmi_tty_driver);
+err_drv_reg:
+	put_tty_driver(kgdb_nmi_tty_driver);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kgdb_register_nmi_console);
+
+int kgdb_unregister_nmi_console(void)
+{
+	int ret;
+
+	kgdb_enable_nmi(0);
+	kdb_unregister("nmi_console");
+
+	ret = unregister_console(&kgdb_nmi_console);
+	if (ret)
+		return ret;
+
+	ret = tty_unregister_driver(kgdb_nmi_tty_driver);
+	if (ret)
+		return ret;
+	put_tty_driver(kgdb_nmi_tty_driver);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kgdb_unregister_nmi_console);
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index 2b42a01..ed97cfd 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -145,6 +145,8 @@ __setup("kgdboc=", kgdboc_option_setup);
 
 static void cleanup_kgdboc(void)
 {
+	if (kgdb_unregister_nmi_console())
+		return;
 	kgdboc_unregister_kbd();
 	if (configured == 1)
 		kgdb_unregister_io_module(&kgdboc_io_ops);
@@ -198,6 +200,10 @@ do_register:
 	if (err)
 		goto noconfig;
 
+	err = kgdb_register_nmi_console();
+	if (err)
+		goto noconfig;
+
 	configured = 1;
 
 	return 0;
diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 3b111a6..9bf888e 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -245,6 +245,16 @@ extern void kgdb_arch_enable_nmi(bool on);
  */
 extern int kgdb_enable_nmi(bool on);
 
+#ifdef CONFIG_SERIAL_KGDB_NMI
+extern int kgdb_register_nmi_console(void);
+extern int kgdb_unregister_nmi_console(void);
+extern bool kgdb_nmi_poll_knock(void);
+#else
+static inline int kgdb_register_nmi_console(void) { return 0; }
+static inline int kgdb_unregister_nmi_console(void) { return 0; }
+static inline bool kgdb_nmi_poll_knock(void) { return 1; }
+#endif
+
 /**
  * struct kgdb_arch - Describe architecture specific values.
  * @gdb_bpt_instr: The instruction to trigger a breakpoint.
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 08/11] ARM: Move some macros from entry-armv to entry-header
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
                   ` (6 preceding siblings ...)
  2012-09-13 15:03 ` [PATCH 07/11] tty/serial: Add kgdb_nmi driver Anton Vorontsov
@ 2012-09-13 15:03 ` Anton Vorontsov
  2012-09-13 15:03 ` [PATCH 09/11] ARM: Add KGDB/KDB FIQ debugger generic code Anton Vorontsov
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 15:03 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

Just move the macros into header file as we would want to use them for
KGDB FIQ entry code.

The following macros were moved:

 - svc_entry
 - usr_entry
 - kuser_cmpxchg_check
 - vector_stub

To make kuser_cmpxchg_check actually work across different files, we
also have to make kuser_cmpxchg64_fixup global.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 arch/arm/kernel/entry-armv.S   | 167 +---------------------------------------
 arch/arm/kernel/entry-header.S | 170 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 171 insertions(+), 166 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 0f82098..0f15368 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -136,57 +136,6 @@ common_invalid:
 	b	bad_mode
 ENDPROC(__und_invalid)
 
-/*
- * SVC mode handlers
- */
-
-#if defined(CONFIG_AEABI) && (__LINUX_ARM_ARCH__ >= 5)
-#define SPFIX(code...) code
-#else
-#define SPFIX(code...)
-#endif
-
-	.macro	svc_entry, stack_hole=0
- UNWIND(.fnstart		)
- UNWIND(.save {r0 - pc}		)
-	sub	sp, sp, #(S_FRAME_SIZE + \stack_hole - 4)
-#ifdef CONFIG_THUMB2_KERNEL
- SPFIX(	str	r0, [sp]	)	@ temporarily saved
- SPFIX(	mov	r0, sp		)
- SPFIX(	tst	r0, #4		)	@ test original stack alignment
- SPFIX(	ldr	r0, [sp]	)	@ restored
-#else
- SPFIX(	tst	sp, #4		)
-#endif
- SPFIX(	subeq	sp, sp, #4	)
-	stmia	sp, {r1 - r12}
-
-	ldmia	r0, {r3 - r5}
-	add	r7, sp, #S_SP - 4	@ here for interlock avoidance
-	mov	r6, #-1			@  ""  ""      ""       ""
-	add	r2, sp, #(S_FRAME_SIZE + \stack_hole - 4)
- SPFIX(	addeq	r2, r2, #4	)
-	str	r3, [sp, #-4]!		@ save the "real" r0 copied
-					@ from the exception stack
-
-	mov	r3, lr
-
-	@
-	@ We are now ready to fill in the remaining blanks on the stack:
-	@
-	@  r2 - sp_svc
-	@  r3 - lr_svc
-	@  r4 - lr_<exception>, already fixed up for correct return/restart
-	@  r5 - spsr_<exception>
-	@  r6 - orig_r0 (see pt_regs definition in ptrace.h)
-	@
-	stmia	r7, {r2 - r6}
-
-#ifdef CONFIG_TRACE_IRQFLAGS
-	bl	trace_hardirqs_off
-#endif
-	.endm
-
 	.align	5
 __dabt_svc:
 	svc_entry
@@ -348,71 +297,8 @@ ENDPROC(__pabt_svc)
 
 /*
  * User mode handlers
- *
- * EABI note: sp_svc is always 64-bit aligned here, so should S_FRAME_SIZE
  */
 
-#if defined(CONFIG_AEABI) && (__LINUX_ARM_ARCH__ >= 5) && (S_FRAME_SIZE & 7)
-#error "sizeof(struct pt_regs) must be a multiple of 8"
-#endif
-
-	.macro	usr_entry
- UNWIND(.fnstart	)
- UNWIND(.cantunwind	)	@ don't unwind the user space
-	sub	sp, sp, #S_FRAME_SIZE
- ARM(	stmib	sp, {r1 - r12}	)
- THUMB(	stmia	sp, {r0 - r12}	)
-
-	ldmia	r0, {r3 - r5}
-	add	r0, sp, #S_PC		@ here for interlock avoidance
-	mov	r6, #-1			@  ""  ""     ""        ""
-
-	str	r3, [sp]		@ save the "real" r0 copied
-					@ from the exception stack
-
-	@
-	@ We are now ready to fill in the remaining blanks on the stack:
-	@
-	@  r4 - lr_<exception>, already fixed up for correct return/restart
-	@  r5 - spsr_<exception>
-	@  r6 - orig_r0 (see pt_regs definition in ptrace.h)
-	@
-	@ Also, separately save sp_usr and lr_usr
-	@
-	stmia	r0, {r4 - r6}
- ARM(	stmdb	r0, {sp, lr}^			)
- THUMB(	store_user_sp_lr r0, r1, S_SP - S_PC	)
-
-	@
-	@ Enable the alignment trap while in kernel mode
-	@
-	alignment_trap r0
-
-	@
-	@ Clear FP to mark the first stack frame
-	@
-	zero_fp
-
-#ifdef CONFIG_IRQSOFF_TRACER
-	bl	trace_hardirqs_off
-#endif
-	.endm
-
-	.macro	kuser_cmpxchg_check
-#if !defined(CONFIG_CPU_32v6K) && !defined(CONFIG_NEEDS_SYSCALL_FOR_CMPXCHG)
-#ifndef CONFIG_MMU
-#warning "NPTL on non MMU needs fixing"
-#else
-	@ Make sure our user space atomic helper is restarted
-	@ if it was interrupted in a critical region.  Here we
-	@ perform a quick test inline since it should be false
-	@ 99.9999% of the time.  The rest is done out of line.
-	cmp	r4, #TASK_SIZE
-	blhs	kuser_cmpxchg64_fixup
-#endif
-#endif
-	.endm
-
 	.align	5
 __dabt_usr:
 	usr_entry
@@ -846,6 +732,7 @@ __kuser_cmpxchg64:				@ 0xffff0f60
 	ldmfd	sp!, {r4, r5, r6, pc}
 
 	.text
+	.global kuser_cmpxchg64_fixup
 kuser_cmpxchg64_fixup:
 	@ Called from kuser_cmpxchg_fixup.
 	@ r4 = address of interrupted insn (must be preserved).
@@ -976,58 +863,6 @@ __kuser_helper_end:
 
  THUMB(	.thumb	)
 
-/*
- * Vector stubs.
- *
- * This code is copied to 0xffff0200 so we can use branches in the
- * vectors, rather than ldr's.  Note that this code must not
- * exceed 0x300 bytes.
- *
- * Common stub entry macro:
- *   Enter in IRQ mode, spsr = SVC/USR CPSR, lr = SVC/USR PC
- *
- * SP points to a minimal amount of processor-private memory, the address
- * of which is copied into r0 for the mode specific abort handler.
- */
-	.macro	vector_stub, name, mode, correction=0
-	.align	5
-
-vector_\name:
-	.if \correction
-	sub	lr, lr, #\correction
-	.endif
-
-	@
-	@ Save r0, lr_<exception> (parent PC) and spsr_<exception>
-	@ (parent CPSR)
-	@
-	stmia	sp, {r0, lr}		@ save r0, lr
-	mrs	lr, spsr
-	str	lr, [sp, #8]		@ save spsr
-
-	@
-	@ Prepare for SVC32 mode.  IRQs remain disabled.
-	@
-	mrs	r0, cpsr
-	eor	r0, r0, #(\mode ^ SVC_MODE | PSR_ISETSTATE)
-	msr	spsr_cxsf, r0
-
-	@
-	@ the branch table must immediately follow this code
-	@
-	and	lr, lr, #0x0f
- THUMB(	adr	r0, 1f			)
- THUMB(	ldr	lr, [r0, lr, lsl #2]	)
-	mov	r0, sp
- ARM(	ldr	lr, [pc, lr, lsl #2]	)
-	movs	pc, lr			@ branch to handler in SVC mode
-ENDPROC(vector_\name)
-
-	.align	2
-	@ handler addresses follow this label
-1:
-	.endm
-
 	.globl	__stubs_start
 __stubs_start:
 /*
diff --git a/arch/arm/kernel/entry-header.S b/arch/arm/kernel/entry-header.S
index 9a8531e..c3c09ac 100644
--- a/arch/arm/kernel/entry-header.S
+++ b/arch/arm/kernel/entry-header.S
@@ -73,6 +73,109 @@
 	msr	cpsr_c, \rtemp			@ switch back to the SVC mode
 	.endm
 
+/*
+ * Vector stubs.
+ *
+ * This code is copied to 0xffff0200 so we can use branches in the
+ * vectors, rather than ldr's.  Note that this code must not
+ * exceed 0x300 bytes.
+ *
+ * Common stub entry macro:
+ *   Enter in IRQ mode, spsr = SVC/USR CPSR, lr = SVC/USR PC
+ *
+ * SP points to a minimal amount of processor-private memory, the address
+ * of which is copied into r0 for the mode specific abort handler.
+ */
+	.macro	vector_stub, name, mode, correction=0
+	.align	5
+
+vector_\name:
+	.if \correction
+	sub	lr, lr, #\correction
+	.endif
+
+	@
+	@ Save r0, lr_<exception> (parent PC) and spsr_<exception>
+	@ (parent CPSR)
+	@
+	stmia	sp, {r0, lr}		@ save r0, lr
+	mrs	lr, spsr
+	str	lr, [sp, #8]		@ save spsr
+
+	@
+	@ Prepare for SVC32 mode.  IRQs remain disabled.
+	@
+	mrs	r0, cpsr
+	eor	r0, r0, #(\mode ^ SVC_MODE | PSR_ISETSTATE)
+	msr	spsr_cxsf, r0
+
+	@
+	@ the branch table must immediately follow this code
+	@
+	and	lr, lr, #0x0f
+ THUMB(	adr	r0, 1f			)
+ THUMB(	ldr	lr, [r0, lr, lsl #2]	)
+	mov	r0, sp
+ ARM(	ldr	lr, [pc, lr, lsl #2]	)
+	movs	pc, lr			@ branch to handler in SVC mode
+ENDPROC(vector_\name)
+
+	.align	2
+	@ handler addresses follow this label
+1:
+	.endm
+
+/*
+ * SVC mode handlers
+ */
+
+#if defined(CONFIG_AEABI) && (__LINUX_ARM_ARCH__ >= 5)
+#define SPFIX(code...) code
+#else
+#define SPFIX(code...)
+#endif
+
+	.macro	svc_entry, stack_hole=0
+ UNWIND(.fnstart		)
+ UNWIND(.save {r0 - pc}		)
+	sub	sp, sp, #(S_FRAME_SIZE + \stack_hole - 4)
+#ifdef CONFIG_THUMB2_KERNEL
+ SPFIX(	str	r0, [sp]	)	@ temporarily saved
+ SPFIX(	mov	r0, sp		)
+ SPFIX(	tst	r0, #4		)	@ test original stack alignment
+ SPFIX(	ldr	r0, [sp]	)	@ restored
+#else
+ SPFIX(	tst	sp, #4		)
+#endif
+ SPFIX(	subeq	sp, sp, #4	)
+	stmia	sp, {r1 - r12}
+
+	ldmia	r0, {r3 - r5}
+	add	r7, sp, #S_SP - 4	@ here for interlock avoidance
+	mov	r6, #-1			@  ""  ""      ""       ""
+	add	r2, sp, #(S_FRAME_SIZE + \stack_hole - 4)
+ SPFIX(	addeq	r2, r2, #4	)
+	str	r3, [sp, #-4]!		@ save the "real" r0 copied
+					@ from the exception stack
+
+	mov	r3, lr
+
+	@
+	@ We are now ready to fill in the remaining blanks on the stack:
+	@
+	@  r2 - sp_svc
+	@  r3 - lr_svc
+	@  r4 - lr_<exception>, already fixed up for correct return/restart
+	@  r5 - spsr_<exception>
+	@  r6 - orig_r0 (see pt_regs definition in ptrace.h)
+	@
+	stmia	r7, {r2 - r6}
+
+#ifdef CONFIG_TRACE_IRQFLAGS
+	bl	trace_hardirqs_off
+#endif
+	.endm
+
 #ifndef CONFIG_THUMB2_KERNEL
 	.macro	svc_exit, rpsr
 	msr	spsr_cxsf, \rpsr
@@ -164,6 +267,73 @@
 #endif	/* !CONFIG_THUMB2_KERNEL */
 
 /*
+ * User mode handlers
+ *
+ * EABI note: sp_svc is always 64-bit aligned here, so should S_FRAME_SIZE
+ */
+
+#if defined(CONFIG_AEABI) && (__LINUX_ARM_ARCH__ >= 5) && (S_FRAME_SIZE & 7)
+#error "sizeof(struct pt_regs) must be a multiple of 8"
+#endif
+
+	.macro	usr_entry
+ UNWIND(.fnstart	)
+ UNWIND(.cantunwind	)	@ don't unwind the user space
+	sub	sp, sp, #S_FRAME_SIZE
+ ARM(	stmib	sp, {r1 - r12}	)
+ THUMB(	stmia	sp, {r0 - r12}	)
+
+	ldmia	r0, {r3 - r5}
+	add	r0, sp, #S_PC		@ here for interlock avoidance
+	mov	r6, #-1			@  ""  ""     ""        ""
+
+	str	r3, [sp]		@ save the "real" r0 copied
+					@ from the exception stack
+
+	@
+	@ We are now ready to fill in the remaining blanks on the stack:
+	@
+	@  r4 - lr_<exception>, already fixed up for correct return/restart
+	@  r5 - spsr_<exception>
+	@  r6 - orig_r0 (see pt_regs definition in ptrace.h)
+	@
+	@ Also, separately save sp_usr and lr_usr
+	@
+	stmia	r0, {r4 - r6}
+ ARM(	stmdb	r0, {sp, lr}^			)
+ THUMB(	store_user_sp_lr r0, r1, S_SP - S_PC	)
+
+	@
+	@ Enable the alignment trap while in kernel mode
+	@
+	alignment_trap r0
+
+	@
+	@ Clear FP to mark the first stack frame
+	@
+	zero_fp
+
+#ifdef CONFIG_IRQSOFF_TRACER
+	bl	trace_hardirqs_off
+#endif
+	.endm
+
+	.macro	kuser_cmpxchg_check
+#if !defined(CONFIG_CPU_32v6K) && !defined(CONFIG_NEEDS_SYSCALL_FOR_CMPXCHG)
+#ifndef CONFIG_MMU
+#warning "NPTL on non MMU needs fixing"
+#else
+	@ Make sure our user space atomic helper is restarted
+	@ if it was interrupted in a critical region.  Here we
+	@ perform a quick test inline since it should be false
+	@ 99.9999% of the time.  The rest is done out of line.
+	cmp	r4, #TASK_SIZE
+	blhs	kuser_cmpxchg64_fixup
+#endif
+#endif
+	.endm
+
+/*
  * These are the registers used in the syscall handler, and allow us to
  * have in theory up to 7 arguments to a function - r0 to r6.
  *
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 09/11] ARM: Add KGDB/KDB FIQ debugger generic code
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
                   ` (7 preceding siblings ...)
  2012-09-13 15:03 ` [PATCH 08/11] ARM: Move some macros from entry-armv to entry-header Anton Vorontsov
@ 2012-09-13 15:03 ` Anton Vorontsov
  2012-09-13 15:03 ` [PATCH 10/11] ARM: VIC: Add a couple of low-level FIQ management helpers Anton Vorontsov
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 15:03 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

The FIQ debugger may be used to debug situations when the kernel stuck
in uninterruptable sections, e.g. the kernel infinitely loops or
deadlocked in an interrupt or with interrupts disabled.

By default KGDB FIQ is disabled in runtime, but can be enabled with
kgdb_fiq.enable=1 kernel command line option.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 arch/arm/Kconfig                 | 18 ++++++++
 arch/arm/include/asm/kgdb.h      |  8 ++++
 arch/arm/kernel/Makefile         |  1 +
 arch/arm/kernel/kgdb_fiq.c       | 99 ++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/kgdb_fiq_entry.S | 87 +++++++++++++++++++++++++++++++++++
 5 files changed, 213 insertions(+)
 create mode 100644 arch/arm/kernel/kgdb_fiq.c
 create mode 100644 arch/arm/kernel/kgdb_fiq_entry.S

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 6d6e18f..c978c74 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -171,6 +171,24 @@ config GENERIC_ISA_DMA
 config FIQ
 	bool
 
+config ARCH_MIGHT_HAVE_KGDB_FIQ
+	bool
+
+config KGDB_FIQ
+	bool "KGDB/KDB FIQ debugger"
+	depends on KGDB_KDB && ARCH_MIGHT_HAVE_KGDB_FIQ && !THUMB2_KERNEL
+	select FIQ
+	help
+	  The FIQ debugger may be used to debug situations when the
+	  kernel stuck in uninterruptable sections, e.g. the kernel
+	  infinitely loops or deadlocked in an interrupt or with
+	  interrupts disabled.
+
+	  By default KGDB FIQ is disabled in runtime, but can be
+	  enabled with kgdb_fiq.enable=1 kernel command line option.
+
+	  If unsure, say N.
+
 config NEED_RET_TO_USER
 	bool
 
diff --git a/arch/arm/include/asm/kgdb.h b/arch/arm/include/asm/kgdb.h
index 48066ce..807e547 100644
--- a/arch/arm/include/asm/kgdb.h
+++ b/arch/arm/include/asm/kgdb.h
@@ -11,6 +11,8 @@
 #define __ARM_KGDB_H__
 
 #include <linux/ptrace.h>
+#include <linux/linkage.h>
+#include <asm/exception.h>
 
 /*
  * GDB assumes that we're a user process being debugged, so
@@ -47,6 +49,12 @@ static inline void arch_kgdb_breakpoint(void)
 extern void kgdb_handle_bus_error(void);
 extern int kgdb_fault_expected;
 
+extern char kgdb_fiq_handler;
+extern char kgdb_fiq_handler_end;
+asmlinkage void __exception_irq_entry kgdb_fiq_do_handle(struct pt_regs *regs);
+extern int __init kgdb_register_fiq(unsigned int mach_kgdb_fiq,
+		     void (*mach_kgdb_enable_fiq)(unsigned int irq, bool on),
+		     bool (*mach_is_kgdb_fiq)(unsigned int irq));
 #endif /* !__ASSEMBLY__ */
 
 /*
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 7ad2d5c..5aa079b 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_ATAGS_PROC)	+= atags.o
 obj-$(CONFIG_OABI_COMPAT)	+= sys_oabi-compat.o
 obj-$(CONFIG_ARM_THUMBEE)	+= thumbee.o
 obj-$(CONFIG_KGDB)		+= kgdb.o
+obj-$(CONFIG_KGDB_FIQ)		+= kgdb_fiq_entry.o kgdb_fiq.o
 obj-$(CONFIG_ARM_UNWIND)	+= unwind.o
 obj-$(CONFIG_HAVE_TCM)		+= tcm.o
 obj-$(CONFIG_OF)		+= devtree.o
diff --git a/arch/arm/kernel/kgdb_fiq.c b/arch/arm/kernel/kgdb_fiq.c
new file mode 100644
index 0000000..8443af1
--- /dev/null
+++ b/arch/arm/kernel/kgdb_fiq.c
@@ -0,0 +1,99 @@
+/*
+ * KGDB FIQ
+ *
+ * Copyright 2010 Google, Inc.
+ *		  Arve Hjønnevåg <arve@android.com>
+ *		  Colin Cross <ccross@android.com>
+ * Copyright 2012 Linaro Ltd.
+ *		  Anton Vorontsov <anton.vorontsov@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/hardirq.h>
+#include <linux/kdb.h>
+#include <linux/kgdb.h>
+#include <asm/fiq.h>
+#include <asm/exception.h>
+
+static int kgdb_fiq_enabled;
+module_param_named(enable, kgdb_fiq_enabled, int, 0600);
+MODULE_PARM_DESC(enable, "set to 1 to enable FIQ KGDB");
+
+static unsigned int kgdb_fiq;
+static bool (*is_kgdb_fiq)(unsigned int irq);
+
+asmlinkage void __exception_irq_entry kgdb_fiq_do_handle(struct pt_regs *regs)
+{
+	if (!is_kgdb_fiq(kgdb_fiq))
+		return;
+	if (!kgdb_nmi_poll_knock())
+		return;
+
+	nmi_enter();
+	kgdb_handle_exception(1, 0, 0, regs);
+	nmi_exit();
+}
+
+static struct fiq_handler kgdb_fiq_desc = {
+	.name = "kgdb",
+};
+
+static long kgdb_fiq_setup_stack(void *info)
+{
+	struct pt_regs regs;
+
+	regs.ARM_sp = __get_free_pages(GFP_KERNEL, THREAD_SIZE_ORDER) +
+			THREAD_START_SP;
+	WARN_ON(!regs.ARM_sp);
+
+	set_fiq_regs(&regs);
+	return 0;
+}
+
+static void (*kgdb_enable_fiq)(unsigned int irq, bool on);
+
+void kgdb_arch_enable_nmi(bool on)
+{
+	if (!kgdb_enable_fiq)
+		return;
+	kgdb_enable_fiq(kgdb_fiq, on);
+}
+
+int __init kgdb_register_fiq(unsigned int mach_kgdb_fiq,
+		void (*mach_kgdb_enable_fiq)(unsigned int irq, bool on),
+		bool (*mach_is_kgdb_fiq)(unsigned int irq))
+{
+	int err;
+	int cpu;
+
+	if (!kgdb_fiq_enabled)
+		return -ENODEV;
+	if (kgdb_fiq)
+		return -EBUSY;
+
+	kgdb_fiq = mach_kgdb_fiq;
+	kgdb_enable_fiq = mach_kgdb_enable_fiq;
+	is_kgdb_fiq = mach_is_kgdb_fiq;
+
+	err = claim_fiq(&kgdb_fiq_desc);
+	if (err) {
+		pr_warn("%s: unable to claim fiq", __func__);
+		return err;
+	}
+
+	for_each_possible_cpu(cpu)
+		work_on_cpu(cpu, kgdb_fiq_setup_stack, NULL);
+
+	set_fiq_handler(&kgdb_fiq_handler,
+			&kgdb_fiq_handler_end - &kgdb_fiq_handler);
+
+	return 0;
+}
diff --git a/arch/arm/kernel/kgdb_fiq_entry.S b/arch/arm/kernel/kgdb_fiq_entry.S
new file mode 100644
index 0000000..d6becca
--- /dev/null
+++ b/arch/arm/kernel/kgdb_fiq_entry.S
@@ -0,0 +1,87 @@
+/*
+ * KGDB FIQ entry
+ *
+ * Copyright 1996,1997,1998 Russell King.
+ * Copyright 2012 Linaro Ltd.
+ *		  Anton Vorontsov <anton.vorontsov@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+#include <asm/memory.h>
+#include <asm/unwind.h>
+#include "entry-header.S"
+
+	.text
+
+@ This is needed for usr_entry/alignment_trap
+.LCcralign:
+	.long	cr_alignment
+.LCdohandle:
+	.long	kgdb_fiq_do_handle
+
+	.macro	fiq_handler
+	ldr	r1, =.LCdohandle
+	mov	r0, sp
+	adr	lr, BSYM(9997f)
+	ldr	pc, [r1]
+9997:
+	.endm
+
+	.align	5
+__fiq_svc:
+	svc_entry
+	fiq_handler
+	mov	r0, sp
+	ldmib	r0, {r1 - r14}
+	msr	cpsr_c, #FIQ_MODE | PSR_I_BIT | PSR_F_BIT
+	add	r8, r0, #S_PC
+	ldr	r9, [r0, #S_PSR]
+	msr	spsr_cxsf, r9
+	ldr	r0, [r0, #S_R0]
+	ldmia	r8, {pc}^
+
+ UNWIND(.fnend		)
+ENDPROC(__fiq_svc)
+	.ltorg
+
+	.align	5
+__fiq_usr:
+	usr_entry
+	kuser_cmpxchg_check
+	fiq_handler
+	get_thread_info tsk
+	mov	why, #0
+	b	ret_to_user_from_irq
+ UNWIND(.fnend		)
+ENDPROC(__fiq_usr)
+	.ltorg
+
+	.global kgdb_fiq_handler
+kgdb_fiq_handler:
+
+	vector_stub	fiq, FIQ_MODE, 4
+
+	.long	__fiq_usr			@  0  (USR_26 / USR_32)
+	.long	__fiq_svc			@  1  (FIQ_26 / FIQ_32)
+	.long	__fiq_svc			@  2  (IRQ_26 / IRQ_32)
+	.long	__fiq_svc			@  3  (SVC_26 / SVC_32)
+	.long	__fiq_svc			@  4
+	.long	__fiq_svc			@  5
+	.long	__fiq_svc			@  6
+	.long	__fiq_svc			@  7
+	.long	__fiq_svc			@  8
+	.long	__fiq_svc			@  9
+	.long	__fiq_svc			@  a
+	.long	__fiq_svc			@  b
+	.long	__fiq_svc			@  c
+	.long	__fiq_svc			@  d
+	.long	__fiq_svc			@  e
+	.long	__fiq_svc			@  f
+
+	.global kgdb_fiq_handler_end
+kgdb_fiq_handler_end:
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 10/11] ARM: VIC: Add a couple of low-level FIQ management helpers
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
                   ` (8 preceding siblings ...)
  2012-09-13 15:03 ` [PATCH 09/11] ARM: Add KGDB/KDB FIQ debugger generic code Anton Vorontsov
@ 2012-09-13 15:03 ` Anton Vorontsov
  2012-09-13 15:03 ` [PATCH 11/11] ARM: versatile: Make able to use UART ports for KGDB FIQ debugger Anton Vorontsov
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 15:03 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

Just a couple of calls to manage VIC FIQ routing. We'll use them for
KGDB FIQ support on ARM Versatile machines.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 arch/arm/common/vic.c               | 28 ++++++++++++++++++++++++++++
 arch/arm/include/asm/hardware/vic.h |  2 ++
 2 files changed, 30 insertions(+)

diff --git a/arch/arm/common/vic.c b/arch/arm/common/vic.c
index e0d5388..df2fc82 100644
--- a/arch/arm/common/vic.c
+++ b/arch/arm/common/vic.c
@@ -66,6 +66,34 @@ static struct vic_device vic_devices[CONFIG_ARM_VIC_NR];
 
 static int vic_id;
 
+static void __iomem *vic_base(struct irq_data *d)
+{
+	return (void __iomem *)irq_data_get_irq_chip_data(d);
+}
+
+void vic_fiq_select(unsigned int irq, bool on)
+{
+	void __iomem *base = vic_base(&irq_to_desc(irq)->irq_data);
+	void __iomem *sel = base + VIC_INT_SELECT;
+	u32 msk = 1 << irq;
+	u32 val;
+
+	pr_debug("rerouting VIC vector %d to %s\n", irq, on ? "FIQ" : "IRQ");
+
+	val = readl(sel);
+	val &= ~msk;
+	if (on)
+		val |= msk;
+	writel(val, sel);
+}
+
+bool vic_is_fiq_rised(unsigned int irq)
+{
+	void __iomem *base = vic_base(&irq_to_desc(irq)->irq_data);
+
+	return readl(base + VIC_FIQ_STATUS) & (1 << irq);
+}
+
 /**
  * vic_init2 - common initialisation code
  * @base: Base of the VIC.
diff --git a/arch/arm/include/asm/hardware/vic.h b/arch/arm/include/asm/hardware/vic.h
index e14af1a..2728975 100644
--- a/arch/arm/include/asm/hardware/vic.h
+++ b/arch/arm/include/asm/hardware/vic.h
@@ -52,6 +52,8 @@ void __vic_init(void __iomem *base, unsigned int irq_start, u32 vic_sources,
 void vic_init(void __iomem *base, unsigned int irq_start, u32 vic_sources, u32 resume_sources);
 int vic_of_init(struct device_node *node, struct device_node *parent);
 void vic_handle_irq(struct pt_regs *regs);
+void vic_fiq_select(unsigned int irq, bool on);
+bool vic_is_fiq_rised(unsigned int irq);
 
 #endif /* __ASSEMBLY__ */
 #endif
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 11/11] ARM: versatile: Make able to use UART ports for KGDB FIQ debugger
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
                   ` (9 preceding siblings ...)
  2012-09-13 15:03 ` [PATCH 10/11] ARM: VIC: Add a couple of low-level FIQ management helpers Anton Vorontsov
@ 2012-09-13 15:03 ` Anton Vorontsov
  2012-09-13 17:04 ` [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Alan Cox
  2012-09-17 11:37 ` Greg Kroah-Hartman
  12 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 15:03 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

If enabled, kernel will able to enter KGDB upon serial line activity on
UART ports.

Note that even with this patch and CONFIG_KGDB_FIQ is enabled, you still
need to pass kgdb_fiq.enable=1 kernel command line option, otherwise UART
will behave in a normal way.

By default UART0 is used, but this can be changed via kgdb_fiq.uart_num
kernel command line option.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 arch/arm/Kconfig                   |  1 +
 arch/arm/mach-versatile/Makefile   |  1 +
 arch/arm/mach-versatile/kgdb_fiq.c | 31 +++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+)
 create mode 100644 arch/arm/mach-versatile/kgdb_fiq.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c978c74..1a9881a 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -336,6 +336,7 @@ config ARCH_VERSATILE
 	select PLAT_VERSATILE_CLCD
 	select PLAT_VERSATILE_FPGA_IRQ
 	select ARM_TIMER_SP804
+	select ARCH_MIGHT_HAVE_KGDB_FIQ
 	help
 	  This enables support for ARM Ltd Versatile board.
 
diff --git a/arch/arm/mach-versatile/Makefile b/arch/arm/mach-versatile/Makefile
index 81fa3fe..bfd761f 100644
--- a/arch/arm/mach-versatile/Makefile
+++ b/arch/arm/mach-versatile/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_ARCH_VERSATILE_PB)		+= versatile_pb.o
 obj-$(CONFIG_MACH_VERSATILE_AB)		+= versatile_ab.o
 obj-$(CONFIG_MACH_VERSATILE_DT)		+= versatile_dt.o
 obj-$(CONFIG_PCI)			+= pci.o
+obj-$(CONFIG_KGDB_FIQ)			+= kgdb_fiq.o
diff --git a/arch/arm/mach-versatile/kgdb_fiq.c b/arch/arm/mach-versatile/kgdb_fiq.c
new file mode 100644
index 0000000..3cdf71d
--- /dev/null
+++ b/arch/arm/mach-versatile/kgdb_fiq.c
@@ -0,0 +1,31 @@
+/*
+ * KGDB FIQ board support
+ *
+ * Copyright 2012 Linaro Ltd.
+ *		  Anton Vorontsov <anton.vorontsov@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kgdb.h>
+#include <mach/hardware.h>
+#include <mach/platform.h>
+#include <asm/hardware/vic.h>
+
+static int kgdb_fiq;
+module_param_named(uart_num, kgdb_fiq, int, 0600);
+MODULE_PARM_DESC(uart_num, "UART<number> port to use for KGDB FIQ");
+
+static int __init kgdb_fiq_init(void)
+{
+	WARN_ON(kgdb_fiq > INT_UARTINT2 - INT_UARTINT0);
+
+	return kgdb_register_fiq(INT_UARTINT0 + kgdb_fiq,
+				 vic_fiq_select,
+				 vic_is_fiq_rised);
+}
+console_initcall(kgdb_fiq_init);
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger
  2012-09-13 17:04 ` [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Alan Cox
@ 2012-09-13 16:55   ` Anton Vorontsov
  0 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-13 16:55 UTC (permalink / raw)
  To: Alan Cox
  Cc: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On Thu, Sep 13, 2012 at 06:04:57PM +0100, Alan Cox wrote:
> On Thu, 13 Sep 2012 08:01:33 -0700
> Anton Vorontsov <anton.vorontsov@linaro.org> wrote:
> 
> > Hi all,
> > 
> > Here comes the lucky v7:
> > 
> > - Per Alan Cox's suggestion added hangup method and removed a small
> >   leftover;
> > - Per Colin Cross' suggestion moved IRQ quiescing logic into
> >   poll_get_char routine. IIUC, Alan is less unhappy about it.  As a
> >   result, clear_irq() callback dropped.
> 
> Ok that has my ack. Some of it is not pretty but debugger hooks that
> have to run behind the OS while debugging it never are.
> 
> Acked-by: Alan Cox <alan@linux.intel.com>

Great! Much thanks for reviews, Alan.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
                   ` (10 preceding siblings ...)
  2012-09-13 15:03 ` [PATCH 11/11] ARM: versatile: Make able to use UART ports for KGDB FIQ debugger Anton Vorontsov
@ 2012-09-13 17:04 ` Alan Cox
  2012-09-13 16:55   ` Anton Vorontsov
  2012-09-17 11:37 ` Greg Kroah-Hartman
  12 siblings, 1 reply; 29+ messages in thread
From: Alan Cox @ 2012-09-13 17:04 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On Thu, 13 Sep 2012 08:01:33 -0700
Anton Vorontsov <anton.vorontsov@linaro.org> wrote:

> Hi all,
> 
> Here comes the lucky v7:
> 
> - Per Alan Cox's suggestion added hangup method and removed a small
>   leftover;
> - Per Colin Cross' suggestion moved IRQ quiescing logic into
>   poll_get_char routine. IIUC, Alan is less unhappy about it.  As a
>   result, clear_irq() callback dropped.

Ok that has my ack. Some of it is not pretty but debugger hooks that
have to run behind the OS while debugging it never are.

Acked-by: Alan Cox <alan@linux.intel.com>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 07/11] tty/serial: Add kgdb_nmi driver
  2012-09-13 15:03 ` [PATCH 07/11] tty/serial: Add kgdb_nmi driver Anton Vorontsov
@ 2012-09-15 15:52   ` Francesco Lavra
  2012-09-17  1:07     ` [PATCH updated " Anton Vorontsov
  2012-09-19 11:52   ` [PATCH " Jason Wessel
  1 sibling, 1 reply; 29+ messages in thread
From: Francesco Lavra @ 2012-09-15 15:52 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman,
	Alan Cox, linaro-kernel, patches, Brian Swetland, linux-kernel,
	Arve Hjønnevåg, linux-serial, Colin Cross,
	kgdb-bugreport, kernel-team, linux-arm-kernel

On 09/13/2012 05:03 PM, Anton Vorontsov wrote:
...
> +static int kgdb_nmi_tty_install(struct tty_driver *drv, struct tty_struct *tty)
> +{
> +	struct kgdb_nmi_tty_priv *priv;
> +	int ret;
> +
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	INIT_KFIFO(priv->fifo);
> +	tasklet_init(&priv->tlet, kgdb_nmi_tty_receiver, (unsigned long)priv);
> +	tty_port_init(&priv->port);
> +	priv->port.ops = &kgdb_nmi_tty_port_ops;
> +	tty->driver_data = priv;
> +
> +	ret = tty_port_install(&priv->port, drv, tty);
> +	if (ret) {
> +		pr_err("%s: can't nstall tty port: %d\n", __func__, ret);

s/nstall/install

...
> diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> index 2b42a01..ed97cfd 100644
> --- a/drivers/tty/serial/kgdboc.c
> +++ b/drivers/tty/serial/kgdboc.c
> @@ -145,6 +145,8 @@ __setup("kgdboc=", kgdboc_option_setup);
>  
>  static void cleanup_kgdboc(void)
>  {
> +	if (kgdb_unregister_nmi_console())
> +		return;
>  	kgdboc_unregister_kbd();
>  	if (configured == 1)
>  		kgdb_unregister_io_module(&kgdboc_io_ops);
> @@ -198,6 +200,10 @@ do_register:
>  	if (err)
>  		goto noconfig;
>  
> +	err = kgdb_register_nmi_console();
> +	if (err)
> +		goto noconfig;

If kgdb_register_nmi_console() fails, kgdb_unregister_io_module() must
be called for proper cleanup

--
Francesco

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH updated 07/11] tty/serial: Add kgdb_nmi driver
  2012-09-15 15:52   ` Francesco Lavra
@ 2012-09-17  1:07     ` Anton Vorontsov
  0 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-17  1:07 UTC (permalink / raw)
  To: Francesco Lavra
  Cc: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman,
	Alan Cox, linaro-kernel, patches, Brian Swetland, linux-kernel,
	Arve Hjønnevåg, linux-serial, Colin Cross,
	kgdb-bugreport, kernel-team, linux-arm-kernel

This special driver makes it possible to temporary use NMI debugger port
as a normal console by issuing 'nmi_console' command (assuming that the
port is attached to KGDB).

Unlike KDB's disable_nmi command, with this driver you are always able
to go back to the debugger using KGDB escape sequence ($3#33).  This is
because this console driver processes the input in NMI context, and thus
is able to intercept the magic sequence.

Note that since the console interprets input and uses polling
communication methods, for things like PPP it is still better to fully
detach debugger port from the KGDB NMI (i.e. disable_nmi), and use raw
console.

Usually, to enter the debugger one have to type the magic sequence, so
initially the kernel will print the following prompt on the NMI debugger
console:

	Type $3#33 to enter the debugger>

For convenience, there is a kgdb_fiq.knock kernel command line option,
when set to 0, this turns the special command to just a return key
press, so the kernel will be printing this:

	Hit <return> to enter the debugger>

This is more convenient for long debugging sessions, although it makes
nmi_console feature somewhat useless.

And for the cases when NMI connected to a dedicated button, the knocking
can be disabled altogether by setting kgdb_fiq.knock to -1.

Suggested-by: Colin Cross <ccross@android.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
Acked-by: Alan Cox <alan@linux.intel.com>
---

On Sat, Sep 15, 2012 at 05:52:54PM +0200, Francesco Lavra wrote:
[...]
> > +		pr_err("%s: can't nstall tty port: %d\n", __func__, ret);
> 
> s/nstall/install
[..]
> > +	err = kgdb_register_nmi_console();
> > +	if (err)
> > +		goto noconfig;
> 
> If kgdb_register_nmi_console() fails, kgdb_unregister_io_module() must
> be called for proper cleanup

Thanks for catching! Both issues fixed now, the updated patch down blow.

 drivers/tty/serial/Kconfig    |  19 ++
 drivers/tty/serial/Makefile   |   1 +
 drivers/tty/serial/kgdb_nmi.c | 396 ++++++++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/kgdboc.c   |   9 +
 include/linux/kgdb.h          |  10 ++
 5 files changed, 435 insertions(+)
 create mode 100644 drivers/tty/serial/kgdb_nmi.c

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 26907cf..b22e45b 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -141,6 +141,25 @@ config SERIAL_ATMEL_TTYAT
 
 	  Say Y if you have an external 8250/16C550 UART.  If unsure, say N.
 
+config SERIAL_KGDB_NMI
+	bool "Serial console over KGDB NMI debugger port"
+	depends on KGDB_SERIAL_CONSOLE
+	help
+	  This special driver allows you to temporary use NMI debugger port
+	  as a normal console (assuming that the port is attached to KGDB).
+
+	  Unlike KDB's disable_nmi command, with this driver you are always
+	  able to go back to the debugger using KGDB escape sequence ($3#33).
+	  This is because this console driver processes the input in NMI
+	  context, and thus is able to intercept the magic sequence.
+
+	  Note that since the console interprets input and uses polling
+	  communication methods, for things like PPP you still must fully
+	  detach debugger port from the KGDB NMI (i.e. disable_nmi), and
+	  use raw console.
+
+	  If unsure, say N.
+
 config SERIAL_KS8695
 	bool "Micrel KS8695 (Centaur) serial port support"
 	depends on ARCH_KS8695
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index ce88667..4f694da 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_SERIAL_MSM_HS) += msm_serial_hs.o
 obj-$(CONFIG_SERIAL_NETX) += netx-serial.o
 obj-$(CONFIG_SERIAL_OF_PLATFORM) += of_serial.o
 obj-$(CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL) += nwpserial.o
+obj-$(CONFIG_SERIAL_KGDB_NMI) += kgdb_nmi.o
 obj-$(CONFIG_SERIAL_KS8695) += serial_ks8695.o
 obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
 obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o
diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c
new file mode 100644
index 0000000..ccc7437
--- /dev/null
+++ b/drivers/tty/serial/kgdb_nmi.c
@@ -0,0 +1,396 @@
+/*
+ * KGDB NMI serial console
+ *
+ * Copyright 2010 Google, Inc.
+ *		  Arve Hjønnevåg <arve@android.com>
+ *		  Colin Cross <ccross@android.com>
+ * Copyright 2012 Linaro Ltd.
+ *		  Anton Vorontsov <anton.vorontsov@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/compiler.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/atomic.h>
+#include <linux/console.h>
+#include <linux/tty.h>
+#include <linux/tty_driver.h>
+#include <linux/tty_flip.h>
+#include <linux/interrupt.h>
+#include <linux/hrtimer.h>
+#include <linux/tick.h>
+#include <linux/kfifo.h>
+#include <linux/kgdb.h>
+#include <linux/kdb.h>
+
+static int kgdb_nmi_knock = 1;
+module_param_named(knock, kgdb_nmi_knock, int, 0600);
+MODULE_PARM_DESC(knock, "if set to 1 (default), the special '$3#33' command "
+			"must be used to enter the debugger; when set to 0, "
+			"hitting return key is enough to enter the debugger; "
+			"when set to -1, the debugger is entered immediately "
+			"upon NMI");
+
+static char *kgdb_nmi_magic = "$3#33";
+module_param_named(magic, kgdb_nmi_magic, charp, 0600);
+MODULE_PARM_DESC(magic, "magic sequence to enter NMI debugger (default $3#33)");
+
+static bool kgdb_nmi_tty_enabled;
+
+static void kgdb_nmi_console_write(struct console *co, const char *s, uint c)
+{
+	int i;
+
+	if (!kgdb_nmi_tty_enabled || atomic_read(&kgdb_active) >= 0)
+		return;
+
+	for (i = 0; i < c; i++)
+		dbg_io_ops->write_char(s[i]);
+}
+
+static struct tty_driver *kgdb_nmi_tty_driver;
+
+static struct tty_driver *kgdb_nmi_console_device(struct console *co, int *idx)
+{
+	*idx = co->index;
+	return kgdb_nmi_tty_driver;
+}
+
+static struct console kgdb_nmi_console = {
+	.name	= "ttyNMI",
+	.write	= kgdb_nmi_console_write,
+	.device	= kgdb_nmi_console_device,
+	.flags	= CON_PRINTBUFFER | CON_ANYTIME | CON_ENABLED,
+	.index	= -1,
+};
+
+/*
+ * This is usually the maximum rate on debug ports. We make fifo large enough
+ * to make copy-pasting to the terminal usable.
+ */
+#define KGDB_NMI_BAUD		115200
+#define KGDB_NMI_FIFO_SIZE	roundup_pow_of_two(KGDB_NMI_BAUD / 8 / HZ)
+
+struct kgdb_nmi_tty_priv {
+	struct tty_port port;
+	struct tasklet_struct tlet;
+	STRUCT_KFIFO(char, KGDB_NMI_FIFO_SIZE) fifo;
+};
+
+static struct kgdb_nmi_tty_priv *kgdb_nmi_port_to_priv(struct tty_port *port)
+{
+	return container_of(port, struct kgdb_nmi_tty_priv, port);
+}
+
+/*
+ * Our debugging console is polled in a tasklet, so we'll check for input
+ * every tick. In HZ-less mode, we should program the next tick.  We have
+ * to use the lowlevel stuff as no locks should be grabbed.
+ */
+#ifdef CONFIG_HIGH_RES_TIMERS
+static void kgdb_tty_poke(void)
+{
+	tick_program_event(ktime_get(), 0);
+}
+#else
+static inline void kgdb_tty_poke(void) {}
+#endif
+
+static struct tty_port *kgdb_nmi_port;
+
+static void kgdb_tty_recv(int ch)
+{
+	struct kgdb_nmi_tty_priv *priv;
+	char c = ch;
+
+	if (!kgdb_nmi_port || ch < 0)
+		return;
+	/*
+	 * Can't use port->tty->driver_data as tty might be not there. Tasklet
+	 * will check for tty and will get the ref, but here we don't have to
+	 * do that, and actually, we can't: we're in NMI context, no locks are
+	 * possible.
+	 */
+	priv = kgdb_nmi_port_to_priv(kgdb_nmi_port);
+	kfifo_in(&priv->fifo, &c, 1);
+	kgdb_tty_poke();
+}
+
+static int kgdb_nmi_poll_one_knock(void)
+{
+	static int n;
+	int c = -1;
+	const char *magic = kgdb_nmi_magic;
+	size_t m = strlen(magic);
+	bool printch = 0;
+
+	c = dbg_io_ops->read_char();
+	if (c == NO_POLL_CHAR)
+		return c;
+
+	if (!kgdb_nmi_knock && (c == '\r' || c == '\n')) {
+		return 1;
+	} else if (c == magic[n]) {
+		n = (n + 1) % m;
+		if (!n)
+			return 1;
+		printch = 1;
+	} else {
+		n = 0;
+	}
+
+	if (kgdb_nmi_tty_enabled) {
+		kgdb_tty_recv(c);
+		return 0;
+	}
+
+	if (printch) {
+		kdb_printf("%c", c);
+		return 0;
+	}
+
+	kdb_printf("\r%s %s to enter the debugger> %*s",
+		   kgdb_nmi_knock ? "Type" : "Hit",
+		   kgdb_nmi_knock ? magic  : "<return>", m, "");
+	while (m--)
+		kdb_printf("\b");
+	return 0;
+}
+
+/**
+ * kgdb_nmi_poll_knock - Check if it is time to enter the debugger
+ *
+ * "Serial ports are often noisy, especially when muxed over another port (we
+ * often use serial over the headset connector). Noise on the async command
+ * line just causes characters that are ignored, on a command line that blocked
+ * execution noise would be catastrophic." -- Colin Cross
+ *
+ * So, this function implements KGDB/KDB knocking on the serial line: we won't
+ * enter the debugger until we receive a known magic phrase (which is actually
+ * "$3#33", known as "escape to KDB" command. There is also a relaxed variant
+ * of knocking, i.e. just pressing the return key is enough to enter the
+ * debugger. And if knocking is disabled, the function always returns 1.
+ */
+bool kgdb_nmi_poll_knock(void)
+{
+	if (kgdb_nmi_knock < 0)
+		return 1;
+
+	while (1) {
+		int ret;
+
+		ret = kgdb_nmi_poll_one_knock();
+		if (ret == NO_POLL_CHAR)
+			return 0;
+		else if (ret == 1)
+			break;
+	}
+	return 1;
+}
+
+/*
+ * The tasklet is cheap, it does not cause wakeups when reschedules itself,
+ * instead it waits for the next tick.
+ */
+static void kgdb_nmi_tty_receiver(unsigned long data)
+{
+	struct kgdb_nmi_tty_priv *priv = (void *)data;
+	struct tty_struct *tty;
+	char ch;
+
+	tasklet_schedule(&priv->tlet);
+
+	if (likely(!kgdb_nmi_tty_enabled || !kfifo_len(&priv->fifo)))
+		return;
+
+	/* Port is there, but tty might be hung up, check. */
+	tty = tty_port_tty_get(kgdb_nmi_port);
+	if (!tty)
+		return;
+
+	while (kfifo_out(&priv->fifo, &ch, 1))
+		tty_insert_flip_char(priv->port.tty, ch, TTY_NORMAL);
+	tty_flip_buffer_push(priv->port.tty);
+
+	tty_kref_put(tty);
+}
+
+static int kgdb_nmi_tty_activate(struct tty_port *port, struct tty_struct *tty)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	kgdb_nmi_port = port;
+	tasklet_schedule(&priv->tlet);
+	return 0;
+}
+
+static void kgdb_nmi_tty_shutdown(struct tty_port *port)
+{
+	struct kgdb_nmi_tty_priv *priv = port->tty->driver_data;
+
+	tasklet_kill(&priv->tlet);
+	kgdb_nmi_port = NULL;
+}
+
+static const struct tty_port_operations kgdb_nmi_tty_port_ops = {
+	.activate	= kgdb_nmi_tty_activate,
+	.shutdown	= kgdb_nmi_tty_shutdown,
+};
+
+static int kgdb_nmi_tty_install(struct tty_driver *drv, struct tty_struct *tty)
+{
+	struct kgdb_nmi_tty_priv *priv;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	INIT_KFIFO(priv->fifo);
+	tasklet_init(&priv->tlet, kgdb_nmi_tty_receiver, (unsigned long)priv);
+	tty_port_init(&priv->port);
+	priv->port.ops = &kgdb_nmi_tty_port_ops;
+	tty->driver_data = priv;
+
+	ret = tty_port_install(&priv->port, drv, tty);
+	if (ret) {
+		pr_err("%s: can't install tty port: %d\n", __func__, ret);
+		goto err;
+	}
+	return 0;
+err:
+	kfree(priv);
+	return ret;
+}
+
+static void kgdb_nmi_tty_cleanup(struct tty_struct *tty)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	tty->driver_data = NULL;
+	kfree(priv);
+}
+
+static int kgdb_nmi_tty_open(struct tty_struct *tty, struct file *file)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	return tty_port_open(&priv->port, tty, file);
+}
+
+static void kgdb_nmi_tty_close(struct tty_struct *tty, struct file *file)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	tty_port_close(&priv->port, tty, file);
+}
+
+static void kgdb_nmi_tty_hangup(struct tty_struct *tty)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	tty_port_hangup(&priv->port);
+}
+
+static int kgdb_nmi_tty_write_room(struct tty_struct *tty)
+{
+	/* Actually, we can handle any amount as we use polled writes. */
+	return 2048;
+}
+
+static int kgdb_nmi_tty_write(struct tty_struct *tty, const unchar *buf, int c)
+{
+	int i;
+
+	for (i = 0; i < c; i++)
+		dbg_io_ops->write_char(buf[i]);
+	return c;
+}
+
+static const struct tty_operations kgdb_nmi_tty_ops = {
+	.open		= kgdb_nmi_tty_open,
+	.close		= kgdb_nmi_tty_close,
+	.install	= kgdb_nmi_tty_install,
+	.cleanup	= kgdb_nmi_tty_cleanup,
+	.hangup		= kgdb_nmi_tty_hangup,
+	.write_room	= kgdb_nmi_tty_write_room,
+	.write		= kgdb_nmi_tty_write,
+};
+
+static int kgdb_nmi_enable_console(int argc, const char *argv[])
+{
+	kgdb_nmi_tty_enabled = !(argc == 1 && !strcmp(argv[1], "off"));
+	return 0;
+}
+
+int kgdb_register_nmi_console(void)
+{
+	int ret;
+
+	kgdb_nmi_tty_driver = alloc_tty_driver(1);
+	if (!kgdb_nmi_tty_driver) {
+		pr_err("%s: cannot allocate tty\n", __func__);
+		return -ENOMEM;
+	}
+	kgdb_nmi_tty_driver->driver_name	= "ttyNMI";
+	kgdb_nmi_tty_driver->name		= "ttyNMI";
+	kgdb_nmi_tty_driver->num		= 1;
+	kgdb_nmi_tty_driver->type		= TTY_DRIVER_TYPE_SERIAL;
+	kgdb_nmi_tty_driver->subtype		= SERIAL_TYPE_NORMAL;
+	kgdb_nmi_tty_driver->flags		= TTY_DRIVER_REAL_RAW;
+	kgdb_nmi_tty_driver->init_termios	= tty_std_termios;
+	tty_termios_encode_baud_rate(&kgdb_nmi_tty_driver->init_termios,
+				     KGDB_NMI_BAUD, KGDB_NMI_BAUD);
+	tty_set_operations(kgdb_nmi_tty_driver, &kgdb_nmi_tty_ops);
+
+	ret = tty_register_driver(kgdb_nmi_tty_driver);
+	if (ret) {
+		pr_err("%s: can't register tty driver: %d\n", __func__, ret);
+		goto err_drv_reg;
+	}
+
+	ret = kdb_register("nmi_console", kgdb_nmi_enable_console, "[off]",
+			   "switch to Linux NMI console", 0);
+	if (ret) {
+		pr_err("%s: can't register kdb command: %d\n", __func__, ret);
+		goto err_kdb_reg;
+	}
+
+	register_console(&kgdb_nmi_console);
+	kgdb_enable_nmi(1);
+
+	return 0;
+err_kdb_reg:
+	tty_unregister_driver(kgdb_nmi_tty_driver);
+err_drv_reg:
+	put_tty_driver(kgdb_nmi_tty_driver);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kgdb_register_nmi_console);
+
+int kgdb_unregister_nmi_console(void)
+{
+	int ret;
+
+	kgdb_enable_nmi(0);
+	kdb_unregister("nmi_console");
+
+	ret = unregister_console(&kgdb_nmi_console);
+	if (ret)
+		return ret;
+
+	ret = tty_unregister_driver(kgdb_nmi_tty_driver);
+	if (ret)
+		return ret;
+	put_tty_driver(kgdb_nmi_tty_driver);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kgdb_unregister_nmi_console);
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index 2b42a01..3f63d83 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -145,6 +145,8 @@ __setup("kgdboc=", kgdboc_option_setup);
 
 static void cleanup_kgdboc(void)
 {
+	if (kgdb_unregister_nmi_console())
+		return;
 	kgdboc_unregister_kbd();
 	if (configured == 1)
 		kgdb_unregister_io_module(&kgdboc_io_ops);
@@ -198,11 +200,18 @@ do_register:
 	if (err)
 		goto noconfig;
 
+	err = kgdb_register_nmi_console();
+	if (err)
+		goto nmi_con_failed;
+
 	configured = 1;
 
 	return 0;
 
+nmi_con_failed:
+	kgdb_unregister_io_module(&kgdboc_io_ops);
 noconfig:
+	kgdboc_unregister_kbd();
 	config[0] = 0;
 	configured = 0;
 	cleanup_kgdboc();
diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 3b111a6..9bf888e 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -245,6 +245,16 @@ extern void kgdb_arch_enable_nmi(bool on);
  */
 extern int kgdb_enable_nmi(bool on);
 
+#ifdef CONFIG_SERIAL_KGDB_NMI
+extern int kgdb_register_nmi_console(void);
+extern int kgdb_unregister_nmi_console(void);
+extern bool kgdb_nmi_poll_knock(void);
+#else
+static inline int kgdb_register_nmi_console(void) { return 0; }
+static inline int kgdb_unregister_nmi_console(void) { return 0; }
+static inline bool kgdb_nmi_poll_knock(void) { return 1; }
+#endif
+
 /**
  * struct kgdb_arch - Describe architecture specific values.
  * @gdb_bpt_instr: The instruction to trigger a breakpoint.
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger
  2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
                   ` (11 preceding siblings ...)
  2012-09-13 17:04 ` [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Alan Cox
@ 2012-09-17 11:37 ` Greg Kroah-Hartman
  2012-09-17 12:43   ` Jason Wessel
  12 siblings, 1 reply; 29+ messages in thread
From: Greg Kroah-Hartman @ 2012-09-17 11:37 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Andrew Morton, Russell King, Jason Wessel, Alan Cox,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On Thu, Sep 13, 2012 at 08:01:33AM -0700, Anton Vorontsov wrote:
> Hi all,
> 
> Here comes the lucky v7:
> 
> - Per Alan Cox's suggestion added hangup method and removed a small
>   leftover;
> - Per Colin Cross' suggestion moved IRQ quiescing logic into
>   poll_get_char routine. IIUC, Alan is less unhappy about it.  As a
>   result, clear_irq() callback dropped.
> 
> These patches can be found in the following repo (based on tty-next):
> 
> 	git://git.infradead.org/users/cbou/linux-nmi-kdb.git master
> 
> Old changelogs and rationale for these patches can be found here:
> 
> 	v1-v5, rationale: http://lkml.org/lkml/2012/9/10/2
> 	v6: http://lkml.org/lkml/2012/9/10/2

I have no objection to these patches, and as they are based on my
tty-next tree, should I be the ones accepting them?  If so, then I need
a bunch of acks from others involved in the kdb/kgdb code before I can
do so.

To make it easier, can I just take the tty driver patches now?  Will
that break anything?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger
  2012-09-17 11:37 ` Greg Kroah-Hartman
@ 2012-09-17 12:43   ` Jason Wessel
  2012-09-17 12:54     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 29+ messages in thread
From: Jason Wessel @ 2012-09-17 12:43 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Anton Vorontsov, Andrew Morton, Russell King, Alan Cox,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On 09/17/2012 06:37 AM, Greg Kroah-Hartman wrote:
> On Thu, Sep 13, 2012 at 08:01:33AM -0700, Anton Vorontsov wrote:
>> Hi all,
>>
>> Here comes the lucky v7:
>>
>> - Per Alan Cox's suggestion added hangup method and removed a small
>>   leftover;
>> - Per Colin Cross' suggestion moved IRQ quiescing logic into
>>   poll_get_char routine. IIUC, Alan is less unhappy about it.  As a
>>   result, clear_irq() callback dropped.
>>
>> These patches can be found in the following repo (based on tty-next):
>>
>> 	git://git.infradead.org/users/cbou/linux-nmi-kdb.git master
>>
>> Old changelogs and rationale for these patches can be found here:
>>
>> 	v1-v5, rationale: http://lkml.org/lkml/2012/9/10/2
>> 	v6: http://lkml.org/lkml/2012/9/10/2
> I have no objection to these patches, and as they are based on my
> tty-next tree, should I be the ones accepting them?  If so, then I need
> a bunch of acks from others involved in the kdb/kgdb code before I can
> do so.
>
> To make it easier, can I just take the tty driver patches now?  Will
> that break anything?

I have not made my way through the entire series yet, so I am not sure if you need the kdb header changes or not, but so far it looks like the tty pieces are separate.  If you add your ack Greg, I'll take the whole series and merge it into kgdb-next, or after I finish the review we could do it the other way around.

Cheers,
Jason.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger
  2012-09-17 12:43   ` Jason Wessel
@ 2012-09-17 12:54     ` Greg Kroah-Hartman
  2012-09-17 15:39       ` Anton Vorontsov
  0 siblings, 1 reply; 29+ messages in thread
From: Greg Kroah-Hartman @ 2012-09-17 12:54 UTC (permalink / raw)
  To: Jason Wessel
  Cc: Anton Vorontsov, Andrew Morton, Russell King, Alan Cox,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On Mon, Sep 17, 2012 at 07:43:11AM -0500, Jason Wessel wrote:
> On 09/17/2012 06:37 AM, Greg Kroah-Hartman wrote:
> > On Thu, Sep 13, 2012 at 08:01:33AM -0700, Anton Vorontsov wrote:
> >> Hi all,
> >>
> >> Here comes the lucky v7:
> >>
> >> - Per Alan Cox's suggestion added hangup method and removed a small
> >>   leftover;
> >> - Per Colin Cross' suggestion moved IRQ quiescing logic into
> >>   poll_get_char routine. IIUC, Alan is less unhappy about it.  As a
> >>   result, clear_irq() callback dropped.
> >>
> >> These patches can be found in the following repo (based on tty-next):
> >>
> >> 	git://git.infradead.org/users/cbou/linux-nmi-kdb.git master
> >>
> >> Old changelogs and rationale for these patches can be found here:
> >>
> >> 	v1-v5, rationale: http://lkml.org/lkml/2012/9/10/2
> >> 	v6: http://lkml.org/lkml/2012/9/10/2
> > I have no objection to these patches, and as they are based on my
> > tty-next tree, should I be the ones accepting them?  If so, then I need
> > a bunch of acks from others involved in the kdb/kgdb code before I can
> > do so.
> >
> > To make it easier, can I just take the tty driver patches now?  Will
> > that break anything?
> 
> I have not made my way through the entire series yet, so I am not sure
> if you need the kdb header changes or not, but so far it looks like
> the tty pieces are separate.  If you add your ack Greg, I'll take the
> whole series and merge it into kgdb-next, or after I finish the review
> we could do it the other way around.

Your tree is fine:

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Let me know if you have problems merging this due to any tty patches,
and if so, I'll be glad to take them through my tree.

greg k-h

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger
  2012-09-17 12:54     ` Greg Kroah-Hartman
@ 2012-09-17 15:39       ` Anton Vorontsov
  2012-09-17 15:54         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-17 15:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jason Wessel, Andrew Morton, Russell King, Alan Cox,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On Mon, Sep 17, 2012 at 05:54:27AM -0700, Greg Kroah-Hartman wrote:
[...]
> > > To make it easier, can I just take the tty driver patches now?  Will
> > > that break anything?
> > 
> > I have not made my way through the entire series yet, so I am not sure
> > if you need the kdb header changes or not, but so far it looks like
> > the tty pieces are separate.  If you add your ack Greg, I'll take the
> > whole series and merge it into kgdb-next, or after I finish the review
> > we could do it the other way around.
> 
> Your tree is fine:
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Let me know if you have problems merging this due to any tty patches,
> and if so, I'll be glad to take them through my tree.

Guys, thanks for taking a look into this!

The patch that adds nmi console driver (i.e. tty/serial/kgdb_nmi.c)
depends on the first KDB patches in these series, and it depends on
amba-pl1011 and tty_port work, which is in tty-next.

So, the series have to go via tty-next; I believe Jason won't able
to take it via his tree.

Thanks,

Anton.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger
  2012-09-17 15:39       ` Anton Vorontsov
@ 2012-09-17 15:54         ` Greg Kroah-Hartman
  2012-09-19 11:57           ` Jason Wessel
  0 siblings, 1 reply; 29+ messages in thread
From: Greg Kroah-Hartman @ 2012-09-17 15:54 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Jason Wessel, Andrew Morton, Russell King, Alan Cox,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On Mon, Sep 17, 2012 at 08:39:43AM -0700, Anton Vorontsov wrote:
> On Mon, Sep 17, 2012 at 05:54:27AM -0700, Greg Kroah-Hartman wrote:
> [...]
> > > > To make it easier, can I just take the tty driver patches now?  Will
> > > > that break anything?
> > > 
> > > I have not made my way through the entire series yet, so I am not sure
> > > if you need the kdb header changes or not, but so far it looks like
> > > the tty pieces are separate.  If you add your ack Greg, I'll take the
> > > whole series and merge it into kgdb-next, or after I finish the review
> > > we could do it the other way around.
> > 
> > Your tree is fine:
> > 
> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > Let me know if you have problems merging this due to any tty patches,
> > and if so, I'll be glad to take them through my tree.
> 
> Guys, thanks for taking a look into this!
> 
> The patch that adds nmi console driver (i.e. tty/serial/kgdb_nmi.c)
> depends on the first KDB patches in these series, and it depends on
> amba-pl1011 and tty_port work, which is in tty-next.
> 
> So, the series have to go via tty-next; I believe Jason won't able
> to take it via his tree.

Ok, I'll wait for Jason to review them.  Jason, let me know how it
goes...

greg k-h

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/11] kernel/debug: Mask KGDB NMI upon entry
  2012-09-13 15:03 ` [PATCH 01/11] kernel/debug: Mask KGDB NMI upon entry Anton Vorontsov
@ 2012-09-19 11:52   ` Jason Wessel
  0 siblings, 0 replies; 29+ messages in thread
From: Jason Wessel @ 2012-09-19 11:52 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Andrew Morton, Russell King, Greg Kroah-Hartman, Alan Cox,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On 09/13/2012 10:03 AM, Anton Vorontsov wrote:
> The new arch callback should manage NMIs that usually cause KGDB to
> enter. That is, not all NMIs should be enabled/disabled, but only
> those that issue kgdb_handle_exception().
> 
> We must mask it as serial-line interrupt can be used as an NMI, so
> if the original KGDB-entry cause was say a breakpoint, then every
> input to KDB console will cause KGDB to reenter, which we don't want.
> 
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
> ---
>  include/linux/kgdb.h      | 23 +++++++++++++++++++++++
>  kernel/debug/debug_core.c | 36 +++++++++++++++++++++++++++++++++---
>  2 files changed, 56 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> index c4d2fc1..3b111a6 100644
> --- a/include/linux/kgdb.h
> +++ b/include/linux/kgdb.h
> @@ -221,6 +221,29 @@ extern int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt);
>   */
>  extern void kgdb_arch_late(void);
>  
> +/**
> + *	kgdb_arch_enable_nmi - Enable or disable KGDB-entry NMI
> + *	@on: Flag to either enable or disable an NMI
> + *
> + *	This is an architecture-specific "back-end" for kgdb_enable_nmi(). The
> + *	call does not count disable/enable requests, do not use it directly.
> + */
> +extern void kgdb_arch_enable_nmi(bool on);

I realize this is more of a clean up, and this doesn't necessarily
have to gate the acceptance of these patches to the mainline, but I
would like to not see us using the kgdb_arch_enable_nmi() as a weak
function.  This belongs in:

struct kgdb_arch {
	...
	void enable_nmi(bool on);
}


> +
> +/**
> + *	kgdb_enable_nmi - Enable or disable KGDB-entry NMI
> + *	@on: Flag to either enable or disable an NMI
> + *
> + *	This function manages NMIs that usually cause KGDB to enter. That is,
> + *	not all NMIs should be enabled or disabled, but only those that issue
> + *	kgdb_handle_exception().
> + *
> + *	The call counts disable requests, and thus allows to nest disables.
> + *	But trying to enable already enabled NMI is an error. The call returns
> + *	1 if NMI has been actually enabled after the call, and a value <= 0 if
> + *	it is still disabled.
> + */
> +extern int kgdb_enable_nmi(bool on);


Clearly you need something arch specific here.  With respect to the
atomic math, it is not clear that would be the case for this function
unilaterally accross other architectures.  This comment block and the
"implementation" code belong in the arch stub: arch/kernel/arm/kgdb.c
The whole kgdb_enable_nmi can go away if using the struct kgdb_arch
callbacks


> --- a/kernel/debug/debug_core.c
> +++ b/kernel/debug/debug_core.c
> @@ -214,6 +214,30 @@ int __weak kgdb_skipexception(int exception, struct pt_regs *regs)
>  	return 0;
>  }
>  
> +void __weak kgdb_arch_enable_nmi(bool on)
> +{
> +}
> +
> +int kgdb_enable_nmi(bool on)
> +{
> +	static atomic_t cnt;
> +	int ret;
> +
> +	ret = atomic_add_return(on ? 1 : -1, &cnt);
> +	if (ret > 1 && on) {
> +		/*
> +		 * There should be only one instance that calls this function
> +		 * in "enable, disable" order. All other users must call
> +		 * disable first, then enable. If not, something is wrong.
> +		 */
> +		WARN_ON(1);
> +		return 1;
> +	}
> +
> +	kgdb_arch_enable_nmi(ret > 0);
> +	return ret;
> +}
> +
>  /*
>   * Some architectures need cache flushes when we set/clear a
>   * breakpoint:
> @@ -672,6 +696,9 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
>  {
>  	struct kgdb_state kgdb_var;
>  	struct kgdb_state *ks = &kgdb_var;
> +	int ret = 0;
> +
> +	kgdb_enable_nmi(0);


With what I mentioned before this becomes:

	if (arch_kgdb_ops->enable_nmi)
		arch_kgdb_ops->enable_nmi(0)


>  
>  	ks->cpu			= raw_smp_processor_id();
>  	ks->ex_vector		= evector;
> @@ -681,11 +708,14 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
>  	ks->linux_regs		= regs;
>  
>  	if (kgdb_reenter_check(ks))
> -		return 0; /* Ouch, double exception ! */
> +		goto out; /* Ouch, double exception ! */
>  	if (kgdb_info[ks->cpu].enter_kgdb != 0)
> -		return 0;
> +		goto out;
>  
> -	return kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
> +	ret = kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
> +out:
> +	kgdb_enable_nmi(1);


Becomes:
	if (arch_kgdb_ops->enable_nmi)
		arch_kgdb_ops->enable_nmi(1)


> +	return ret;
>  }
>  
>  int kgdb_nmicallback(int cpu, void *regs)
> 


Cheers,
Jason.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 02/11] kdb: Implement disable_nmi command
  2012-09-13 15:03 ` [PATCH 02/11] kdb: Implement disable_nmi command Anton Vorontsov
@ 2012-09-19 11:52   ` Jason Wessel
  0 siblings, 0 replies; 29+ messages in thread
From: Jason Wessel @ 2012-09-19 11:52 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Andrew Morton, Russell King, Greg Kroah-Hartman, Alan Cox,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On 09/13/2012 10:03 AM, Anton Vorontsov wrote:
> This command disables NMI-entry. If NMI source has been previously shared
> with a serial console ("debug port"), this effectively releases the port
> from KDB exclusive use, and makes the console available for normal use.
> 
> Of course, NMI can be reenabled, enable_nmi modparam is used for that:
> 
> 	echo 1 > /sys/module/kdb/parameters/enable_nmi
> 
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
> ---
>  kernel/debug/kdb/kdb_main.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 31df170..9fadff1 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -21,6 +21,7 @@
>  #include <linux/smp.h>
>  #include <linux/utsname.h>
>  #include <linux/vmalloc.h>
> +#include <linux/atomic.h>
>  #include <linux/module.h>
>  #include <linux/mm.h>
>  #include <linux/init.h>
> @@ -2107,6 +2108,32 @@ static int kdb_dmesg(int argc, const char **argv)
>  	return 0;
>  }
>  #endif /* CONFIG_PRINTK */
> +
> +/* Make sure we balance enable/disable calls, must disable first. */
> +static atomic_t kdb_nmi_disabled;
> +
> +static int kdb_disable_nmi(int argc, const char *argv[])
> +{
> +	if (atomic_read(&kdb_nmi_disabled))
> +		return 0;
> +	atomic_set(&kdb_nmi_disabled, 1);
> +	kgdb_enable_nmi(0);
> +	return 0;
> +}
> +
> +static int kdb_param_enable_nmi(const char *val, const struct kernel_param *kp)
> +{
> +	if (!atomic_add_unless(&kdb_nmi_disabled, -1, 0))
> +		return -EINVAL;
> +	kgdb_enable_nmi(1);
> +	return 0;
> +}
> +
> +static const struct kernel_param_ops kdb_param_ops_enable_nmi = {
> +	.set = kdb_param_enable_nmi,
> +};
> +module_param_cb(enable_nmi, &kdb_param_ops_enable_nmi, NULL, 0600);
> +
>  /*
>   * kdb_cpu - This function implements the 'cpu' command.
>   *	cpu	[<cpunum>]
> @@ -2851,6 +2878,8 @@ static void __init kdb_inittab(void)
>  	kdb_register_repeat("dmesg", kdb_dmesg, "[lines]",
>  	  "Display syslog buffer", 0, KDB_REPEAT_NONE);
>  #endif


Based on what I commented in the 01/11 patch, we don't want to register a command if the architecture doesn't actually have it, because it just leads to confusion.

It needs to have an "if"

if (arch_kgdb_ops->enable_nmi) {


> +	kdb_register_repeat("disable_nmi", kdb_disable_nmi, "",
> +	  "Disable NMI entry to KDB", 0, KDB_REPEAT_NONE);

}

>  	kdb_register_repeat("defcmd", kdb_defcmd, "name \"usage\" \"help\"",
>  	  "Define a set of commands, down to endefcmd", 0, KDB_REPEAT_NONE);
>  	kdb_register_repeat("kill", kdb_kill, "<-signal> <pid>",
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 07/11] tty/serial: Add kgdb_nmi driver
  2012-09-13 15:03 ` [PATCH 07/11] tty/serial: Add kgdb_nmi driver Anton Vorontsov
  2012-09-15 15:52   ` Francesco Lavra
@ 2012-09-19 11:52   ` Jason Wessel
  2012-09-19 16:54     ` Anton Vorontsov
  1 sibling, 1 reply; 29+ messages in thread
From: Jason Wessel @ 2012-09-19 11:52 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Andrew Morton, Russell King, Greg Kroah-Hartman, Alan Cox,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On 09/13/2012 10:03 AM, Anton Vorontsov wrote:
> This special driver makes it possible to temporary use NMI debugger port
> as a normal console by issuing 'nmi_console' command (assuming that the
> port is attached to KGDB).
> 
> Unlike KDB's disable_nmi command, with this driver you are always able
> to go back to the debugger using KGDB escape sequence ($3#33).  This is
> because this console driver processes the input in NMI context, and thus
> is able to intercept the magic sequence.
> 
> Note that since the console interprets input and uses polling
> communication methods, for things like PPP it is still better to fully
> detach debugger port from the KGDB NMI (i.e. disable_nmi), and use raw
> console.
> 
> Usually, to enter the debugger one have to type the magic sequence, so
> initially the kernel will print the following prompt on the NMI debugger
> console:
> 
> 	Type $3#33 to enter the debugger>
> 
> For convenience, there is a kgdb_fiq.knock kernel command line option,
> when set to 0, this turns the special command to just a return key
> press, so the kernel will be printing this:
> 
> 	Hit <return> to enter the debugger>
> 
> This is more convenient for long debugging sessions, although it makes
> nmi_console feature somewhat useless.
> 
> And for the cases when NMI connected to a dedicated button, the knocking
> can be disabled altogether by setting kgdb_fiq.knock to -1.
> 
> Suggested-by: Colin Cross <ccross@android.com>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
> ---
>  drivers/tty/serial/Kconfig    |  19 ++
>  drivers/tty/serial/Makefile   |   1 +
>  drivers/tty/serial/kgdb_nmi.c | 396 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/tty/serial/kgdboc.c   |   6 +
>  include/linux/kgdb.h          |  10 ++
>  5 files changed, 432 insertions(+)
>  create mode 100644 drivers/tty/serial/kgdb_nmi.c
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 26907cf..b22e45b 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -141,6 +141,25 @@ config SERIAL_ATMEL_TTYAT
>  
>  	  Say Y if you have an external 8250/16C550 UART.  If unsure, say N.
>  
> +config SERIAL_KGDB_NMI
> +	bool "Serial console over KGDB NMI debugger port"
> +	depends on KGDB_SERIAL_CONSOLE
> +	help
> +	  This special driver allows you to temporary use NMI debugger port
> +	  as a normal console (assuming that the port is attached to KGDB).
> +
> +	  Unlike KDB's disable_nmi command, with this driver you are always
> +	  able to go back to the debugger using KGDB escape sequence ($3#33).
> +	  This is because this console driver processes the input in NMI
> +	  context, and thus is able to intercept the magic sequence.
> +
> +	  Note that since the console interprets input and uses polling
> +	  communication methods, for things like PPP you still must fully
> +	  detach debugger port from the KGDB NMI (i.e. disable_nmi), and
> +	  use raw console.
> +
> +	  If unsure, say N.
> +
>  config SERIAL_KS8695
>  	bool "Micrel KS8695 (Centaur) serial port support"
>  	depends on ARCH_KS8695
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index ce88667..4f694da 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_SERIAL_MSM_HS) += msm_serial_hs.o
>  obj-$(CONFIG_SERIAL_NETX) += netx-serial.o
>  obj-$(CONFIG_SERIAL_OF_PLATFORM) += of_serial.o
>  obj-$(CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL) += nwpserial.o
> +obj-$(CONFIG_SERIAL_KGDB_NMI) += kgdb_nmi.o
>  obj-$(CONFIG_SERIAL_KS8695) += serial_ks8695.o
>  obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
>  obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o
> diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c
> new file mode 100644
> index 0000000..57bf744
> --- /dev/null
> +++ b/drivers/tty/serial/kgdb_nmi.c
> @@ -0,0 +1,396 @@
> +/*
> + * KGDB NMI serial console
> + *
> + * Copyright 2010 Google, Inc.
> + *		  Arve Hjønnevåg <arve@android.com>
> + *		  Colin Cross <ccross@android.com>
> + * Copyright 2012 Linaro Ltd.
> + *		  Anton Vorontsov <anton.vorontsov@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/compiler.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/errno.h>
> +#include <linux/atomic.h>
> +#include <linux/console.h>
> +#include <linux/tty.h>
> +#include <linux/tty_driver.h>
> +#include <linux/tty_flip.h>
> +#include <linux/interrupt.h>
> +#include <linux/hrtimer.h>
> +#include <linux/tick.h>
> +#include <linux/kfifo.h>
> +#include <linux/kgdb.h>
> +#include <linux/kdb.h>
> +
> +static int kgdb_nmi_knock = 1;
> +module_param_named(knock, kgdb_nmi_knock, int, 0600);
> +MODULE_PARM_DESC(knock, "if set to 1 (default), the special '$3#33' command "
> +			"must be used to enter the debugger; when set to 0, "
> +			"hitting return key is enough to enter the debugger; "
> +			"when set to -1, the debugger is entered immediately "
> +			"upon NMI");
> +
> +static char *kgdb_nmi_magic = "$3#33";
> +module_param_named(magic, kgdb_nmi_magic, charp, 0600);
> +MODULE_PARM_DESC(magic, "magic sequence to enter NMI debugger (default $3#33)");
> +
> +static bool kgdb_nmi_tty_enabled;
> +
> +static void kgdb_nmi_console_write(struct console *co, const char *s, uint c)
> +{
> +	int i;
> +
> +	if (!kgdb_nmi_tty_enabled || atomic_read(&kgdb_active) >= 0)
> +		return;
> +
> +	for (i = 0; i < c; i++)
> +		dbg_io_ops->write_char(s[i]);
> +}
> +
> +static struct tty_driver *kgdb_nmi_tty_driver;
> +
> +static struct tty_driver *kgdb_nmi_console_device(struct console *co, int *idx)
> +{
> +	*idx = co->index;
> +	return kgdb_nmi_tty_driver;
> +}
> +
> +static struct console kgdb_nmi_console = {
> +	.name	= "ttyNMI",
> +	.write	= kgdb_nmi_console_write,
> +	.device	= kgdb_nmi_console_device,
> +	.flags	= CON_PRINTBUFFER | CON_ANYTIME | CON_ENABLED,
> +	.index	= -1,
> +};
> +
> +/*
> + * This is usually the maximum rate on debug ports. We make fifo large enough
> + * to make copy-pasting to the terminal usable.
> + */
> +#define KGDB_NMI_BAUD		115200
> +#define KGDB_NMI_FIFO_SIZE	roundup_pow_of_two(KGDB_NMI_BAUD / 8 / HZ)
> +
> +struct kgdb_nmi_tty_priv {
> +	struct tty_port port;
> +	struct tasklet_struct tlet;
> +	STRUCT_KFIFO(char, KGDB_NMI_FIFO_SIZE) fifo;
> +};
> +
> +static struct kgdb_nmi_tty_priv *kgdb_nmi_port_to_priv(struct tty_port *port)
> +{
> +	return container_of(port, struct kgdb_nmi_tty_priv, port);
> +}
> +
> +/*
> + * Our debugging console is polled in a tasklet, so we'll check for input
> + * every tick. In HZ-less mode, we should program the next tick.  We have
> + * to use the lowlevel stuff as no locks should be grabbed.
> + */
> +#ifdef CONFIG_HIGH_RES_TIMERS
> +static void kgdb_tty_poke(void)
> +{
> +	tick_program_event(ktime_get(), 0);
> +}
> +#else
> +static inline void kgdb_tty_poke(void) {}
> +#endif
> +
> +static struct tty_port *kgdb_nmi_port;
> +
> +static void kgdb_tty_recv(int ch)
> +{
> +	struct kgdb_nmi_tty_priv *priv;
> +	char c = ch;
> +
> +	if (!kgdb_nmi_port || ch < 0)
> +		return;
> +	/*
> +	 * Can't use port->tty->driver_data as tty might be not there. Tasklet
> +	 * will check for tty and will get the ref, but here we don't have to
> +	 * do that, and actually, we can't: we're in NMI context, no locks are
> +	 * possible.
> +	 */
> +	priv = kgdb_nmi_port_to_priv(kgdb_nmi_port);
> +	kfifo_in(&priv->fifo, &c, 1);
> +	kgdb_tty_poke();
> +}
> +
> +static int kgdb_nmi_poll_one_knock(void)
> +{
> +	static int n;
> +	int c = -1;
> +	const char *magic = kgdb_nmi_magic;
> +	size_t m = strlen(magic);
> +	bool printch = 0;
> +
> +	c = dbg_io_ops->read_char();
> +	if (c == NO_POLL_CHAR)
> +		return c;
> +
> +	if (!kgdb_nmi_knock && (c == '\r' || c == '\n')) {
> +		return 1;
> +	} else if (c == magic[n]) {
> +		n = (n + 1) % m;
> +		if (!n)
> +			return 1;
> +		printch = 1;
> +	} else {
> +		n = 0;
> +	}
> +
> +	if (kgdb_nmi_tty_enabled) {
> +		kgdb_tty_recv(c);
> +		return 0;
> +	}
> +
> +	if (printch) {
> +		kdb_printf("%c", c);
> +		return 0;
> +	}
> +
> +	kdb_printf("\r%s %s to enter the debugger> %*s",
> +		   kgdb_nmi_knock ? "Type" : "Hit",
> +		   kgdb_nmi_knock ? magic  : "<return>", m, "");
> +	while (m--)
> +		kdb_printf("\b");
> +	return 0;
> +}
> +
> +/**
> + * kgdb_nmi_poll_knock - Check if it is time to enter the debugger
> + *
> + * "Serial ports are often noisy, especially when muxed over another port (we
> + * often use serial over the headset connector). Noise on the async command
> + * line just causes characters that are ignored, on a command line that blocked
> + * execution noise would be catastrophic." -- Colin Cross
> + *
> + * So, this function implements KGDB/KDB knocking on the serial line: we won't
> + * enter the debugger until we receive a known magic phrase (which is actually
> + * "$3#33", known as "escape to KDB" command. There is also a relaxed variant
> + * of knocking, i.e. just pressing the return key is enough to enter the
> + * debugger. And if knocking is disabled, the function always returns 1.
> + */
> +bool kgdb_nmi_poll_knock(void)
> +{
> +	if (kgdb_nmi_knock < 0)
> +		return 1;
> +
> +	while (1) {
> +		int ret;
> +
> +		ret = kgdb_nmi_poll_one_knock();
> +		if (ret == NO_POLL_CHAR)
> +			return 0;
> +		else if (ret == 1)
> +			break;
> +	}
> +	return 1;
> +}
> +
> +/*
> + * The tasklet is cheap, it does not cause wakeups when reschedules itself,
> + * instead it waits for the next tick.
> + */
> +static void kgdb_nmi_tty_receiver(unsigned long data)
> +{
> +	struct kgdb_nmi_tty_priv *priv = (void *)data;
> +	struct tty_struct *tty;
> +	char ch;
> +
> +	tasklet_schedule(&priv->tlet);
> +
> +	if (likely(!kgdb_nmi_tty_enabled || !kfifo_len(&priv->fifo)))
> +		return;
> +
> +	/* Port is there, but tty might be hung up, check. */
> +	tty = tty_port_tty_get(kgdb_nmi_port);
> +	if (!tty)
> +		return;
> +
> +	while (kfifo_out(&priv->fifo, &ch, 1))
> +		tty_insert_flip_char(priv->port.tty, ch, TTY_NORMAL);
> +	tty_flip_buffer_push(priv->port.tty);
> +
> +	tty_kref_put(tty);
> +}
> +
> +static int kgdb_nmi_tty_activate(struct tty_port *port, struct tty_struct *tty)
> +{
> +	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
> +
> +	kgdb_nmi_port = port;
> +	tasklet_schedule(&priv->tlet);
> +	return 0;
> +}
> +
> +static void kgdb_nmi_tty_shutdown(struct tty_port *port)
> +{
> +	struct kgdb_nmi_tty_priv *priv = port->tty->driver_data;
> +
> +	tasklet_kill(&priv->tlet);
> +	kgdb_nmi_port = NULL;
> +}
> +
> +static const struct tty_port_operations kgdb_nmi_tty_port_ops = {
> +	.activate	= kgdb_nmi_tty_activate,
> +	.shutdown	= kgdb_nmi_tty_shutdown,
> +};
> +
> +static int kgdb_nmi_tty_install(struct tty_driver *drv, struct tty_struct *tty)
> +{
> +	struct kgdb_nmi_tty_priv *priv;
> +	int ret;
> +
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	INIT_KFIFO(priv->fifo);
> +	tasklet_init(&priv->tlet, kgdb_nmi_tty_receiver, (unsigned long)priv);
> +	tty_port_init(&priv->port);
> +	priv->port.ops = &kgdb_nmi_tty_port_ops;
> +	tty->driver_data = priv;
> +
> +	ret = tty_port_install(&priv->port, drv, tty);
> +	if (ret) {
> +		pr_err("%s: can't nstall tty port: %d\n", __func__, ret);
> +		goto err;
> +	}
> +	return 0;
> +err:
> +	kfree(priv);
> +	return ret;
> +}
> +
> +static void kgdb_nmi_tty_cleanup(struct tty_struct *tty)
> +{
> +	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
> +
> +	tty->driver_data = NULL;
> +	kfree(priv);
> +}
> +
> +static int kgdb_nmi_tty_open(struct tty_struct *tty, struct file *file)
> +{
> +	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
> +
> +	return tty_port_open(&priv->port, tty, file);
> +}
> +
> +static void kgdb_nmi_tty_close(struct tty_struct *tty, struct file *file)
> +{
> +	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
> +
> +	tty_port_close(&priv->port, tty, file);
> +}
> +
> +static void kgdb_nmi_tty_hangup(struct tty_struct *tty)
> +{
> +	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
> +
> +	tty_port_hangup(&priv->port);
> +}
> +
> +static int kgdb_nmi_tty_write_room(struct tty_struct *tty)
> +{
> +	/* Actually, we can handle any amount as we use polled writes. */
> +	return 2048;
> +}
> +
> +static int kgdb_nmi_tty_write(struct tty_struct *tty, const unchar *buf, int c)
> +{
> +	int i;
> +
> +	for (i = 0; i < c; i++)
> +		dbg_io_ops->write_char(buf[i]);
> +	return c;
> +}
> +
> +static const struct tty_operations kgdb_nmi_tty_ops = {
> +	.open		= kgdb_nmi_tty_open,
> +	.close		= kgdb_nmi_tty_close,
> +	.install	= kgdb_nmi_tty_install,
> +	.cleanup	= kgdb_nmi_tty_cleanup,
> +	.hangup		= kgdb_nmi_tty_hangup,
> +	.write_room	= kgdb_nmi_tty_write_room,
> +	.write		= kgdb_nmi_tty_write,
> +};
> +
> +static int kgdb_nmi_enable_console(int argc, const char *argv[])
> +{
> +	kgdb_nmi_tty_enabled = !(argc == 1 && !strcmp(argv[1], "off"));
> +	return 0;
> +}
> +
> +int kgdb_register_nmi_console(void)
> +{
> +	int ret;
> +
> +	kgdb_nmi_tty_driver = alloc_tty_driver(1);
> +	if (!kgdb_nmi_tty_driver) {
> +		pr_err("%s: cannot allocate tty\n", __func__);
> +		return -ENOMEM;
> +	}
> +	kgdb_nmi_tty_driver->driver_name	= "ttyNMI";
> +	kgdb_nmi_tty_driver->name		= "ttyNMI";
> +	kgdb_nmi_tty_driver->num		= 1;
> +	kgdb_nmi_tty_driver->type		= TTY_DRIVER_TYPE_SERIAL;
> +	kgdb_nmi_tty_driver->subtype		= SERIAL_TYPE_NORMAL;
> +	kgdb_nmi_tty_driver->flags		= TTY_DRIVER_REAL_RAW;
> +	kgdb_nmi_tty_driver->init_termios	= tty_std_termios;
> +	tty_termios_encode_baud_rate(&kgdb_nmi_tty_driver->init_termios,
> +				     KGDB_NMI_BAUD, KGDB_NMI_BAUD);
> +	tty_set_operations(kgdb_nmi_tty_driver, &kgdb_nmi_tty_ops);
> +
> +	ret = tty_register_driver(kgdb_nmi_tty_driver);
> +	if (ret) {
> +		pr_err("%s: can't register tty driver: %d\n", __func__, ret);
> +		goto err_drv_reg;
> +	}
> +
> +	ret = kdb_register("nmi_console", kgdb_nmi_enable_console, "[off]",
> +			   "switch to Linux NMI console", 0);
> +	if (ret) {
> +		pr_err("%s: can't register kdb command: %d\n", __func__, ret);
> +		goto err_kdb_reg;
> +	}
> +
> +	register_console(&kgdb_nmi_console);
> +	kgdb_enable_nmi(1);
> +
> +	return 0;
> +err_kdb_reg:
> +	tty_unregister_driver(kgdb_nmi_tty_driver);
> +err_drv_reg:
> +	put_tty_driver(kgdb_nmi_tty_driver);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(kgdb_register_nmi_console);
> +
> +int kgdb_unregister_nmi_console(void)
> +{
> +	int ret;
> +
> +	kgdb_enable_nmi(0);
> +	kdb_unregister("nmi_console");
> +
> +	ret = unregister_console(&kgdb_nmi_console);
> +	if (ret)
> +		return ret;
> +
> +	ret = tty_unregister_driver(kgdb_nmi_tty_driver);
> +	if (ret)
> +		return ret;
> +	put_tty_driver(kgdb_nmi_tty_driver);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(kgdb_unregister_nmi_console);
> diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> index 2b42a01..ed97cfd 100644
> --- a/drivers/tty/serial/kgdboc.c
> +++ b/drivers/tty/serial/kgdboc.c
> @@ -145,6 +145,8 @@ __setup("kgdboc=", kgdboc_option_setup);
>  
>  static void cleanup_kgdboc(void)
>  {
> +	if (kgdb_unregister_nmi_console())
> +		return;
>  	kgdboc_unregister_kbd();
>  	if (configured == 1)
>  		kgdb_unregister_io_module(&kgdboc_io_ops);
> @@ -198,6 +200,10 @@ do_register:
>  	if (err)
>  		goto noconfig;
>  
> +	err = kgdb_register_nmi_console();
> +	if (err)
> +		goto noconfig;
> +


Just one question on this one, what do you expect to happen, or how does this work when you use a different serial port for example on a board that has a real serial port and an FIQ port?   It was not obvious that there was a path to check if the port you are registering is an FIQ enabled port and then call the kgdb_register_nmi_console at that point.

Jason.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger
  2012-09-17 15:54         ` Greg Kroah-Hartman
@ 2012-09-19 11:57           ` Jason Wessel
  2012-09-19 16:57             ` Anton Vorontsov
  0 siblings, 1 reply; 29+ messages in thread
From: Jason Wessel @ 2012-09-19 11:57 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Anton Vorontsov, Andrew Morton, Russell King, Alan Cox,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On 09/17/2012 10:54 AM, Greg Kroah-Hartman wrote:
> On Mon, Sep 17, 2012 at 08:39:43AM -0700, Anton Vorontsov wrote:
>> On Mon, Sep 17, 2012 at 05:54:27AM -0700, Greg Kroah-Hartman wrote:
>> [...]
>>>>> To make it easier, can I just take the tty driver patches now?  Will
>>>>> that break anything?
>>>> I have not made my way through the entire series yet, so I am not sure
>>>> if you need the kdb header changes or not, but so far it looks like
>>>> the tty pieces are separate.  If you add your ack Greg, I'll take the
>>>> whole series and merge it into kgdb-next, or after I finish the review
>>>> we could do it the other way around.
>>> Your tree is fine:
>>>
>>> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>
>>> Let me know if you have problems merging this due to any tty patches,
>>> and if so, I'll be glad to take them through my tree.
>> Guys, thanks for taking a look into this!
>>
>> The patch that adds nmi console driver (i.e. tty/serial/kgdb_nmi.c)
>> depends on the first KDB patches in these series, and it depends on
>> amba-pl1011 and tty_port work, which is in tty-next.
>>
>> So, the series have to go via tty-next; I believe Jason won't able
>> to take it via his tree.
> Ok, I'll wait for Jason to review them.  Jason, let me know how it
> goes...
>

Other than 1, 2 and 7.   They all get:

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

I ran out of time to review things yesterday, but it is all reviewed now.   After we get some more comments from Anton, we can move forward.

Cheers,
Jason.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 07/11] tty/serial: Add kgdb_nmi driver
  2012-09-19 11:52   ` [PATCH " Jason Wessel
@ 2012-09-19 16:54     ` Anton Vorontsov
  0 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-19 16:54 UTC (permalink / raw)
  To: Jason Wessel
  Cc: Andrew Morton, Russell King, Greg Kroah-Hartman, Alan Cox,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On Wed, Sep 19, 2012 at 06:52:32AM -0500, Jason Wessel wrote:
[...]
> > --- a/drivers/tty/serial/kgdboc.c
> > +++ b/drivers/tty/serial/kgdboc.c
> > @@ -145,6 +145,8 @@ __setup("kgdboc=", kgdboc_option_setup);
> >  
> >  static void cleanup_kgdboc(void)
> >  {
> > +	if (kgdb_unregister_nmi_console())
> > +		return;
> >  	kgdboc_unregister_kbd();
> >  	if (configured == 1)
> >  		kgdb_unregister_io_module(&kgdboc_io_ops);
> > @@ -198,6 +200,10 @@ do_register:
> >  	if (err)
> >  		goto noconfig;
> >  
> > +	err = kgdb_register_nmi_console();
> > +	if (err)
> > +		goto noconfig;
> > +
> 
> Just one question on this one, what do you expect to happen, or how
> does this work when you use a different serial port for example on a
> board that has a real serial port and an FIQ port?   It was not
> obvious that there was a path to check if the port you are
> registering is an FIQ enabled port and then call the
> kgdb_register_nmi_console at that point.

Yes, it works perfectly fine. If the port is not FIQ-enabled (which,
technically, can be enabled later -- it's just not implemented), then
ttyNMI just won't able to receive data.

The reason why I register nmi console in KGDB code (as opposite to arch
code), is the dependcy: the port should be initialized before it can be
used as ttyNMI. We could place the ttyNMI registration code into
late_initcall in arch/, and check for dbg_io_ops, but that is ugly by
itself, plus dbg_io_ops can change.

But with your kgdb_arch->enable_nmi suggestion (which I'll surely
implement), we can skip the registration at least for the cases when
the arch doesn't have this feature.

Thanks!

Anton.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger
  2012-09-19 11:57           ` Jason Wessel
@ 2012-09-19 16:57             ` Anton Vorontsov
  0 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-19 16:57 UTC (permalink / raw)
  To: Jason Wessel
  Cc: Greg Kroah-Hartman, Andrew Morton, Russell King, Alan Cox,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

On Wed, Sep 19, 2012 at 06:57:24AM -0500, Jason Wessel wrote:
[..]
> Other than 1, 2 and 7.   They all get:
> 
> Acked-by: Jason Wessel <jason.wessel@windriver.com>
> 
> I ran out of time to review things yesterday, but it is all reviewed now.
> After we get some more comments from Anton, we can move forward.

Thanks for reviews, Jason!

I'll implement all your suggestions soonish.

Cheers,

Anton.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 01/11] kernel/debug: Mask KGDB NMI upon entry
  2012-09-24 21:26 [PATCH v9 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
@ 2012-09-24 21:27 ` Anton Vorontsov
  0 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-24 21:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Russell King
  Cc: Alan Cox, Jason Wessel, Arve Hjønnevåg, Colin Cross,
	Brian Swetland, John Stultz, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

The new arch callback should manage NMIs that usually cause KGDB to
enter. That is, not all NMIs should be enabled/disabled, but only
those that issue kgdb_handle_exception().

We must mask it as serial-line interrupt can be used as an NMI, so
if the original KGDB-entry cause was say a breakpoint, then every
input to KDB console will cause KGDB to reenter, which we don't want.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
Acked-by: Jason Wessel <jason.wessel@windriver.com>
---
 include/linux/kgdb.h      |  3 +++
 kernel/debug/debug_core.c | 14 +++++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index c4d2fc1..7800cce 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -240,6 +240,7 @@ extern void kgdb_arch_late(void);
  * hardware breakpoints.
  * @correct_hw_break: Allow an architecture to specify how to correct the
  * hardware debug registers.
+ * @enable_nmi: Manage NMI-triggered entry to KGDB
  */
 struct kgdb_arch {
 	unsigned char		gdb_bpt_instr[BREAK_INSTR_SIZE];
@@ -252,6 +253,8 @@ struct kgdb_arch {
 	void	(*disable_hw_break)(struct pt_regs *regs);
 	void	(*remove_all_hw_break)(void);
 	void	(*correct_hw_break)(void);
+
+	void	(*enable_nmi)(bool on);
 };
 
 /**
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 0557f24..17e073c 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -672,6 +672,10 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
 {
 	struct kgdb_state kgdb_var;
 	struct kgdb_state *ks = &kgdb_var;
+	int ret = 0;
+
+	if (arch_kgdb_ops.enable_nmi)
+		arch_kgdb_ops.enable_nmi(0);
 
 	ks->cpu			= raw_smp_processor_id();
 	ks->ex_vector		= evector;
@@ -681,11 +685,15 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
 	ks->linux_regs		= regs;
 
 	if (kgdb_reenter_check(ks))
-		return 0; /* Ouch, double exception ! */
+		goto out; /* Ouch, double exception ! */
 	if (kgdb_info[ks->cpu].enter_kgdb != 0)
-		return 0;
+		goto out;
 
-	return kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
+	ret = kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
+out:
+	if (arch_kgdb_ops.enable_nmi)
+		arch_kgdb_ops.enable_nmi(1);
+	return ret;
 }
 
 int kgdb_nmicallback(int cpu, void *regs)
-- 
1.7.12


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 01/11] kernel/debug: Mask KGDB NMI upon entry
  2012-09-19 23:40 [PATCH v8 " Anton Vorontsov
@ 2012-09-19 23:41 ` Anton Vorontsov
  0 siblings, 0 replies; 29+ messages in thread
From: Anton Vorontsov @ 2012-09-19 23:41 UTC (permalink / raw)
  To: Andrew Morton, Russell King, Jason Wessel, Greg Kroah-Hartman, Alan Cox
  Cc: Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport,
	linux-serial

The new arch callback should manage NMIs that usually cause KGDB to
enter. That is, not all NMIs should be enabled/disabled, but only
those that issue kgdb_handle_exception().

We must mask it as serial-line interrupt can be used as an NMI, so
if the original KGDB-entry cause was say a breakpoint, then every
input to KDB console will cause KGDB to reenter, which we don't want.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 include/linux/kgdb.h      |  3 +++
 kernel/debug/debug_core.c | 14 +++++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index c4d2fc1..7800cce 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -240,6 +240,7 @@ extern void kgdb_arch_late(void);
  * hardware breakpoints.
  * @correct_hw_break: Allow an architecture to specify how to correct the
  * hardware debug registers.
+ * @enable_nmi: Manage NMI-triggered entry to KGDB
  */
 struct kgdb_arch {
 	unsigned char		gdb_bpt_instr[BREAK_INSTR_SIZE];
@@ -252,6 +253,8 @@ struct kgdb_arch {
 	void	(*disable_hw_break)(struct pt_regs *regs);
 	void	(*remove_all_hw_break)(void);
 	void	(*correct_hw_break)(void);
+
+	void	(*enable_nmi)(bool on);
 };
 
 /**
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 0557f24..17e073c 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -672,6 +672,10 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
 {
 	struct kgdb_state kgdb_var;
 	struct kgdb_state *ks = &kgdb_var;
+	int ret = 0;
+
+	if (arch_kgdb_ops.enable_nmi)
+		arch_kgdb_ops.enable_nmi(0);
 
 	ks->cpu			= raw_smp_processor_id();
 	ks->ex_vector		= evector;
@@ -681,11 +685,15 @@ kgdb_handle_exception(int evector, int signo, int ecode, struct pt_regs *regs)
 	ks->linux_regs		= regs;
 
 	if (kgdb_reenter_check(ks))
-		return 0; /* Ouch, double exception ! */
+		goto out; /* Ouch, double exception ! */
 	if (kgdb_info[ks->cpu].enter_kgdb != 0)
-		return 0;
+		goto out;
 
-	return kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
+	ret = kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
+out:
+	if (arch_kgdb_ops.enable_nmi)
+		arch_kgdb_ops.enable_nmi(1);
+	return ret;
 }
 
 int kgdb_nmicallback(int cpu, void *regs)
-- 
1.7.11.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2012-09-24 21:31 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-13 15:01 [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
2012-09-13 15:03 ` [PATCH 01/11] kernel/debug: Mask KGDB NMI upon entry Anton Vorontsov
2012-09-19 11:52   ` Jason Wessel
2012-09-13 15:03 ` [PATCH 02/11] kdb: Implement disable_nmi command Anton Vorontsov
2012-09-19 11:52   ` Jason Wessel
2012-09-13 15:03 ` [PATCH 03/11] kdb: Turn KGDB_KDB=n stubs into static inlines Anton Vorontsov
2012-09-13 15:03 ` [PATCH 04/11] tty/serial/core: Introduce poll_init callback Anton Vorontsov
2012-09-13 15:03 ` [PATCH 05/11] tty/serial/amba-pl011: Implement " Anton Vorontsov
2012-09-13 15:03 ` [PATCH 06/11] tty/serial/amba-pl011: Quiesce interrupts in poll_get_char Anton Vorontsov
2012-09-13 15:03 ` [PATCH 07/11] tty/serial: Add kgdb_nmi driver Anton Vorontsov
2012-09-15 15:52   ` Francesco Lavra
2012-09-17  1:07     ` [PATCH updated " Anton Vorontsov
2012-09-19 11:52   ` [PATCH " Jason Wessel
2012-09-19 16:54     ` Anton Vorontsov
2012-09-13 15:03 ` [PATCH 08/11] ARM: Move some macros from entry-armv to entry-header Anton Vorontsov
2012-09-13 15:03 ` [PATCH 09/11] ARM: Add KGDB/KDB FIQ debugger generic code Anton Vorontsov
2012-09-13 15:03 ` [PATCH 10/11] ARM: VIC: Add a couple of low-level FIQ management helpers Anton Vorontsov
2012-09-13 15:03 ` [PATCH 11/11] ARM: versatile: Make able to use UART ports for KGDB FIQ debugger Anton Vorontsov
2012-09-13 17:04 ` [PATCH v7 0/11] KGDB/KDB FIQ (NMI) debugger Alan Cox
2012-09-13 16:55   ` Anton Vorontsov
2012-09-17 11:37 ` Greg Kroah-Hartman
2012-09-17 12:43   ` Jason Wessel
2012-09-17 12:54     ` Greg Kroah-Hartman
2012-09-17 15:39       ` Anton Vorontsov
2012-09-17 15:54         ` Greg Kroah-Hartman
2012-09-19 11:57           ` Jason Wessel
2012-09-19 16:57             ` Anton Vorontsov
2012-09-19 23:40 [PATCH v8 " Anton Vorontsov
2012-09-19 23:41 ` [PATCH 01/11] kernel/debug: Mask KGDB NMI upon entry Anton Vorontsov
2012-09-24 21:26 [PATCH v9 0/11] KGDB/KDB FIQ (NMI) debugger Anton Vorontsov
2012-09-24 21:27 ` [PATCH 01/11] kernel/debug: Mask KGDB NMI upon entry Anton Vorontsov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).