This patch adds a new "KDB_REASON" code (KDB_REASON_SYSTEM_NMI). This
is purely cosmetic to distinguish it from the other various reasons that
NMI may occur and are usually after an error occurred. Also the dumping
of registers is not done to more closely match what is displayed when KDB
is entered manually via the sysreq 'g' key.
Signed-off-by: Mike Travis <[email protected]>
Reviewed-by: Dimitri Sivanich <[email protected]>
Reviewed-by: Hedi Berriche <[email protected]>
---
include/linux/kdb.h | 1 +
include/linux/kgdb.h | 1 +
kernel/debug/debug_core.c | 5 +++++
kernel/debug/kdb/kdb_debugger.c | 5 ++++-
kernel/debug/kdb/kdb_main.c | 3 +++
5 files changed, 14 insertions(+), 1 deletion(-)
--- linux.orig/include/linux/kdb.h
+++ linux/include/linux/kdb.h
@@ -109,6 +109,7 @@ typedef enum {
KDB_REASON_RECURSE, /* Recursive entry to kdb;
* regs probably valid */
KDB_REASON_SSTEP, /* Single Step trap. - regs valid */
+ KDB_REASON_SYSTEM_NMI, /* In NMI due to SYSTEM cmd; regs valid */
} kdb_reason_t;
extern int kdb_trap_printk;
--- linux.orig/include/linux/kgdb.h
+++ linux/include/linux/kgdb.h
@@ -52,6 +52,7 @@ extern int kgdb_connected;
extern int kgdb_io_module_registered;
extern atomic_t kgdb_setting_breakpoint;
+extern atomic_t kgdb_system_nmi;
extern atomic_t kgdb_cpu_doing_single_step;
extern struct task_struct *kgdb_usethread;
--- linux.orig/kernel/debug/debug_core.c
+++ linux/kernel/debug/debug_core.c
@@ -125,6 +125,7 @@ static atomic_t masters_in_kgdb;
static atomic_t slaves_in_kgdb;
static atomic_t kgdb_break_tasklet_var;
atomic_t kgdb_setting_breakpoint;
+atomic_t kgdb_system_nmi;
struct task_struct *kgdb_usethread;
struct task_struct *kgdb_contthread;
@@ -760,7 +761,11 @@ int kgdb_nmicallin(int cpu, int trapnr,
/* Indicate there are slaves waiting */
kgdb_info[cpu].send_ready = send_ready;
+
+ /* Use new reason code "SYSTEM_NMI" */
+ atomic_inc(&kgdb_system_nmi);
kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
+ atomic_dec(&kgdb_system_nmi);
kgdb_do_roundup = save_kgdb_do_roundup;
kgdb_info[cpu].send_ready = NULL;
--- linux.orig/kernel/debug/kdb/kdb_debugger.c
+++ linux/kernel/debug/kdb/kdb_debugger.c
@@ -69,7 +69,10 @@ int kdb_stub(struct kgdb_state *ks)
if (atomic_read(&kgdb_setting_breakpoint))
reason = KDB_REASON_KEYBOARD;
- if (in_nmi())
+ if (atomic_read(&kgdb_system_nmi))
+ reason = KDB_REASON_SYSTEM_NMI;
+
+ else if (in_nmi())
reason = KDB_REASON_NMI;
for (i = 0, bp = kdb_breakpoints; i < KDB_MAXBPT; i++, bp++) {
--- linux.orig/kernel/debug/kdb/kdb_main.c
+++ linux/kernel/debug/kdb/kdb_main.c
@@ -1200,6 +1200,9 @@ static int kdb_local(kdb_reason_t reason
instruction_pointer(regs));
kdb_dumpregs(regs);
break;
+ case KDB_REASON_SYSTEM_NMI:
+ kdb_printf("due to System NonMaskable Interrupt\n");
+ break;
case KDB_REASON_NMI:
kdb_printf("due to NonMaskable Interrupt @ "
kdb_machreg_fmt "\n",
--
On 09/05/2013 05:50 PM, Mike Travis wrote:
> This patch adds a new "KDB_REASON" code (KDB_REASON_SYSTEM_NMI). This
> is purely cosmetic to distinguish it from the other various reasons that
> NMI may occur and are usually after an error occurred. Also the dumping
> of registers is not done to more closely match what is displayed when KDB
> is entered manually via the sysreq 'g' key.
This patch is not quite right. See below.
>
> Signed-off-by: Mike Travis <[email protected]>
> Reviewed-by: Dimitri Sivanich <[email protected]>
> Reviewed-by: Hedi Berriche <[email protected]>
> ---
> include/linux/kdb.h | 1 +
> include/linux/kgdb.h | 1 +
> kernel/debug/debug_core.c | 5 +++++
> kernel/debug/kdb/kdb_debugger.c | 5 ++++-
> kernel/debug/kdb/kdb_main.c | 3 +++
> 5 files changed, 14 insertions(+), 1 deletion(-)
>
> --- linux.orig/include/linux/kdb.h
> +++ linux/include/linux/kdb.h
> @@ -109,6 +109,7 @@ typedef enum {
> KDB_REASON_RECURSE, /* Recursive entry to kdb;
> * regs probably valid */
> KDB_REASON_SSTEP, /* Single Step trap. - regs valid */
> + KDB_REASON_SYSTEM_NMI, /* In NMI due to SYSTEM cmd; regs valid */
> } kdb_reason_t;
>
> extern int kdb_trap_printk;
> --- linux.orig/include/linux/kgdb.h
> +++ linux/include/linux/kgdb.h
> @@ -52,6 +52,7 @@ extern int kgdb_connected;
> extern int kgdb_io_module_registered;
>
> extern atomic_t kgdb_setting_breakpoint;
> +extern atomic_t kgdb_system_nmi;
We don't need extra atomics. You should add another variable to the kgdb_state which is processor specific in this case.
Better yet, just set the ks->err_code properly in your kgdb_nmicallin() or in the origination call to kgdb_nmicallback() from your nmi handler (remember I still have the question pending if we actually need kgdb_nmicallin() in the first place. You already did the work of adding another NMI type to the enum. We just need to use the ks->err_code variable as well.
> extern atomic_t kgdb_cpu_doing_single_step;
>
> extern struct task_struct *kgdb_usethread;
> --- linux.orig/kernel/debug/debug_core.c
> +++ linux/kernel/debug/debug_core.c
> @@ -125,6 +125,7 @@ static atomic_t masters_in_kgdb;
> static atomic_t slaves_in_kgdb;
> static atomic_t kgdb_break_tasklet_var;
> atomic_t kgdb_setting_breakpoint;
> +atomic_t kgdb_system_nmi;
>
> struct task_struct *kgdb_usethread;
> struct task_struct *kgdb_contthread;
> @@ -760,7 +761,11 @@ int kgdb_nmicallin(int cpu, int trapnr,
>
> /* Indicate there are slaves waiting */
> kgdb_info[cpu].send_ready = send_ready;
> +
> + /* Use new reason code "SYSTEM_NMI" */
> + atomic_inc(&kgdb_system_nmi);
> kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
> + atomic_dec(&kgdb_system_nmi);
> kgdb_do_roundup = save_kgdb_do_roundup;
> kgdb_info[cpu].send_ready = NULL;
>
> --- linux.orig/kernel/debug/kdb/kdb_debugger.c
> +++ linux/kernel/debug/kdb/kdb_debugger.c
> @@ -69,7 +69,10 @@ int kdb_stub(struct kgdb_state *ks)
> if (atomic_read(&kgdb_setting_breakpoint))
> reason = KDB_REASON_KEYBOARD;
>
> - if (in_nmi())
> + if (atomic_read(&kgdb_system_nmi))
> + reason = KDB_REASON_SYSTEM_NMI;
This would get changed to if (ks->err == KDB_REASON_SYSNMI && ks->signo == SIGTRAP) ....
Cheers,
Jason.
> +
> + else if (in_nmi())
> reason = KDB_REASON_NMI;
>
> for (i = 0, bp = kdb_breakpoints; i < KDB_MAXBPT; i++, bp++) {
> --- linux.orig/kernel/debug/kdb/kdb_main.c
> +++ linux/kernel/debug/kdb/kdb_main.c
> @@ -1200,6 +1200,9 @@ static int kdb_local(kdb_reason_t reason
> instruction_pointer(regs));
> kdb_dumpregs(regs);
> break;
> + case KDB_REASON_SYSTEM_NMI:
> + kdb_printf("due to System NonMaskable Interrupt\n");
> + break;
> case KDB_REASON_NMI:
> kdb_printf("due to NonMaskable Interrupt @ "
> kdb_machreg_fmt "\n",
>
> --
>
On 9/5/2013 10:00 PM, Jason Wessel wrote:
> On 09/05/2013 05:50 PM, Mike Travis wrote:
>> This patch adds a new "KDB_REASON" code (KDB_REASON_SYSTEM_NMI). This
>> is purely cosmetic to distinguish it from the other various reasons that
>> NMI may occur and are usually after an error occurred. Also the dumping
>> of registers is not done to more closely match what is displayed when KDB
>> is entered manually via the sysreq 'g' key.
>
>
> This patch is not quite right. See below.
>
>
>>
>> Signed-off-by: Mike Travis <[email protected]>
>> Reviewed-by: Dimitri Sivanich <[email protected]>
>> Reviewed-by: Hedi Berriche <[email protected]>
>> ---
>> include/linux/kdb.h | 1 +
>> include/linux/kgdb.h | 1 +
>> kernel/debug/debug_core.c | 5 +++++
>> kernel/debug/kdb/kdb_debugger.c | 5 ++++-
>> kernel/debug/kdb/kdb_main.c | 3 +++
>> 5 files changed, 14 insertions(+), 1 deletion(-)
>>
>> --- linux.orig/include/linux/kdb.h
>> +++ linux/include/linux/kdb.h
>> @@ -109,6 +109,7 @@ typedef enum {
>> KDB_REASON_RECURSE, /* Recursive entry to kdb;
>> * regs probably valid */
>> KDB_REASON_SSTEP, /* Single Step trap. - regs valid */
>> + KDB_REASON_SYSTEM_NMI, /* In NMI due to SYSTEM cmd; regs valid */
>> } kdb_reason_t;
>>
>> extern int kdb_trap_printk;
>> --- linux.orig/include/linux/kgdb.h
>> +++ linux/include/linux/kgdb.h
>> @@ -52,6 +52,7 @@ extern int kgdb_connected;
>> extern int kgdb_io_module_registered;
>>
>> extern atomic_t kgdb_setting_breakpoint;
>> +extern atomic_t kgdb_system_nmi;
>
>
> We don't need extra atomics. You should add another variable to the
kgdb_state which is processor specific in this case.
>
> Better yet, just set the ks->err_code properly in your
kgdb_nmicallin() or in the origination call to kgdb_nmicallback() from
your nmi handler (remember I still have the question pending if we
actually need kgdb_nmicallin() in the first place. You already did the
work of adding another NMI type to the enum. We just need to use the
ks->err_code variable as well.
Good idea, I hadn't thought of using that field. In fact, it
simplified the patch enough that I just folded into the other.
I'll address your other question separately.
Thanks!
Mike
>
>
>> extern atomic_t kgdb_cpu_doing_single_step;
>>
>> extern struct task_struct *kgdb_usethread;
>> --- linux.orig/kernel/debug/debug_core.c
>> +++ linux/kernel/debug/debug_core.c
>> @@ -125,6 +125,7 @@ static atomic_t masters_in_kgdb;
>> static atomic_t slaves_in_kgdb;
>> static atomic_t kgdb_break_tasklet_var;
>> atomic_t kgdb_setting_breakpoint;
>> +atomic_t kgdb_system_nmi;
>>
>> struct task_struct *kgdb_usethread;
>> struct task_struct *kgdb_contthread;
>> @@ -760,7 +761,11 @@ int kgdb_nmicallin(int cpu, int trapnr,
>>
>> /* Indicate there are slaves waiting */
>> kgdb_info[cpu].send_ready = send_ready;
>> +
>> + /* Use new reason code "SYSTEM_NMI" */
>> + atomic_inc(&kgdb_system_nmi);
>> kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
>> + atomic_dec(&kgdb_system_nmi);
>> kgdb_do_roundup = save_kgdb_do_roundup;
>> kgdb_info[cpu].send_ready = NULL;
>>
>> --- linux.orig/kernel/debug/kdb/kdb_debugger.c
>> +++ linux/kernel/debug/kdb/kdb_debugger.c
>> @@ -69,7 +69,10 @@ int kdb_stub(struct kgdb_state *ks)
>> if (atomic_read(&kgdb_setting_breakpoint))
>> reason = KDB_REASON_KEYBOARD;
>>
>> - if (in_nmi())
>> + if (atomic_read(&kgdb_system_nmi))
>> + reason = KDB_REASON_SYSTEM_NMI;
>
>
> This would get changed to if (ks->err == KDB_REASON_SYSNMI &&
ks->signo == SIGTRAP) ....
>
> Cheers,
> Jason.
>
>> +
>> + else if (in_nmi())
>> reason = KDB_REASON_NMI;
>>
>> for (i = 0, bp = kdb_breakpoints; i < KDB_MAXBPT; i++, bp++) {
>> --- linux.orig/kernel/debug/kdb/kdb_main.c
>> +++ linux/kernel/debug/kdb/kdb_main.c
>> @@ -1200,6 +1200,9 @@ static int kdb_local(kdb_reason_t reason
>> instruction_pointer(regs));
>> kdb_dumpregs(regs);
>> break;
>> + case KDB_REASON_SYSTEM_NMI:
>> + kdb_printf("due to System NonMaskable Interrupt\n");
>> + break;
>> case KDB_REASON_NMI:
>> kdb_printf("due to NonMaskable Interrupt @ "
>> kdb_machreg_fmt "\n",
>>
>> --
>>
>