Hi Gautham, I'm just getting back to this. Sorry for the late reply, and thanks for the reviews.
On Tue, 28 Feb 2017 22:45:46 +0530 Gautham R Shenoy <ego.l...@gmail.com> wrote: > Hi Nick, > > On Fri, Feb 17, 2017 at 12:09 AM, Nicholas Piggin <npig...@gmail.com> wrote: > > The ISA specifies power save wakeup can cause a machine check interrupt. > > The machine check handler currently has code to handle that for POWER8, > > but POWER9 crashes when trying to execute the P8 style sleep > > instructions. > > > > So queue up the machine check, then call into the idle code to wake up > > as the system reset interrupt does, rather than attempting to sleep > > again without going through the main idle path. > > > > Signed-off-by: Nicholas Piggin <npig...@gmail.com> > > --- > > arch/powerpc/kernel/exceptions-64s.S | 71 > > ++++++++++++++++++------------------ > > 1 file changed, 36 insertions(+), 35 deletions(-) > > > > diff --git a/arch/powerpc/kernel/exceptions-64s.S > > b/arch/powerpc/kernel/exceptions-64s.S > > index 5f775783f744..0388843c8d12 100644 > > --- a/arch/powerpc/kernel/exceptions-64s.S > > +++ b/arch/powerpc/kernel/exceptions-64s.S > > @@ -329,6 +329,35 @@ EXC_COMMON_BEGIN(machine_check_common) > > /* restore original r1. */ \ > > ld r1,GPR1(r1) > > > > +#ifdef CONFIG_PPC_P7_NAP > > +EXC_COMMON_BEGIN(machine_check_idle_common) > > + bl machine_check_queue_event > > + /* > > + * Queue the machine check, then reload SRR1 and use it to set > > + * CR3 according to pnv_powersave_wakeup convention. > > + */ > > + ld r12,_MSR(r1) > > + rlwinm r11,r12,47-31,30,31 > > + cmpwi cr3,r11,2 > > + > > + /* > > + * Now have to make SRR1 wake up reason look like a system reset > > + * interrupt. Put 0xf in there, which is reserved (and does not > > + * match HMI). > > The only places where the wakeup reason is presently checked on the way out > of idle-exit are in KVM guest entry path in kvmppc_check_wake_reason() > and on the CPU-Hotplug exit path pnv_smp_cpu_kill_self(). In both places > we do a positive check for EE, Doorbell, HVEE . The kvm case is also > interested in > HMI. We ignore all the other reasons at the moment. > > So this should be fine. Okay, thanks for confirming. We will have to be careful about this I suppose if the wakeup reasons are expanded. I'll make a note of it in asm/reg.h with the WAKEMASK definitions. > > + */ > > + li r11,0xf > > + insrdi r12,r11,4,45 > > > Shouldn't this be insrdi r12,r11,4,42? The exception bits are 42:45. > I always have trouble wrapping my head around these nifty > rotate-shift-mask-insert instructions. So I might as well be wrong! Ah I think you're right, good catch. Maybe oris r12,r12,0x3c is a better choice than that insrdi? > > Otherwise, the patch looks correct to me. > Reviewed-by: Gautham R. Shenoy <e...@linux.vnet.ibm.com> Very much appreciate the reviews. I'm just getting some time to work on the winkle count patch, so I'll repost with your suggestions when that's done. Thanks, Nick
