* [PATCH] MIPS: add nmi_enter() + nmi_exit() to nmi_exception_handler()
@ 2015-10-19 18:49 Petri Gynther
2015-11-02 20:50 ` Petri Gynther
0 siblings, 1 reply; 6+ messages in thread
From: Petri Gynther @ 2015-10-19 18:49 UTC (permalink / raw)
To: linux-mips; +Cc: ralf, Petri Gynther
We need to enter NMI context when NMI interrupt fires.
Signed-off-by: Petri Gynther <pgynther@google.com>
---
arch/mips/kernel/traps.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index fdb392b..efcedd4 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -1856,12 +1856,14 @@ void __noreturn nmi_exception_handler(struct pt_regs *regs)
{
char str[100];
+ nmi_enter();
raw_notifier_call_chain(&nmi_chain, 0, regs);
bust_spinlocks(1);
snprintf(str, 100, "CPU%d NMI taken, CP0_EPC=%lx\n",
smp_processor_id(), regs->cp0_epc);
regs->cp0_epc = read_c0_errorepc();
die(str, regs);
+ nmi_exit();
}
#define VECTORSPACING 0x100 /* for EI/VI mode */
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] MIPS: add nmi_enter() + nmi_exit() to nmi_exception_handler()
2015-10-19 18:49 [PATCH] MIPS: add nmi_enter() + nmi_exit() to nmi_exception_handler() Petri Gynther
@ 2015-11-02 20:50 ` Petri Gynther
2015-11-09 8:09 ` Ralf Baechle
0 siblings, 1 reply; 6+ messages in thread
From: Petri Gynther @ 2015-11-02 20:50 UTC (permalink / raw)
To: linux-mips; +Cc: Ralf Baechle, Petri Gynther
On Mon, Oct 19, 2015 at 11:49 AM, Petri Gynther <pgynther@google.com> wrote:
>
> We need to enter NMI context when NMI interrupt fires.
>
> Signed-off-by: Petri Gynther <pgynther@google.com>
> ---
> arch/mips/kernel/traps.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> index fdb392b..efcedd4 100644
> --- a/arch/mips/kernel/traps.c
> +++ b/arch/mips/kernel/traps.c
> @@ -1856,12 +1856,14 @@ void __noreturn nmi_exception_handler(struct pt_regs *regs)
> {
> char str[100];
>
> + nmi_enter();
> raw_notifier_call_chain(&nmi_chain, 0, regs);
> bust_spinlocks(1);
> snprintf(str, 100, "CPU%d NMI taken, CP0_EPC=%lx\n",
> smp_processor_id(), regs->cp0_epc);
> regs->cp0_epc = read_c0_errorepc();
> die(str, regs);
> + nmi_exit();
> }
>
> #define VECTORSPACING 0x100 /* for EI/VI mode */
> --
> 2.6.0.rc2.230.g3dd15c0
>
Any comments/concerns about this patch?
On our systems, we have seen stack traces like this:
<4>[158549.586000] : [<800103c0>] show_stack+0x78/0x90
<4>[158549.586000] : [<807340a0>] dump_stack+0xd0/0x144
<4>[158549.586000] : [<8006475c>] dequeue_task_idle+0x38/0x4c
<4>[158549.586000] : [<80735c8c>] __schedule+0x49c/0xa68
<4>[158549.586000] : [<80736294>] schedule+0x3c/0x9c
<4>[158549.586000] : [<80739930>] schedule_timeout+0x144/0x254
<4>[158549.586000] : [<800964f8>] msleep+0x40/0x54
<4>[158549.586000] : [<80010604>] die+0x124/0x130
<4>[158549.586000] : [<80012abc>] set_vi_handler+0x0/0x24
<4>[158549.586000] :
<3>[158549.586000] bad: scheduling from the idle thread!
80012a24 <nmi_exception_handler>:
...
80012ab4: 0c004138 jal 800104e0 <die>
80012ab8: 02002821 move a1,s0
80012abc <set_vi_handler>:
...
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] MIPS: add nmi_enter() + nmi_exit() to nmi_exception_handler()
2015-11-02 20:50 ` Petri Gynther
@ 2015-11-09 8:09 ` Ralf Baechle
2016-01-12 1:03 ` Maciej W. Rozycki
0 siblings, 1 reply; 6+ messages in thread
From: Ralf Baechle @ 2015-11-09 8:09 UTC (permalink / raw)
To: Petri Gynther; +Cc: linux-mips
On Mon, Nov 02, 2015 at 12:50:50PM -0800, Petri Gynther wrote:
> On Mon, Oct 19, 2015 at 11:49 AM, Petri Gynther <pgynther@google.com> wrote:
> >
> > We need to enter NMI context when NMI interrupt fires.
> >
> > Signed-off-by: Petri Gynther <pgynther@google.com>
> > ---
> > arch/mips/kernel/traps.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> > index fdb392b..efcedd4 100644
> > --- a/arch/mips/kernel/traps.c
> > +++ b/arch/mips/kernel/traps.c
> > @@ -1856,12 +1856,14 @@ void __noreturn nmi_exception_handler(struct pt_regs *regs)
> > {
> > char str[100];
> >
> > + nmi_enter();
> > raw_notifier_call_chain(&nmi_chain, 0, regs);
> > bust_spinlocks(1);
> > snprintf(str, 100, "CPU%d NMI taken, CP0_EPC=%lx\n",
> > smp_processor_id(), regs->cp0_epc);
> > regs->cp0_epc = read_c0_errorepc();
> > die(str, regs);
> > + nmi_exit();
> > }
> >
> > #define VECTORSPACING 0x100 /* for EI/VI mode */
> > --
> > 2.6.0.rc2.230.g3dd15c0
> >
>
> Any comments/concerns about this patch?
Is NMI on your systems actually recoverable? I never bothered with
nmi_enther / nmi_exit and other fine details of the NMI implementations
because as defined by the MIPS architecture an NMI may be pretty destructive
and closer to a reset than what other architectures describer as their NMI.
Think what's going to happen if it hits during any phase when $k0 / $k1
are active.
Ralf
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] MIPS: add nmi_enter() + nmi_exit() to nmi_exception_handler()
2015-11-09 8:09 ` Ralf Baechle
@ 2016-01-12 1:03 ` Maciej W. Rozycki
2016-01-12 13:50 ` Ralf Baechle
0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2016-01-12 1:03 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Petri Gynther, linux-mips
On Mon, 9 Nov 2015, Ralf Baechle wrote:
> > > diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> > > index fdb392b..efcedd4 100644
> > > --- a/arch/mips/kernel/traps.c
> > > +++ b/arch/mips/kernel/traps.c
> > > @@ -1856,12 +1856,14 @@ void __noreturn nmi_exception_handler(struct pt_regs *regs)
> > > {
> > > char str[100];
> > >
> > > + nmi_enter();
> > > raw_notifier_call_chain(&nmi_chain, 0, regs);
> > > bust_spinlocks(1);
> > > snprintf(str, 100, "CPU%d NMI taken, CP0_EPC=%lx\n",
> > > smp_processor_id(), regs->cp0_epc);
> > > regs->cp0_epc = read_c0_errorepc();
> > > die(str, regs);
> > > + nmi_exit();
> > > }
> > >
> > > #define VECTORSPACING 0x100 /* for EI/VI mode */
> > > --
> > > 2.6.0.rc2.230.g3dd15c0
> > >
> >
> > Any comments/concerns about this patch?
>
> Is NMI on your systems actually recoverable? I never bothered with
> nmi_enther / nmi_exit and other fine details of the NMI implementations
> because as defined by the MIPS architecture an NMI may be pretty destructive
> and closer to a reset than what other architectures describer as their NMI.
> Think what's going to happen if it hits during any phase when $k0 / $k1
> are active.
We could do better though, by having a register stash area defined
somewhere in low memory (0x0-0x7fff) -- of course if physical memory is
actually available there in a given system. Remember that setting
CP0.Status.ERL makes KUSEG identity mapped, making it possible to access
its beginning off $zero and save all GPRs in a non-destructive manner.
That is however assuming we can take control at all in the first place as
the NMI vector is hardwired and points to a ROM location in a typical
system.
Maciej
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] MIPS: add nmi_enter() + nmi_exit() to nmi_exception_handler()
2016-01-12 1:03 ` Maciej W. Rozycki
@ 2016-01-12 13:50 ` Ralf Baechle
2016-01-12 21:38 ` Maciej W. Rozycki
0 siblings, 1 reply; 6+ messages in thread
From: Ralf Baechle @ 2016-01-12 13:50 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Petri Gynther, linux-mips
On Tue, Jan 12, 2016 at 01:03:18AM +0000, Maciej W. Rozycki wrote:
> On Mon, 9 Nov 2015, Ralf Baechle wrote:
>
> > > > diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> > > > index fdb392b..efcedd4 100644
> > > > --- a/arch/mips/kernel/traps.c
> > > > +++ b/arch/mips/kernel/traps.c
> > > > @@ -1856,12 +1856,14 @@ void __noreturn nmi_exception_handler(struct pt_regs *regs)
> > > > {
> > > > char str[100];
> > > >
> > > > + nmi_enter();
> > > > raw_notifier_call_chain(&nmi_chain, 0, regs);
> > > > bust_spinlocks(1);
> > > > snprintf(str, 100, "CPU%d NMI taken, CP0_EPC=%lx\n",
> > > > smp_processor_id(), regs->cp0_epc);
> > > > regs->cp0_epc = read_c0_errorepc();
> > > > die(str, regs);
> > > > + nmi_exit();
> > > > }
> > > >
> > > > #define VECTORSPACING 0x100 /* for EI/VI mode */
> > > > --
> > > > 2.6.0.rc2.230.g3dd15c0
> > > >
> > >
> > > Any comments/concerns about this patch?
> >
> > Is NMI on your systems actually recoverable? I never bothered with
> > nmi_enther / nmi_exit and other fine details of the NMI implementations
> > because as defined by the MIPS architecture an NMI may be pretty destructive
> > and closer to a reset than what other architectures describer as their NMI.
> > Think what's going to happen if it hits during any phase when $k0 / $k1
> > are active.
>
> We could do better though, by having a register stash area defined
> somewhere in low memory (0x0-0x7fff) -- of course if physical memory is
> actually available there in a given system. Remember that setting
> CP0.Status.ERL makes KUSEG identity mapped, making it possible to access
> its beginning off $zero and save all GPRs in a non-destructive manner.
>
> That is however assuming we can take control at all in the first place as
> the NMI vector is hardwired and points to a ROM location in a typical
> system.
NMIs don't nest; the system is lost if it receives another NMI before the
state of the first is saved. It's currently up to the system to avoid that
probably by yes masking the non-maskable interrupt.
ErrorEPC is also used by cache errors so an NMI following a cache error
exception before state has been saved might be fatal.
These are scenarios that are taken care of by CISC architectures but on a
purebred RISC they're up to system implementors.
Ralf
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] MIPS: add nmi_enter() + nmi_exit() to nmi_exception_handler()
2016-01-12 13:50 ` Ralf Baechle
@ 2016-01-12 21:38 ` Maciej W. Rozycki
0 siblings, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2016-01-12 21:38 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Petri Gynther, linux-mips
On Tue, 12 Jan 2016, Ralf Baechle wrote:
> > We could do better though, by having a register stash area defined
> > somewhere in low memory (0x0-0x7fff) -- of course if physical memory is
> > actually available there in a given system. Remember that setting
> > CP0.Status.ERL makes KUSEG identity mapped, making it possible to access
> > its beginning off $zero and save all GPRs in a non-destructive manner.
> >
> > That is however assuming we can take control at all in the first place as
> > the NMI vector is hardwired and points to a ROM location in a typical
> > system.
>
> NMIs don't nest; the system is lost if it receives another NMI before the
> state of the first is saved. It's currently up to the system to avoid that
> probably by yes masking the non-maskable interrupt.
Indeed, ErrorEPC will be lost on a nested NMI. We should be able to
detect it and let the handler complete gracefully if it reaches to the end
uninterrupted.
> ErrorEPC is also used by cache errors so an NMI following a cache error
> exception before state has been saved might be fatal.
Hmm, I think a cache error is fatal by itself, so this scenario is
probably not of much concern -- just dumping the available state to the
console and panicking should do.
> These are scenarios that are taken care of by CISC architectures but on a
> purebred RISC they're up to system implementors.
E.g. x86 masks NMIs internally to avoid nesting, but it is able to notice
another incoming NMI and releases it as soon as the handling of the
previous one has completed. We'd need to have external circuitry for any
handling of this kind.
Maciej
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-12 21:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-19 18:49 [PATCH] MIPS: add nmi_enter() + nmi_exit() to nmi_exception_handler() Petri Gynther
2015-11-02 20:50 ` Petri Gynther
2015-11-09 8:09 ` Ralf Baechle
2016-01-12 1:03 ` Maciej W. Rozycki
2016-01-12 13:50 ` Ralf Baechle
2016-01-12 21:38 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox