On Wed, Mar 08, 2017 at 10:19:19PM +1000, Nicholas Piggin wrote: > Hi Gautham, > > I'm just getting back to this. Sorry for the late reply, and > thanks for the reviews.
No problems. > [..snip..] > > > +#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. Sounds reasonable. > > > > + */ > > > + 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? Perhaps oris is a better choice in this case since we are anyway setting every bit in 42:45 range. Not sure if it will save any cycles, but it will certainly reduce an instruction! > > > > > > 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. > Looking forward to the new version! > Thanks, > Nick > -- Thanks and Regards gautham.
