From: Nicholas Piggin <npiggin@gmail.com>
To: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>,
Michael Neuling <mikey@neuling.org>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
"Shreyas B. Prabhu" <shreyasbp@gmail.com>,
Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>,
Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>,
Anton Blanchard <anton@samba.org>,
Balbir Singh <bsingharora@gmail.com>,
Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [v2 PATCH 4/4] powernv: Recover correct PACA on wakeup from a stop on P9 DD1
Date: Wed, 22 Mar 2017 18:30:55 +1000 [thread overview]
Message-ID: <20170322183055.1ce9f6ed@roar.ozlabs.ibm.com> (raw)
In-Reply-To: <20170322055846.GB8326@in.ibm.com>
On Wed, 22 Mar 2017 11:28:46 +0530
Gautham R Shenoy <ego@linux.vnet.ibm.com> wrote:
> On Tue, Mar 21, 2017 at 02:59:46AM +1000, Nicholas Piggin wrote:
> > On Mon, 20 Mar 2017 21:24:18 +0530
> > "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> wrote:
> >
> > > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> > >
> > > POWER9 DD1.0 hardware has an issue due to which the SPRs of a thread
> > > waking up from stop 0,1,2 with ESL=1 can endup being misplaced in the
> > > core. Thus the HSPRG0 of a thread waking up from can contain the paca
> > > pointer of its sibling.
> > >
> > > This patch implements a context recovery framework within threads of a
> > > core, by provisioning space in paca_struct for saving every sibling
> > > threads's paca pointers. Basically, we should be able to arrive at the
> > > right paca pointer from any of the thread's existing paca pointer.
> > >
> > > At bootup, during powernv idle-init, we save the paca address of every
> > > CPU in each one its siblings paca_struct in the slot corresponding to
> > > this CPU's index in the core.
> > >
> > > On wakeup from a stop, the thread will determine its index in the core
> > > from the lower 2 bits of the PIR register and recover its PACA pointer
> > > by indexing into the correct slot in the provisioned space in the
> > > current PACA.
> > >
> > > Furthermore, ensure that the NVGPRs are restored from the stack on the
> > > way out by setting the NAPSTATELOST in paca.
> >
> > Thanks for expanding on this, it makes the patch easier to follow :)
> >
> > As noted before, I think if we use PACA_EXNMI for system reset, then
> > *hopefully* there should be minimal races with the initial use of other
> > thread's PACA at the start of the exception. So I'll work on getting
> > that in, but it need not prevent this patch from being merged first
> > IMO.
> >
> > > [Changelog written with inputs from svaidy@linux.vnet.ibm.com]
> > >
> > > Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> > > ---
> > > arch/powerpc/include/asm/paca.h | 5 ++++
> > > arch/powerpc/kernel/asm-offsets.c | 1 +
> > > arch/powerpc/kernel/idle_book3s.S | 49 ++++++++++++++++++++++++++++++++++-
> > > arch/powerpc/platforms/powernv/idle.c | 22 ++++++++++++++++
> > > 4 files changed, 76 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
> > > index 708c3e5..4405630 100644
> > > --- a/arch/powerpc/include/asm/paca.h
> > > +++ b/arch/powerpc/include/asm/paca.h
> > > @@ -172,6 +172,11 @@ struct paca_struct {
> > > u8 thread_mask;
> > > /* Mask to denote subcore sibling threads */
> > > u8 subcore_sibling_mask;
> > > + /*
> > > + * Pointer to an array which contains pointer
> > > + * to the sibling threads' paca.
> > > + */
> > > + struct paca_struct *thread_sibling_pacas[8];
>
> >
> > Is 8 the right number? I wonder if we have a define for it.
>
> Thats the maximum number of threads per core that we have had on POWER
> so far.
>
> Perhaps, I can make this
>
> struct paca_struct **thread_sibling_pacas;
>
> and allocate threads_per_core number of slots in
> pnv_init_idle_states. Sounds ok ?
I guess that would minimise PACA overhead for non-DD1 machines,
so if it's not too much trouble, that might be good.
> > > +power9_dd1_recover_paca:
> > > + mfspr r4, SPRN_PIR
> > > + clrldi r4, r4, 62
> >
> > Does SPRN_TIR work?
>
> I wasn't aware of SPRN_TIR!
>
> I can check this. If my reading of the ISA is correct, TIR should
> contain the thread number which are in the range [0..3].
Yep.
> > Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> >
>
> Thanks for reviewing the patch.
No problems. Don't worry about the machine check wakeup for the moment
either. It's more important to just get the normal wakeup fix in I think.
We can revisit what to do there after my machine check patches go in
(idle machine check does not really work right now for POWER9 anyway).
Thanks,
Nick
prev parent reply other threads:[~2017-03-22 8:31 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-20 15:54 [v2 PATCH 0/4] powernv:idle: Fixes for CPU-Hotplug on POWER DD1.0 Gautham R. Shenoy
2017-03-20 15:54 ` [v2 PATCH 1/4] powernv: Move CPU-Offline idle state invocation from smp.c to idle.c Gautham R. Shenoy
2017-03-20 16:35 ` Nicholas Piggin
2017-03-22 5:41 ` Gautham R Shenoy
2017-03-20 15:54 ` [v2 PATCH 2/4] powernv:smp: Add busy-wait loop as fall back for CPU-Hotplug Gautham R. Shenoy
2017-03-20 15:54 ` [v2 PATCH 3/4] powernv:idle: Don't override default/deepest directly in kernel Gautham R. Shenoy
2017-03-20 16:39 ` Nicholas Piggin
2017-03-22 5:43 ` Gautham R Shenoy
2017-03-20 15:54 ` [v2 PATCH 4/4] powernv: Recover correct PACA on wakeup from a stop on P9 DD1 Gautham R. Shenoy
2017-03-20 16:59 ` Nicholas Piggin
2017-03-22 3:29 ` Nicholas Piggin
2017-03-22 5:58 ` Gautham R Shenoy
2017-03-22 8:30 ` Nicholas Piggin [this message]
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=20170322183055.1ce9f6ed@roar.ozlabs.ibm.com \
--to=npiggin@gmail.com \
--cc=akshay.adiga@linux.vnet.ibm.com \
--cc=anton@samba.org \
--cc=benh@kernel.crashing.org \
--cc=bsingharora@gmail.com \
--cc=ego@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mikey@neuling.org \
--cc=mpe@ellerman.id.au \
--cc=shilpa.bhat@linux.vnet.ibm.com \
--cc=shreyasbp@gmail.com \
--cc=svaidy@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).