KGDB/KDB FIQ (NMI) debugger

Hi all,

Here is a new revision, mostly tty reworks. The new tty_port stuff is a
bliss: no more per-driver mutex, no more counting for open(), well-separated initialization callbacks (I hope I got them right :-).

But since I now use a lot of new tty_port stuff, I had to rebase the patch set on top of tty-next, so there's no point in cherry-picking anymore.

So, in v6:

- Converted the NMI tty driver to use tty_port helpers, per Alan Cox's suggestions;

- In uart's poll_init callback fixed a race, spotted by Alan;

- Use test_bit instead of touching port->flags directly;

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: 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     |  66 ++++-
 drivers/tty/serial/kgdb_nmi.c       | 391 ++++++++++++++++++++++++++++++
 drivers/tty/serial/kgdboc.c         |  16 ++
 drivers/tty/serial/serial_core.c    |  32 +++
 include/linux/kdb.h                 |  29 ++-
 include/linux/kgdb.h                |  34 +++
 include/linux/serial_core.h         |   2 +
 include/linux/tty_driver.h          |   1 +
 kernel/debug/debug_core.c           |  36 ++-
 kernel/debug/kdb/kdb_main.c         |  29 +++
 23 files changed, 1076 insertions(+), 193 deletions(-)

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.

---
 include/linux/kgdb.h      | 23 +++++++++++++++++++++++
 kernel/debug/debug_core.c | 36 +++++++++++++++++++++++++++++++++---
 2 files changed, 56 insertions(+), 3 deletions(-)

index c4d2fc1..3b111a6 100644
@@ -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.

index 0557f24..b621d1e 100644
@@ -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

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

---
 kernel/debug/kdb/kdb_main.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

index 31df170..9fadff1 100644
@@ -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

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.

---
 include/linux/kdb.h | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

index 42d9e86..7f6fe6e 100644
@@ -13,6 +13,14 @@
  * Copyright (C) 2009 Jason Wessel <[email protected]>
  */
 
+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

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.

---
 drivers/tty/serial/serial_core.c | 17 +++++++++++++++++
 include/linux/serial_core.h      |  1 +
 2 files changed, 18 insertions(+)

index 046279c..dcb2d5a 100644
@@ -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);

index 7cf0b68..822c887 100644
@@ -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

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)

---
 drivers/tty/serial/amba-pl011.c | 44 ++++++++++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 11 deletions(-)

index cede938..45137e4 100644
@@ -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

This patch implements a new callback: clear_irqs. It is used for the cases when KDB-entry (e.g. NMI) and KDB IO (e.g. serial port) shares the same interrupt. To get the idea, let's take some real example (ARM machine): we have a serial port which interrupt is routed to an NMI, and the interrupt is used to enter KDB. Once there is some activity on the serial port, the CPU receives NMI exception, and we fall into KDB shell. So, it is our "debug console", and it is able to interrupt (and thus debug) even IRQ handlers themselves.

When used that way, the interrupt never reaches serial driver's IRQ handler routine, which means that serial driver will not silence the interrupt. NMIs behaviour are quite arch-specific, and we can't assume that we can use them as ordinary IRQs, e.g. on some arches (like ARM) we can't handle data aborts, the behaviour is undefined then. So we can't just handle execution to serial driver's IRQ handler from the NMI context once we're done with KDB (plus this would defeat the debugger's
purpose: we want the NMI handler be as simple as possible, so it will have less chances to hang).

So, given that have to deal with it somehow, we have two options:

1. Implement something that clears the interrupt; 2. Implement a whole new concept of grabbing tty for exclusive KDB use, plus implement mask/unmask callbacks, i.e.:
- Since consoles might use ttys w/o opending them, we would have to make kdb respect CON_ENABLED flag (maybe a good idea to do it anyway);
- Add 'bool exclusive' argument to tty_find_polling_driver(), if set to 1, the function will refuse to return an already opened tty; and will use the flag in tty_reopen() to not allow multiple users (there are already checks for pty masters, which are "open once" ttys);
- Once we got the tty exclusively, we would need to call some new uart->mask_all_but_rx_interrupts call before we want to use the port for NMI/KDB, and unmask_all_but_rx_interrupts after we're done with it.

The second option is obviously more complex, needlessly so, and less generic. So I went with the first one: we just consume all the interrupts. The tty becomes silently unusable for the rest of the world when we use it with KDB; but once we reroute the serial IRQ source back from NMI to an ordinary IRQ (in KDB this can be done with 'disable_nmi' command), it will behave as normal.

p.s. Since the callback is so far used only by polling users, we place it under the appropriate #ifdef.

---
 drivers/tty/serial/kgdboc.c      | 10 ++++++++++
 drivers/tty/serial/serial_core.c | 15 +++++++++++++++
 include/linux/kgdb.h             |  1 +
 include/linux/serial_core.h      |  1 +
 include/linux/tty_driver.h       |  1 +
 5 files changed, 28 insertions(+)

index 2b42a01..0aa08c8 100644
@@ -227,6 +227,15 @@ static int kgdboc_get_char(void)
 						kgdb_tty_line);
 }
 
+static void kgdboc_clear_irqs(void)
+{
+	if (!kgdb_tty_driver)
+		return;
+	if (kgdb_tty_driver->ops->clear_irqs)
+		kgdb_tty_driver->ops->clear_irqs(kgdb_tty_driver,
+						 kgdb_tty_line);
+}
+
 static void kgdboc_put_char(u8 chr)
 {
 	if (!kgdb_tty_driver)

@@ -298,6 +307,7 @@ static struct kgdb_io kgdboc_io_ops = {
 	.name			= "kgdboc",
 	.read_char		= kgdboc_get_char,
 	.write_char		= kgdboc_put_char,
+	.clear_irqs		= kgdboc_clear_irqs,
 	.pre_exception		= kgdboc_pre_exp_handler,
 	.post_exception		= kgdboc_post_exp_handler,
 };

index dcb2d5a..93c36cb 100644
@@ -2187,6 +2187,20 @@ static void uart_poll_put_char(struct tty_driver *driver, int line, char ch)
 	port = state->uart_port;
 	port->ops->poll_put_char(port, ch);
 }
+
+static void uart_clear_irqs(struct tty_driver *driver, int line)
+{
+	struct uart_driver *drv = driver->driver_state;
+	struct uart_state *state = drv->state + line;
+	struct uart_port *port;
+
+	if (!state || !state->uart_port)
+		return;
+
+	port = state->uart_port;
+	if (port->ops->clear_irqs)
+		port->ops->clear_irqs(port);
+}
 #endif
 
 static const struct tty_operations uart_ops = {

@@ -2219,6 +2233,7 @@ static const struct tty_operations uart_ops = {
 	.poll_init	= uart_poll_init,
 	.poll_get_char	= uart_poll_get_char,
 	.poll_put_char	= uart_poll_put_char,
+	.clear_irqs	= uart_clear_irqs,
 #endif
 };
 

index 3b111a6..1fd1cf0 100644
@@ -295,6 +295,7 @@ struct kgdb_io {
 	const char		*name;
 	int			(*read_char) (void);
 	void			(*write_char) (u8);
+	void			(*clear_irqs) (void);
 	void			(*flush) (void);
 	int			(*init) (void);
 	void			(*pre_exception) (void);

index 822c887..855fb6e 100644
@@ -277,6 +277,7 @@ struct uart_ops {
 	int		(*poll_init)(struct uart_port *);
 	void	(*poll_put_char)(struct uart_port *, unsigned char);
 	int		(*poll_get_char)(struct uart_port *);
+	void	(*clear_irqs)(struct uart_port *);
 #endif
 };
 

index dd976cf..42f8a87 100644
@@ -282,6 +282,7 @@ struct tty_operations {
 	int (*poll_init)(struct tty_driver *driver, int line, char *options);
 	int (*poll_get_char)(struct tty_driver *driver, int line);
 	void (*poll_put_char)(struct tty_driver *driver, int line, char ch);
+	void (*clear_irqs)(struct tty_driver *driver, int line);
 #endif
 	const struct file_operations *proc_fops;
 };

1.7.11.5

This bit I still really don't like. I would like to know what the generic IRQ folks thing about it and if Thomas Gleixner has any brilliant ideas here. I don't think its a show stopper it would just be nice if there was a better solution first.

Alan

Yup, good idea, Cc'ing.

Hello Thomas,

We're dissussing a patch that adds a clear_irq callback into UART drivers. For convenience, the particular patch is inlined at the end of this email. The rationale and the background for the whole thing can be found here: http://lkml.org/lkml/2012/9/10/2

So, just for visual clearness, and for the fun of it, here is some glorious ascii art of what we have:

                         ,---NMI-|`````|
UART_IRQ---INT_controller        | CPU |
                         `---IRQ-|,,,,,|

Pretty much standard scheme. That is, on the interrupt controller level we can reroute any IRQ to NMI, and back in 2008 folks at Google found that rerouting the UART IRQ to NMI brings some really cool features: we can have a very reliable and powerful debugger pretty much on every embedded machine, and without loosing the UART/console port itself.

So, it works like this:

- At boot time, Linux arch/ code reroutes UART IRQ to NMI, we connect the port to the KGDBoC, and so forth...;
- User starts typing on the serial port;
- UART raises its IRQ line;
- Through the controller, one of CPUs gets an NMI;
- In NMI context, CPU reads a character from UART;
- Then it checks if the character resulted into the special 'enter KGDB' magic sequence:
- If yes, then the CPU invites other CPUs into the KGDB, and thus kernel enters the debugger;
- If the character wasn't part of the magic command (or the sequence is yet incomplete), then CPU exits NMI and continues as normal.

The "problem" is in the last step. If we exit NMI without making UART know that we're done with the interrupt, we will reenter the NMI immediately, even without any new characters from the UART.

The obvious solution would be to "mask/reroute NMI at INT_controller level or queue serial port's IRQ routine from somewhere, e.g. a tasklet or software raised IRQ". Unfortunately, this means that we have to keep NMI disabled for this long:

1. We exit the NMI context with NMI source disabled/rerouted;
2. CPU continues to execute the kernel;
3. Kernel receives a timer interrupt, or software-raised interrupt, or UART IRQ, which was temporary rerouted back to normal interrupts;
4. It executes normal IRQ-entry, tracing, lots of other stuff, interrupt handlers, softirq handlers, and thus we clear the UART interrupt;
5. Once the UART is cleared, we reenable NMI (in the arch-code, we can do that in our do_IRQ());

As you can see, with this solution the NMI debugger will be effectively disabled from 1. to 5., thus shall the hang happen in this sensitive code, we would no longer able to debug it.

And this defeats the main purpose of the NMI debugger: we must keep NMI enabled all the time when we're not in the debugger, the NMI debugger is always available (unless the debugger crashed :-)

That's why I came with the clear_irq callback in the serial drivers that we call from the NMI context, it's much simpler and keeps the debugger robust. So, personally I too can't think of any other plausible solution that would keep all the features intact.

Thanks,

Anton.

- - - -
[PATCH] tty/serial/kgdboc: Add and wire up clear_irqs callback

This patch implements a new callback: clear_irqs. It is used for the cases when KDB-entry (e.g. NMI) and KDB IO (e.g. serial port) shares the same interrupt. To get the idea, let's take some real example (ARM machine): we have a serial port which interrupt is routed to an NMI, and the interrupt is used to enter KDB. Once there is some activity on the serial port, the CPU receives NMI exception, and we fall into KDB shell. So, it is our "debug console", and it is able to interrupt (and thus debug) even IRQ handlers themselves.

When used that way, the interrupt never reaches serial driver's IRQ handler routine, which means that serial driver will not silence the interrupt. NMIs behaviour are quite arch-specific, and we can't assume that we can use them as ordinary IRQs, e.g. on some arches (like ARM) we can't handle data aborts, the behaviour is undefined then. So we can't just handle execution to serial driver's IRQ handler from the NMI context once we're done with KDB (plus this would defeat the debugger's
purpose: we want the NMI handler be as simple as possible, so it will have less chances to hang).

So, given that have to deal with it somehow, we have two options:

1. Implement something that clears the interrupt; 2. Implement a whole new concept of grabbing tty for exclusive KDB use, plus implement mask/unmask callbacks, i.e.:
- Since consoles might use ttys w/o opending them, we would have to make kdb respect CON_ENABLED flag (maybe a good idea to do it anyway);
- Add 'bool exclusive' argument to tty_find_polling_driver(), if set to 1, the function will refuse to return an already opened tty; and will use the flag in tty_reopen() to not allow multiple users (there are already checks for pty masters, which are "open once" ttys);
- Once we got the tty exclusively, we would need to call some new uart->mask_all_but_rx_interrupts call before we want to use the port for NMI/KDB, and unmask_all_but_rx_interrupts after we're done with it.

The second option is obviously more complex, needlessly so, and less generic. So I went with the first one: we just consume all the interrupts. The tty becomes silently unusable for the rest of the world when we use it with KDB; but once we reroute the serial IRQ source back from NMI to an ordinary IRQ (in KDB this can be done with 'disable_nmi' command), it will behave as normal.

p.s. Since the callback is so far used only by polling users, we place it under the appropriate #ifdef.

---
 drivers/tty/serial/kgdboc.c      | 10 ++++++++++
 drivers/tty/serial/serial_core.c | 15 +++++++++++++++
 include/linux/kgdb.h             |  1 +
 include/linux/serial_core.h      |  1 +
 include/linux/tty_driver.h       |  1 +
 5 files changed, 28 insertions(+)

index 2b42a01..0aa08c8 100644
@@ -227,6 +227,15 @@ static int kgdboc_get_char(void)
 						kgdb_tty_line);
 }
 
+static void kgdboc_clear_irqs(void)
+{
+	if (!kgdb_tty_driver)
+		return;
+	if (kgdb_tty_driver->ops->clear_irqs)
+		kgdb_tty_driver->ops->clear_irqs(kgdb_tty_driver,
+						 kgdb_tty_line);
+}
+
 static void kgdboc_put_char(u8 chr)
 {
 	if (!kgdb_tty_driver)

@@ -298,6 +307,7 @@ static struct kgdb_io kgdboc_io_ops = {
 	.name			= "kgdboc",
 	.read_char		= kgdboc_get_char,
 	.write_char		= kgdboc_put_char,
+	.clear_irqs		= kgdboc_clear_irqs,
 	.pre_exception		= kgdboc_pre_exp_handler,
 	.post_exception		= kgdboc_post_exp_handler,
 };

index dcb2d5a..93c36cb 100644
@@ -2187,6 +2187,20 @@ static void uart_poll_put_char(struct tty_driver *driver, int line, char ch)
 	port = state->uart_port;
 	port->ops->poll_put_char(port, ch);
 }
+
+static void uart_clear_irqs(struct tty_driver *driver, int line)
+{
+	struct uart_driver *drv = driver->driver_state;
+	struct uart_state *state = drv->state + line;
+	struct uart_port *port;
+
+	if (!state || !state->uart_port)
+		return;
+
+	port = state->uart_port;
+	if (port->ops->clear_irqs)
+		port->ops->clear_irqs(port);
+}
 #endif
 
 static const struct tty_operations uart_ops = {

@@ -2219,6 +2233,7 @@ static const struct tty_operations uart_ops = {
 	.poll_init	= uart_poll_init,
 	.poll_get_char	= uart_poll_get_char,
 	.poll_put_char	= uart_poll_put_char,
+	.clear_irqs	= uart_clear_irqs,
 #endif
 };
 

index 3b111a6..1fd1cf0 100644
@@ -295,6 +295,7 @@ struct kgdb_io {
 	const char		*name;
 	int			(*read_char) (void);
 	void			(*write_char) (u8);
+	void			(*clear_irqs) (void);
 	void			(*flush) (void);
 	int			(*init) (void);
 	void			(*pre_exception) (void);

index 822c887..855fb6e 100644
@@ -277,6 +277,7 @@ struct uart_ops {
 	int		(*poll_init)(struct uart_port *);
 	void	(*poll_put_char)(struct uart_port *, unsigned char);
 	int		(*poll_get_char)(struct uart_port *);
+	void	(*clear_irqs)(struct uart_port *);
 #endif
 };
 

index dd976cf..42f8a87 100644
@@ -282,6 +282,7 @@ struct tty_operations {
 	int (*poll_init)(struct tty_driver *driver, int line, char *options);
 	int (*poll_get_char)(struct tty_driver *driver, int line);
 	void (*poll_put_char)(struct tty_driver *driver, int line, char ch);
+	void (*clear_irqs)(struct tty_driver *driver, int line);
 #endif
 	const struct file_operations *proc_fops;
 };

1.7.11.5

The UART irq line should go low when you read the character out of the receive buffer, or the polling rx function should clear the interrupt for you. If you use a clear_irqs callback, you can drop characters if one arrives between the last character buffer read and calling clear_irqs.

On Tue, Sep 11, 2012 at 08:42:46PM -0700, Colin Cross wrote:
[...]

Probably some controllers may lower the line by themselves, but not all, and probably most of them need an explicit clear.

Yes, that's an option. But that way we add a new semantic for the polling routines, and effecitvely we just merge the two callbacks.

Of course, if Alan is OK with this, I'm more than OK too. :-)

(But the polling routines would need to clear all interrupts, not just rx/tx. For example, if the controller indicated some error, and nobody clears it, then we'll start reentering infinitely.)

Only if we call clear_irqs() after reading the characters, but we do it before. So if new characters are available, we will reenter NMI, which is OK.

But if used incorrectly, it truly can cause dropping (or staling) of characters, so I'd better add some comments about this.

Thanks!

Anton.

Anything 8250-based will clear the interrupt automatically, assuming you read the status registers as well as the character register.

For exynos5, the only non-8250 based serial port I've come across, we clear all interrupts in the rx poll function (see
https://android.googlesource.com/kernel/exynos/+/ef427aafffb7153dde59745e440fd7ec41ea969d/arch/arm/mach-exynos/exynos_fiq_debugger.c).

What does clear_irqs() mean for a status or tx interrupt? The tx interrupt will generally re-assert as long as the tx fifo is empty, which would require disabling it. On 8250 ports, status interrupts will re-assert until the corresponding status register is read.

On Tue, Sep 11, 2012 at 09:40:35PM -0700, Colin Cross wrote:
[..]

Yes, but if you'd like to merge your code, some might ask you: why?

You'd answer that you need to clear the interrupts, otherwise you'll keep reentering NMI. The next that you might get is this: "this does not belong to the getc callback, it's better to factor it out". :-) And here comes clear_irq() (or alike, see below).

Yup, and that's exactly what we do: http://lkml.org/lkml/2012/9/11/119

Your words made me think that clear_irq() might be indeed a somewhat inappropriate name. We have to be even stricter on its definition and behaviour.

So, returning to your question "What does clear_irqs() mean", I'd answer that: the function must do whatever needed to lower the IRQ line, plus the function must leave the port in the state that it's still able to throw RX interrupts after the call.

So, the 100% proper name for this function would be this:

quiesce_irqs_but_rx()

It's a bit long, but does exactly what the name states.

Thanks!
Anton.

It may well be better.

For a lot of devices and platforms you'd probably mask them instead ?

Recursively or not... again you get platform specific magic in places we don't want.

In the driver poll method for most uarts is going to be at least buried in what is usually arch specific uarts.

If there is no way to clear them, yes, we obviously would want to mask them before using the port for NMI debugger. Then we'd need three callbacks:

- mask_all_irqs_but_rx() -- used before we want to start using the port for the NMI debugger;
- clear_rx_irq() -- (optional) clears rx IRQ for controllers that need it;
- restore_irqs() -- unmasks interrupts that were previously masked.

If we ever encounter a case when just clearing interrupts doesn't work, we can surely implement the above scheme... It's just so far I don't see any need to over-design this, but again, it's your call, I told my opinion on this, but I'll do whatever you guys like more. :-)

I really really don't see how this is platform-specific. All we ask the serial driver is to quiesce its interrupt. Whether we can handle NMIs/IRQs recursively or not is not serial driver's worry, since its IRQ handler is not going to fire anyway. The polling routines already gave us the power to steal/inject the data, so now we're stealing the interrupt too.

How we use the callback is indeed platform-specific, but so is the whole KGDB, and that knowledge is hidden there.

For serial driver it's all pretty much clear: lower the interrupt, but don't turn off rx detection.

Thanks!

Anton.

It's all pretty straightforward, except for TXIM interrupt. The interrupt has meaning "ready to transmit", 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.

---
 drivers/tty/serial/amba-pl011.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

index 45137e4..ec15312 100644
@@ -1307,6 +1307,27 @@ static void pl011_put_poll_char(struct uart_port *port,
 	writew(ch, uap->port.membase + UART01x_DR);
 }
 
+static void pl011_clear_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
+	 * clear_irq() caller 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 calls clear_irqs() assumes that the caller 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);
+}
 #endif /* CONFIG_CONSOLE_POLL */
 
 static int pl011_hwinit(struct uart_port *port)

@@ -1712,6 +1733,7 @@ static struct uart_ops amba_pl011_pops = {
 	.poll_init     = pl011_hwinit,
 	.poll_get_char = pl011_get_poll_char,
 	.poll_put_char = pl011_put_poll_char,
+	.clear_irqs    = pl011_clear_irqs,
 #endif
 };
 

1.7.11.5

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.

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

index 26907cf..b22e45b 100644
@@ -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

index ce88667..4f694da 100644
@@ -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

index 0aa08c8..bb69803 100644
@@ -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;

index 1fd1cf0..fb7c55c 100644
@@ -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

Yup, a leftover, with tty_port no longer needed. Thanks for noticing.

Will add. Thanks a lot!

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.

---
 arch/arm/kernel/entry-armv.S   | 167 +---------------------------------------
 arch/arm/kernel/entry-header.S | 170 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 171 insertions(+), 166 deletions(-)

index 0f82098..0f15368 100644
@@ -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:
 /*

1.7.11.5

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.

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

index 6d6e18f..c978c74 100644
@@ -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
 

index 48066ce..807e547 100644
@@ -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__ */
 
 /*

index 7ad2d5c..5aa079b 100644
@@ -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

1.7.11.5

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

---
 arch/arm/common/vic.c               | 28 ++++++++++++++++++++++++++++
 arch/arm/include/asm/hardware/vic.h |  2 ++
 2 files changed, 30 insertions(+)

index e0d5388..df2fc82 100644
@@ -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.

index e14af1a..2728975 100644
@@ -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

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.

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

index c978c74..1a9881a 100644
@@ -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.
 

index 81fa3fe..bfd761f 100644
@@ -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

new file mode 100644
index 0000000..3cdf71d
@@ -0,0 +1,31 @@
+/*
+ * KGDB FIQ board support
+ *
+ * Copyright 2012 Linaro Ltd.
+ *		  Anton Vorontsov <[email protected]>
+ *
+ * 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