On Mon, 12 Jun 2017 15:11:06 +0530
Gautham R Shenoy <e...@linux.vnet.ibm.com> wrote:
> Hi Nick,
>
> On Mon, Jun 12, 2017 at 09:58:25AM +1000, Nicholas Piggin wrote:
> > When the CPU wakes from low power state, it begins at the system reset
> > interrupt with the exception that caused the wakeup encoded in SRR1.
> >
> > Today, powernv idle wakeup ignores the wakeup reason (except a special
> > case for HMI), and the regular interrupt corresponding to the
> > exception will fire after the idle wakeup exits.
> >
> > Change this to replay the interrupt from the idle wakeup before
> > interrupts are hard-enabled.
> >
> > Test on POWER8 of context_switch selftests benchmark with polling idle
> > disabled (e.g., always nap, giving cross-CPU IPIs) gives the following
> > results:
> >
> > original wakeup direct
> > Different threads, same core: 315k/s 264k/s
> > Different cores: 235k/s 242k/s
> >
> > There is a slowdown for doorbell IPI (same core) case because system
> > reset wakeup does not clear the message and the doorbell interrupt
> > fires again needlessly.
>
> Should we clear the doorbell message if we are recording the fact that
> the Doorbell irq has happened in paca ?
As you saw, I clear it in the next patch.
It would be quite tidy to msgclear when moving srr1 wakeup into pending
mask. But on the other hand, I thought that deferring the clear as late
as possible and moving it closer to where IRQs will be hard enabled
again may help to avoid taking other IPIs.
>
> >
> > Signed-off-by: Nicholas Piggin <npig...@gmail.com>
> > ---
> > arch/powerpc/include/asm/hw_irq.h | 2 ++
> > arch/powerpc/kernel/irq.c | 25 +++++++++++++++++++++++++
> > arch/powerpc/platforms/powernv/idle.c | 10 ++++++++--
> > 3 files changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/hw_irq.h
> > b/arch/powerpc/include/asm/hw_irq.h
> > index f06112cf8734..8366bdc69988 100644
>
> [..snip..]
>
> > --- a/arch/powerpc/kernel/irq.c
> > +++ b/arch/powerpc/kernel/irq.c
> > @@ -348,6 +348,7 @@ bool prep_irq_for_idle(void)
> > return true;
> > }
> >
> > +#ifdef CONFIG_PPC_BOOK3S
> > /*
> > * This is for idle sequences that return with IRQs off, but the
> > * idle state itself wakes on interrupt. Tell the irq tracer that
> > @@ -379,6 +380,30 @@ bool prep_irq_for_idle_irqsoff(void)
> > }
> >
> > /*
> > + * Take the SRR1 wakeup reason, index into this table to find the
> > + * appropriate irq_happened bit.
> > + */
> > +static const u8 srr1_to_irq[0x10] = {
> > + 0, 0, 0,
> > + PACA_IRQ_DBELL,
> > + 0,
> > + PACA_IRQ_DBELL,
> > + PACA_IRQ_DEC,
> > + 0,
> > + PACA_IRQ_EE,
> > + PACA_IRQ_EE,
> > + PACA_IRQ_HMI,
> > + 0, 0, 0, 0, 0 };
> > +
> > +void irq_set_pending_from_srr1(unsigned long srr1)
> > +{
> > + unsigned int idx = (srr1 >> 18) & SRR1_WAKEMASK_P8;
>
> Shouldn't this be
> unsigned int idx = (srr1 & SRR1_WAKEMASK_P8) >> 18;
>
> ?
Yes, good catch. That will teach me not to verify after making a change.
Will fix.
Thanks,
Nick
