Re: [PATCH 3/7] kgdb: Add request_nmi() to the io ops table for kgdboc
From: Daniel Thompson <hidden>
Date: 2020-06-22 16:03:09
Also in:
lkml
On Mon, Jun 22, 2020 at 07:56:20PM +0530, Sumit Garg wrote:
quoted hunk ↗ jump to hunk
From: Daniel Thompson <redacted> Add request_nmi() callback to install a non-maskable interrupt handler corresponding to IRQ retrieved from polling interface. If NMI handler installation fails due to missing support from underlying irqchip driver then fallback to install it as normal interrupt handler. Signed-off-by: Daniel Thompson <redacted> Co-developed-by: Sumit Garg <redacted> Signed-off-by: Sumit Garg <redacted> --- drivers/tty/serial/kgdboc.c | 35 +++++++++++++++++++++++++++++++++++ include/linux/kgdb.h | 7 +++++++ 2 files changed, 42 insertions(+)diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c index 84ffede..263afae 100644 --- a/drivers/tty/serial/kgdboc.c +++ b/drivers/tty/serial/kgdboc.c@@ -19,6 +19,9 @@ #include <linux/console.h> #include <linux/vt_kern.h> #include <linux/input.h> +#include <linux/interrupt.h> +#include <linux/irq.h> +#include <linux/irqdesc.h> #include <linux/module.h> #include <linux/platform_device.h> #include <linux/serial_core.h>@@ -390,12 +393,44 @@ static void kgdboc_post_exp_handler(void) kgdboc_restore_input(); } +static int kgdb_tty_irq; + +static int kgdboc_request_nmi(irq_handler_t fn, void *dev_id) +{ + int irq, res; + + /* Better to avoid double allocation in the tty driver! */ + if (kgdb_tty_irq) + return 0; + + if (!kgdb_tty_driver->ops->poll_get_irq) + return -ENODEV; + + irq = + kgdb_tty_driver->ops->poll_get_irq(kgdb_tty_driver, kgdb_tty_line); + if (irq <= 0) + return irq ? irq : -ENODEV; + + irq_set_status_flags(irq, IRQ_NOAUTOEN); + res = request_nmi(irq, fn, IRQF_PERCPU, "kgdboc", dev_id);
Why do we need IRQF_PERCPU here. A UART interrupt is not normally per-cpu?
+ if (res) {
+ res = request_irq(irq, fn, IRQF_SHARED, "kgdboc", dev_id);IRQF_SHARED? Currrently there is nothing that prevents concurrent activation of ttyNMI0 and the underlying serial driver. Using IRQF_SHARED means it becomes possible for both drivers to try to service the same interrupt. That risks some rather "interesting" problems. Daniel.
quoted hunk ↗ jump to hunk
+ WARN_ON(res); + } + + enable_irq(irq); + + kgdb_tty_irq = irq; + return 0; +} + static struct kgdb_io kgdboc_io_ops = { .name = "kgdboc", .read_char = kgdboc_get_char, .write_char = kgdboc_put_char, .pre_exception = kgdboc_pre_exp_handler, .post_exception = kgdboc_post_exp_handler, + .request_nmi = kgdboc_request_nmi, }; #if IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE)diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index 529116b..b32b044 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h@@ -16,6 +16,7 @@ #include <linux/linkage.h> #include <linux/init.h> #include <linux/atomic.h> +#include <linux/interrupt.h> #ifdef CONFIG_HAVE_ARCH_KGDB #include <asm/kgdb.h> #endif@@ -276,6 +277,10 @@ struct kgdb_arch { * the I/O driver. * @post_exception: Pointer to a function that will do any cleanup work * for the I/O driver. + * @request_nmi: Pointer to a function that can install an non-maskable + * interrupt handler that will be called when a character is pending and that + * can be cleared by calling @read_char until it returns NO_POLL_CHAR. If NMI + * installation fails then fallback to install normal interrupt handler. * @cons: valid if the I/O device is a console; else NULL. */ struct kgdb_io {@@ -287,6 +292,8 @@ struct kgdb_io { void (*deinit) (void); void (*pre_exception) (void); void (*post_exception) (void); + int (*request_nmi)(irq_handler_t nmi_handler, + void *dev_id); struct console *cons; };-- 2.7.4