From: Gautham R Shenoy <ego@linux•vnet.ibm.com>
To: Nicholas Piggin <npiggin@gmail•com>
Cc: Mahesh Jagannath Salgaonkar <mahesh@linux•vnet.ibm.com>,
linuxppc-dev@lists•ozlabs.org,
Gautham R Shenoy <ego@linux•vnet.ibm.com>
Subject: Re: [PATCH 4/8] powerpc/64s: fix POWER9 machine check handler from stop state
Date: Thu, 16 Mar 2017 18:49:07 +0530 [thread overview]
Message-ID: <20170316131907.GF16462@in.ibm.com> (raw)
In-Reply-To: <20170316230520.5ba5528e@roar.ozlabs.ibm.com>
Hi,
On Thu, Mar 16, 2017 at 11:05:20PM +1000, Nicholas Piggin wrote:
> On Thu, 16 Mar 2017 18:10:48 +0530
> Mahesh Jagannath Salgaonkar <mahesh@linux•vnet.ibm.com> wrote:
>
> > On 03/14/2017 02:53 PM, Nicholas Piggin 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.
> > >
> > > Reviewed-by: Gautham R. Shenoy <ego@linux•vnet.ibm.com>
> > > Signed-off-by: Nicholas Piggin <npiggin@gmail•com>
> > > ---
> > > arch/powerpc/include/asm/reg.h | 1 +
> > > arch/powerpc/kernel/exceptions-64s.S | 69 ++++++++++++++++++------------------
> > > 2 files changed, 35 insertions(+), 35 deletions(-)
> > >
> > > diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> > > index fc879fd6bdae..8bbdfacce970 100644
> > > --- a/arch/powerpc/include/asm/reg.h
> > > +++ b/arch/powerpc/include/asm/reg.h
> > > @@ -656,6 +656,7 @@
> > > #define SRR1_ISI_PROT 0x08000000 /* ISI: Other protection fault */
> > > #define SRR1_WAKEMASK 0x00380000 /* reason for wakeup */
> > > #define SRR1_WAKEMASK_P8 0x003c0000 /* reason for wakeup on POWER8 and 9 */
> > > +#define SRR1_WAKEMCE_RESVD 0x003c0000 /* Unused/reserved value used by MCE wakeup to indicate cause to idle wakeup handler */
> > > #define SRR1_WAKESYSERR 0x00300000 /* System error */
> > > #define SRR1_WAKEEE 0x00200000 /* External interrupt */
> > > #define SRR1_WAKEHVI 0x00240000 /* Hypervisor Virtualization Interrupt (P9) */
> > > diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> > > index e390fcd04bcb..5779d2d6a192 100644
> > > --- a/arch/powerpc/kernel/exceptions-64s.S
> > > +++ b/arch/powerpc/kernel/exceptions-64s.S
> > > @@ -306,6 +306,33 @@ 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 put SRR1_WAKEMCE_RESVD into SRR1, allows it to follow the
> > > + * system reset wakeup code.
> > > + */
> > > + oris r12,r12,SRR1_WAKEMCE_RESVD@h
> > > + mtspr SPRN_SRR1,r12
> > > + std r12,_MSR(r1)
> > > +
> > > + /*
> > > + * Decrement MCE nesting after finishing with the stack.
> > > + */
> > > + lhz r11,PACA_IN_MCE(r13)
> > > + subi r11,r11,1
> > > + sth r11,PACA_IN_MCE(r13)
> >
> > Looks like we are not winding up.. Shouldn't we ? What if we may end up
> > in pnv_wakeup_noloss() which assumes that no GPRs are lost. Am I missing
> > anything ?
Nice catch! This can occur if SRR1[46:47] == 0b01.
>
> Hmm, no I think you're right. Thanks, good catch. But can we do it with
> just setting PACA_NAPSTATELOST?
Unconditionally setting PACA_NAPSTATELOST should be sufficient.
>
> >
> > > + b pnv_powersave_wakeup
> > > +#endif
> > > /*
> >
> > [...]
> >
> > Rest looks good to me.
> >
> > Reviewed-by: Mahesh J Salgaonkar <mahesh@linux•vnet.ibm.com>
>
> Thanks,
> Nick
>
next prev parent reply other threads:[~2017-03-16 13:19 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-14 9:23 [PATCH 0/8] idle fixes and changes for POWER8 and POWER9 Nicholas Piggin
2017-03-14 9:23 ` [PATCH 1/8] powerpc/64s: move remaining system reset idle code into idle_book3s.S Nicholas Piggin
2017-03-14 9:23 ` [PATCH 2/8] powerpc/64s: stop using bit in HSPRG0 to test winkle Nicholas Piggin
2017-03-16 11:14 ` Gautham R Shenoy
2017-03-14 9:23 ` [PATCH 3/8] powerpc/64s: use alternative feature patching Nicholas Piggin
2017-03-16 11:21 ` Gautham R Shenoy
2017-03-14 9:23 ` [PATCH 4/8] powerpc/64s: fix POWER9 machine check handler from stop state Nicholas Piggin
2017-03-16 12:40 ` Mahesh Jagannath Salgaonkar
2017-03-16 13:05 ` Nicholas Piggin
2017-03-16 13:19 ` Gautham R Shenoy [this message]
2017-03-20 5:22 ` Mahesh Jagannath Salgaonkar
2017-03-17 2:49 ` Nicholas Piggin
2017-03-17 5:15 ` Nicholas Piggin
2017-03-14 9:23 ` [PATCH 5/8] powerpc/64s: use PACA_THREAD_IDLE_STATE only in POWER8 Nicholas Piggin
2017-03-16 11:54 ` Gautham R Shenoy
2017-03-16 12:16 ` Nicholas Piggin
2017-03-14 9:23 ` [PATCH 6/8] powerpc/64s: idle expand usable core idle state bits Nicholas Piggin
2017-03-16 12:10 ` Gautham R Shenoy
2017-03-14 9:23 ` [PATCH 7/8] powerpc/64s: idle do not hold reservation longer than required Nicholas Piggin
2017-03-16 12:43 ` Gautham R Shenoy
2017-03-16 12:55 ` Nicholas Piggin
2017-03-14 9:23 ` [PATCH 8/8] powerpc/64s: idle POWER8 avoid full state loss recovery when possible Nicholas Piggin
2017-03-16 16:12 ` Gautham R Shenoy
2017-03-17 5:24 ` Nicholas Piggin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170316131907.GF16462@in.ibm.com \
--to=ego@linux$(echo .)vnet.ibm.com \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=mahesh@linux$(echo .)vnet.ibm.com \
--cc=npiggin@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox