[4/7] serial: kgdb_nmi: Add support for interrupt based fallback

Message ID 1592835984-28613-5-git-send-email-sumit.garg@linaro.org
State New
Headers show
Series
  • Enable support for kgdb NMI console feature
Related show

Commit Message

Sumit Garg June 22, 2020, 2:26 p.m.
From: Daniel Thompson <daniel.thompson@linaro.org>


Add a generic NMI fallback to support kgdb NMI console feature which can
be overridden by arch specific implementation.

This common fallback mechanism utilizes kgdb IO based interrupt in order
to support entry into kgdb if a user types in kgdb_nmi_magic sequence. So
during NMI console init, NMI handler is installed corresponding to kgdb
IO based NMI which is invoked when a character is pending and that can be
cleared by calling @read_char until it returns NO_POLL_CHAR.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>

Co-developed-by: Sumit Garg <sumit.garg@linaro.org>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>

---
 drivers/tty/serial/kgdb_nmi.c | 47 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 40 insertions(+), 7 deletions(-)

-- 
2.7.4

Comments

Daniel Thompson June 22, 2020, 4:36 p.m. | #1
On Mon, Jun 22, 2020 at 07:56:21PM +0530, Sumit Garg wrote:
> From: Daniel Thompson <daniel.thompson@linaro.org>

> 

> Add a generic NMI fallback to support kgdb NMI console feature which can

> be overridden by arch specific implementation.


arch_kgdb_ops.enable_nmi should probably be killed off. Given we now
have request_nmi() I'm dubious there are any good reasons to use this
API.


Daniel.


> This common fallback mechanism utilizes kgdb IO based interrupt in order

> to support entry into kgdb if a user types in kgdb_nmi_magic sequence. So

> during NMI console init, NMI handler is installed corresponding to kgdb

> IO based NMI which is invoked when a character is pending and that can be

> cleared by calling @read_char until it returns NO_POLL_CHAR.

> 

> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>

> Co-developed-by: Sumit Garg <sumit.garg@linaro.org>

> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>

> ---

>  drivers/tty/serial/kgdb_nmi.c | 47 ++++++++++++++++++++++++++++++++++++-------

>  1 file changed, 40 insertions(+), 7 deletions(-)

> 

> diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c

> index b32c6b1..2580f39 100644

> --- a/drivers/tty/serial/kgdb_nmi.c

> +++ b/drivers/tty/serial/kgdb_nmi.c

> @@ -42,9 +42,46 @@ MODULE_PARM_DESC(magic, "magic sequence to enter NMI debugger (default $3#33)");

>  static atomic_t kgdb_nmi_num_readers = ATOMIC_INIT(0);

>  static struct console *orig_dbg_cons;

>  

> +static int kgdb_nmi_poll_one_knock(void);

> +

> +static irqreturn_t kgdb_handle_nmi(int irq, void *dev_id)

> +{

> +	int ret;

> +

> +	if (kgdb_nmi_knock < 0) {

> +		kgdb_breakpoint();

> +		return IRQ_HANDLED;

> +	}

> +

> +	ret = kgdb_nmi_poll_one_knock();

> +	if (ret == NO_POLL_CHAR)

> +		return IRQ_NONE;

> +

> +	while (ret != 1) {

> +		ret = kgdb_nmi_poll_one_knock();

> +		if (ret == NO_POLL_CHAR)

> +			return IRQ_HANDLED;

> +	}

> +

> +	kgdb_breakpoint();

> +	return IRQ_HANDLED;

> +}

> +

>  static int kgdb_nmi_console_setup(struct console *co, char *options)

>  {

> -	arch_kgdb_ops.enable_nmi(1);

> +	int res;

> +

> +	if (arch_kgdb_ops.enable_nmi) {

> +		arch_kgdb_ops.enable_nmi(1);

> +	} else if (dbg_io_ops->request_nmi) {

> +		res = dbg_io_ops->request_nmi(kgdb_handle_nmi, co);

> +		if (res) {

> +			pr_err("ttyNMI0: Cannot request nmi/irq\n");

> +			return res;

> +		}

> +	} else {

> +		return -ENODEV;

> +	}

>  

>  	/* The NMI console uses the dbg_io_ops to issue console messages. To

>  	 * avoid duplicate messages during kdb sessions we must inform kdb's

> @@ -328,9 +365,6 @@ int kgdb_register_nmi_console(void)

>  {

>  	int ret;

>  

> -	if (!arch_kgdb_ops.enable_nmi)

> -		return 0;

> -

>  	kgdb_nmi_tty_driver = alloc_tty_driver(1);

>  	if (!kgdb_nmi_tty_driver) {

>  		pr_err("%s: cannot allocate tty\n", __func__);

> @@ -380,9 +414,8 @@ int kgdb_unregister_nmi_console(void)

>  {

>  	int ret;

>  

> -	if (!arch_kgdb_ops.enable_nmi)

> -		return 0;

> -	arch_kgdb_ops.enable_nmi(0);

> +	if (arch_kgdb_ops.enable_nmi)

> +		arch_kgdb_ops.enable_nmi(0);

>  

>  	ret = unregister_console(&kgdb_nmi_console);

>  	if (ret)

> -- 

> 2.7.4

>
Sumit Garg June 23, 2020, 9:59 a.m. | #2
On Mon, 22 Jun 2020 at 22:06, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>

> On Mon, Jun 22, 2020 at 07:56:21PM +0530, Sumit Garg wrote:

> > From: Daniel Thompson <daniel.thompson@linaro.org>

> >

> > Add a generic NMI fallback to support kgdb NMI console feature which can

> > be overridden by arch specific implementation.

>

> arch_kgdb_ops.enable_nmi should probably be killed off. Given we now

> have request_nmi() I'm dubious there are any good reasons to use this

> API.

>


Sounds reasonable. Along with this, kgdb_nmi_poll_knock() seems to be
unused and can be removed as well.

-Sumit

>

> Daniel.

>

>

> > This common fallback mechanism utilizes kgdb IO based interrupt in order

> > to support entry into kgdb if a user types in kgdb_nmi_magic sequence. So

> > during NMI console init, NMI handler is installed corresponding to kgdb

> > IO based NMI which is invoked when a character is pending and that can be

> > cleared by calling @read_char until it returns NO_POLL_CHAR.

> >

> > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>

> > Co-developed-by: Sumit Garg <sumit.garg@linaro.org>

> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>

> > ---

> >  drivers/tty/serial/kgdb_nmi.c | 47 ++++++++++++++++++++++++++++++++++++-------

> >  1 file changed, 40 insertions(+), 7 deletions(-)

> >

> > diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c

> > index b32c6b1..2580f39 100644

> > --- a/drivers/tty/serial/kgdb_nmi.c

> > +++ b/drivers/tty/serial/kgdb_nmi.c

> > @@ -42,9 +42,46 @@ MODULE_PARM_DESC(magic, "magic sequence to enter NMI debugger (default $3#33)");

> >  static atomic_t kgdb_nmi_num_readers = ATOMIC_INIT(0);

> >  static struct console *orig_dbg_cons;

> >

> > +static int kgdb_nmi_poll_one_knock(void);

> > +

> > +static irqreturn_t kgdb_handle_nmi(int irq, void *dev_id)

> > +{

> > +     int ret;

> > +

> > +     if (kgdb_nmi_knock < 0) {

> > +             kgdb_breakpoint();

> > +             return IRQ_HANDLED;

> > +     }

> > +

> > +     ret = kgdb_nmi_poll_one_knock();

> > +     if (ret == NO_POLL_CHAR)

> > +             return IRQ_NONE;

> > +

> > +     while (ret != 1) {

> > +             ret = kgdb_nmi_poll_one_knock();

> > +             if (ret == NO_POLL_CHAR)

> > +                     return IRQ_HANDLED;

> > +     }

> > +

> > +     kgdb_breakpoint();

> > +     return IRQ_HANDLED;

> > +}

> > +

> >  static int kgdb_nmi_console_setup(struct console *co, char *options)

> >  {

> > -     arch_kgdb_ops.enable_nmi(1);

> > +     int res;

> > +

> > +     if (arch_kgdb_ops.enable_nmi) {

> > +             arch_kgdb_ops.enable_nmi(1);

> > +     } else if (dbg_io_ops->request_nmi) {

> > +             res = dbg_io_ops->request_nmi(kgdb_handle_nmi, co);

> > +             if (res) {

> > +                     pr_err("ttyNMI0: Cannot request nmi/irq\n");

> > +                     return res;

> > +             }

> > +     } else {

> > +             return -ENODEV;

> > +     }

> >

> >       /* The NMI console uses the dbg_io_ops to issue console messages. To

> >        * avoid duplicate messages during kdb sessions we must inform kdb's

> > @@ -328,9 +365,6 @@ int kgdb_register_nmi_console(void)

> >  {

> >       int ret;

> >

> > -     if (!arch_kgdb_ops.enable_nmi)

> > -             return 0;

> > -

> >       kgdb_nmi_tty_driver = alloc_tty_driver(1);

> >       if (!kgdb_nmi_tty_driver) {

> >               pr_err("%s: cannot allocate tty\n", __func__);

> > @@ -380,9 +414,8 @@ int kgdb_unregister_nmi_console(void)

> >  {

> >       int ret;

> >

> > -     if (!arch_kgdb_ops.enable_nmi)

> > -             return 0;

> > -     arch_kgdb_ops.enable_nmi(0);

> > +     if (arch_kgdb_ops.enable_nmi)

> > +             arch_kgdb_ops.enable_nmi(0);

> >

> >       ret = unregister_console(&kgdb_nmi_console);

> >       if (ret)

> > --

> > 2.7.4

> >

Patch

diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c
index b32c6b1..2580f39 100644
--- a/drivers/tty/serial/kgdb_nmi.c
+++ b/drivers/tty/serial/kgdb_nmi.c
@@ -42,9 +42,46 @@  MODULE_PARM_DESC(magic, "magic sequence to enter NMI debugger (default $3#33)");
 static atomic_t kgdb_nmi_num_readers = ATOMIC_INIT(0);
 static struct console *orig_dbg_cons;
 
+static int kgdb_nmi_poll_one_knock(void);
+
+static irqreturn_t kgdb_handle_nmi(int irq, void *dev_id)
+{
+	int ret;
+
+	if (kgdb_nmi_knock < 0) {
+		kgdb_breakpoint();
+		return IRQ_HANDLED;
+	}
+
+	ret = kgdb_nmi_poll_one_knock();
+	if (ret == NO_POLL_CHAR)
+		return IRQ_NONE;
+
+	while (ret != 1) {
+		ret = kgdb_nmi_poll_one_knock();
+		if (ret == NO_POLL_CHAR)
+			return IRQ_HANDLED;
+	}
+
+	kgdb_breakpoint();
+	return IRQ_HANDLED;
+}
+
 static int kgdb_nmi_console_setup(struct console *co, char *options)
 {
-	arch_kgdb_ops.enable_nmi(1);
+	int res;
+
+	if (arch_kgdb_ops.enable_nmi) {
+		arch_kgdb_ops.enable_nmi(1);
+	} else if (dbg_io_ops->request_nmi) {
+		res = dbg_io_ops->request_nmi(kgdb_handle_nmi, co);
+		if (res) {
+			pr_err("ttyNMI0: Cannot request nmi/irq\n");
+			return res;
+		}
+	} else {
+		return -ENODEV;
+	}
 
 	/* The NMI console uses the dbg_io_ops to issue console messages. To
 	 * avoid duplicate messages during kdb sessions we must inform kdb's
@@ -328,9 +365,6 @@  int kgdb_register_nmi_console(void)
 {
 	int ret;
 
-	if (!arch_kgdb_ops.enable_nmi)
-		return 0;
-
 	kgdb_nmi_tty_driver = alloc_tty_driver(1);
 	if (!kgdb_nmi_tty_driver) {
 		pr_err("%s: cannot allocate tty\n", __func__);
@@ -380,9 +414,8 @@  int kgdb_unregister_nmi_console(void)
 {
 	int ret;
 
-	if (!arch_kgdb_ops.enable_nmi)
-		return 0;
-	arch_kgdb_ops.enable_nmi(0);
+	if (arch_kgdb_ops.enable_nmi)
+		arch_kgdb_ops.enable_nmi(0);
 
 	ret = unregister_console(&kgdb_nmi_console);
 	if (ret)