| Message ID | 20171215012740.30291-2-bsingharora@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/3] powerpc/crash: Remove the test for cpu_online in the IPI callback | expand |
On Fri, 15 Dec 2017 12:27:39 +1100 Balbir Singh <bsingharora@gmail.com> wrote: > In irq_set_pending_from_srr1() we were missing 0x2 as system > reset identified from SRR1 caused by back to back system > resets or when interrupts are caused by SCOM when the thread > is not in power saving mode. > > This helps us get to NMI handling in both the case where NMI > is caused when in power-saving and not in power-saving mode. > The actual exploitation is expected when we are doing a kdump > and an offline CPU might not be in power-saving mode due to > an already spurious IPI or any other reason. > > Signed-off-by: Balbir Singh <bsingharora@gmail.com> When not in power saving mode, we don't look at SRR1 at all, so we don't need this. You should never be getting it returned as the result of your idle instruction (except on DD1 which has a bug, but firmware doesn't implement the NMI IPI). It's possible we could pay more attention to the reason, for example in the powernv system reset handle we might only call the NMI IPI handler if it was a scom sreset... but that would be a regs->msr test. > --- > arch/powerpc/kernel/irq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c > index b7a84522e652..ec89104e8ab9 100644 > --- a/arch/powerpc/kernel/irq.c > +++ b/arch/powerpc/kernel/irq.c > @@ -413,7 +413,7 @@ bool prep_irq_for_idle_irqsoff(void) > #define IRQ_SYSTEM_RESET 0xff > > static const u8 srr1_to_lazyirq[0x10] = { > - 0, 0, 0, > + 0, 0, IRQ_SYSTEM_RESET, > PACA_IRQ_DBELL, > IRQ_SYSTEM_RESET, > PACA_IRQ_DBELL,
On Fri, Dec 15, 2017 at 1:44 PM, Nicholas Piggin <npiggin@gmail.com> wrote: > On Fri, 15 Dec 2017 12:27:39 +1100 > Balbir Singh <bsingharora@gmail.com> wrote: > >> In irq_set_pending_from_srr1() we were missing 0x2 as system >> reset identified from SRR1 caused by back to back system >> resets or when interrupts are caused by SCOM when the thread >> is not in power saving mode. >> >> This helps us get to NMI handling in both the case where NMI >> is caused when in power-saving and not in power-saving mode. >> The actual exploitation is expected when we are doing a kdump >> and an offline CPU might not be in power-saving mode due to >> an already spurious IPI or any other reason. >> >> Signed-off-by: Balbir Singh <bsingharora@gmail.com> > > When not in power saving mode, we don't look at SRR1 at all, so > we don't need this. You should never be getting it returned as the > result of your idle instruction (except on DD1 which has a bug, > but firmware doesn't implement the NMI IPI). > I added this for the next patch. We call irq_set_pending_from_srr1 while coming out of CPU idle, but if for any reason the IPI was spurious and then we see an NMI during kdump, I did want to detect that and callback into the NMI handler. > It's possible we could pay more attention to the reason, for > example in the powernv system reset handle we might only call > the NMI IPI handler if it was a scom sreset... but that would > be a regs->msr test. regs->msr is the same as SRR1 in my kdump case, but your right in general we want to look at regs->msr Balbir Singh
On Fri, 15 Dec 2017 13:54:18 +1100 Balbir Singh <bsingharora@gmail.com> wrote: > On Fri, Dec 15, 2017 at 1:44 PM, Nicholas Piggin <npiggin@gmail.com> wrote: > > On Fri, 15 Dec 2017 12:27:39 +1100 > > Balbir Singh <bsingharora@gmail.com> wrote: > > > >> In irq_set_pending_from_srr1() we were missing 0x2 as system > >> reset identified from SRR1 caused by back to back system > >> resets or when interrupts are caused by SCOM when the thread > >> is not in power saving mode. > >> > >> This helps us get to NMI handling in both the case where NMI > >> is caused when in power-saving and not in power-saving mode. > >> The actual exploitation is expected when we are doing a kdump > >> and an offline CPU might not be in power-saving mode due to > >> an already spurious IPI or any other reason. > >> > >> Signed-off-by: Balbir Singh <bsingharora@gmail.com> > > > > When not in power saving mode, we don't look at SRR1 at all, so > > we don't need this. You should never be getting it returned as the > > result of your idle instruction (except on DD1 which has a bug, > > but firmware doesn't implement the NMI IPI). > > > > I added this for the next patch. We call irq_set_pending_from_srr1 > while coming out of CPU idle, but if for any reason the IPI was spurious > and then we see an NMI during kdump, I did want to detect that > and callback into the NMI handler. You'll never see it. The SRR1 value you get is the idle wakeup code. So any scom-when-not-idle bit should never be set. Thanks, Nick
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c index b7a84522e652..ec89104e8ab9 100644 --- a/arch/powerpc/kernel/irq.c +++ b/arch/powerpc/kernel/irq.c @@ -413,7 +413,7 @@ bool prep_irq_for_idle_irqsoff(void) #define IRQ_SYSTEM_RESET 0xff static const u8 srr1_to_lazyirq[0x10] = { - 0, 0, 0, + 0, 0, IRQ_SYSTEM_RESET, PACA_IRQ_DBELL, IRQ_SYSTEM_RESET, PACA_IRQ_DBELL,
In irq_set_pending_from_srr1() we were missing 0x2 as system reset identified from SRR1 caused by back to back system resets or when interrupts are caused by SCOM when the thread is not in power saving mode. This helps us get to NMI handling in both the case where NMI is caused when in power-saving and not in power-saving mode. The actual exploitation is expected when we are doing a kdump and an offline CPU might not be in power-saving mode due to an already spurious IPI or any other reason. Signed-off-by: Balbir Singh <bsingharora@gmail.com> --- arch/powerpc/kernel/irq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)