KGDB/KDB FIQ (NMI) debugger

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 <[email protected]>

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

include/linux/kgdb.h +0 -0

index c4d2fc1..3b111a6 100644
@@ -221,6 +221,29 @@ extern int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt);
  */

include/linux/kgdb.h +5 -0

+
+/**
+ *	kgdb_enable_nmi - Enable or disable KGDB-entry NMI
+ *	@on: Flag to either enable or disable an NMI
+ *

kernel/debug/debug_core.c +24 -0

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

kernel/debug/debug_core.c +0 -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))

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 <[email protected]>

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

kernel/debug/kdb/kdb_main.c +27 -0

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

kernel/debug/kdb/kdb_main.c +0 -0

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

drivers/tty/serial/kgdb_nmi.c new file +8 -0

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

drivers/tty/serial/kgdboc.c +2 -0

index 2b42a01..ed97cfd 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);

Review

Quoted 1 line

(quoted diff) +1 -0

+		pr_err("%s: can't nstall tty port: %d\n", __func__, ret);

s/nstall/install

Review

Quoted 3 lines

(quoted diff) +3 -0

+	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

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 <[email protected]>

Signed-off-by Anton Vorontsov <[email protected]>

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

drivers/tty/serial/Kconfig +19 -0

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

drivers/tty/serial/Makefile +1 -0

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

drivers/tty/serial/kgdb_nmi.c new file +396 -0

new file mode 100644
index 0000000..57bf744
@@ -0,0 +1,396 @@
+/*
+ * KGDB NMI serial console
+ *
+ * Copyright 2010 Google, Inc.
+ *		  Arve Hj?nnev?g <[email protected]>
+ *		  Colin Cross <[email protected]>
+ * 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/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);

drivers/tty/serial/kgdboc.c +2 -0

index 2b42a01..ed97cfd 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);

Quoted 15 lines

drivers/tty/serial/kgdboc.c +2 -0

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

drivers/tty/serial/kgdboc.c do_register: · line 206

Quoted 4 lines

drivers/tty/serial/kgdboc.c +4 -0

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

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.

Quoted 9 lines · On Thu, 13 Sep 2012 08:01:33 -0700 Anton Vorontsov…

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 <[email protected]>

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

Quoted 10 lines · Anton Vorontsov

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.

Quoted 8 lines

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?

Quoted 24 lines · Greg Kroah-Hartman

Quoted 10 lines · Anton Vorontsov

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.

Quoted 8 lines

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.

Quoted 2 lines

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.

Quoted 8 lines

Quoted 2 lines

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 <[email protected]>

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.

On Mon, Sep 17, 2012 at 05:54:27AM -0700, Greg Kroah-Hartman wrote:
[...]

Quoted 9 lines

Quoted 8 lines

Quoted 2 lines

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.

Quoted 6 lines

Your tree is fine:

Acked-by Greg Kroah-Hartman <[email protected]>

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.

Quoted 17 lines · Anton Vorontsov

On Mon, Sep 17, 2012 at 05:54:27AM -0700, Greg Kroah-Hartman wrote:
[...]

Quoted 7 lines

Quoted 7 lines

Quoted 2 lines

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.

Quoted 6 lines

Your tree is fine:

Acked-by Greg Kroah-Hartman <[email protected]>

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!

Quoted 6 lines

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 <[email protected]>

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.