From: Russell Currey <ruscur@russell.cc>
To: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc/powernv/idle: Restore IAMR after idle
Date: Wed, 20 Feb 2019 22:20:34 +1100 [thread overview]
Message-ID: <86ab88c86afeb99f8f4c8e1c2c590bf7541e7e40.camel@russell.cc> (raw)
In-Reply-To: <20190220085841.GA11633@aks.ibm>
On Wed, 2019-02-20 at 14:28 +0530, Akshay Adiga wrote:
> On Wed, Feb 06, 2019 at 05:28:37PM +1100, Russell Currey wrote:
> > Without restoring the IAMR after idle, execution prevention on
> > POWER9
> > with Radix MMU is overwritten and the kernel can freely execute
> > userspace without
> > faulting.
> >
> > This is necessary when returning from any stop state that modifies
> > user
> > state, as well as hypervisor state.
> >
> > To test how this fails without this patch, load the lkdtm driver
> > and
> > do the following:
> >
> > echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
> >
> > which won't fault, then boot the kernel with powersave=off, where
> > it
> > will fault. Applying this patch will fix this.
> >
> > Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of
> > user
> > space")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > ---
> > arch/powerpc/include/asm/cpuidle.h | 1 +
> > arch/powerpc/kernel/asm-offsets.c | 1 +
> > arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
> > 3 files changed, 22 insertions(+)
> >
> > diff --git a/arch/powerpc/include/asm/cpuidle.h
> > b/arch/powerpc/include/asm/cpuidle.h
> > index 43e5f31fe64d..ad67dbe59498 100644
> > --- a/arch/powerpc/include/asm/cpuidle.h
> > +++ b/arch/powerpc/include/asm/cpuidle.h
> > @@ -77,6 +77,7 @@ struct stop_sprs {
> > u64 mmcr1;
> > u64 mmcr2;
> > u64 mmcra;
> > + u64 iamr;
> > };
> >
> > #define PNV_IDLE_NAME_LEN 16
> > diff --git a/arch/powerpc/kernel/asm-offsets.c
> > b/arch/powerpc/kernel/asm-offsets.c
> > index 9ffc72ded73a..10e0314c2b0d 100644
> > --- a/arch/powerpc/kernel/asm-offsets.c
> > +++ b/arch/powerpc/kernel/asm-offsets.c
> > @@ -774,6 +774,7 @@ int main(void)
> > STOP_SPR(STOP_MMCR1, mmcr1);
> > STOP_SPR(STOP_MMCR2, mmcr2);
> > STOP_SPR(STOP_MMCRA, mmcra);
> > + STOP_SPR(STOP_IAMR, iamr);
> > #endif
> >
> > DEFINE(PPC_DBELL_SERVER, PPC_DBELL_SERVER);
> > diff --git a/arch/powerpc/kernel/idle_book3s.S
> > b/arch/powerpc/kernel/idle_book3s.S
> > index 7f5ac2e8581b..bb4f552f6c7e 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S
> > @@ -200,6 +200,12 @@ pnv_powersave_common:
> > /* Continue saving state */
> > SAVE_GPR(2, r1)
> > SAVE_NVGPRS(r1)
> > +
> > +BEGIN_FTR_SECTION
> > + mfspr r5, SPRN_IAMR
> > + std r5, STOP_IAMR(r13)
> > +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> > +
> > mfcr r5
> > std r5,_CCR(r1)
> > std r1,PACAR1(r13)
> > @@ -924,6 +930,13 @@ BEGIN_FTR_SECTION
> > END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> > REST_NVGPRS(r1)
> > REST_GPR(2, r1)
> > +
> > +BEGIN_FTR_SECTION
> > + ld r4, STOP_IAMR(r13)
> > + mtspr SPRN_IAMR, r4
> > + isync
> > +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> > +
> > ld r4,PACAKMSR(r13)
> > ld r5,_LINK(r1)
> > ld r6,_CCR(r1)
> > @@ -946,6 +959,13 @@ pnv_wakeup_noloss:
> > BEGIN_FTR_SECTION
> > CHECK_HMI_INTERRUPT
> > END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> > +
> > +BEGIN_FTR_SECTION
> > + ld r4, STOP_IAMR(r13)
> > + mtspr SPRN_IAMR, r4
> > + isync
> > +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> > +
>
> pnv_wakeup_noloss gets called from two paths:
> 1) cpu wakes up at 0x100 (ESL=EC=1)
> 2) cpu wakes up at next instruction (ESL=EC=0)
>
In v2 I drop it from noloss, is that still correct?
> We know for the fact that its not lost with ESL=EC=0 , so we
> should put it somewhere else.
>
> I would like to put the restore code in
> pnv_restore_hyp_resource_arch300
> it already has some work arounds we can add another.
>
> By this time we cr3 has comparison of SRR1[46:47] with 2
>
> 542 pnv_restore_hyp_resource_arch300:
> 543 /*
> 544 * Workaround for POWER9, if we lost resources, the
> ERAT
> 545 * might have been mixed up and needs flushing. We also
> need
> 546 * to reload MMCR0 (see comment above). We also need to
> set
> 547 * then clear bit 60 in MMCRA to ensure the PMU starts
> running.
> 548 */
> 549 blt cr3,1f
> 550 BEGIN_FTR_SECTION
> 551 PPC_INVALIDATE_ERAT
> 552 ld r1,PACAR1(r13)
> 553 ld r4,_MMCR0(r1)
> 554 mtspr SPRN_MMCR0,r4
> 555 END_FTR_SECTION_IFCLR(CPU_FTR_POWER9_DD2_1)
> 556 mfspr r4,SPRN_MMCRA
> 557 ori r4,r4,(1 << (63-60))
> 558 mtspr SPRN_MMCRA,r4
> 559 xori r4,r4,(1 << (63-60))
> 560 mtspr SPRN_MMCRA,r4
> + 561 BEGIN_FTR_SECTION
> + 562 ld r4,STOP_IAMR(r13)
> + 563 mtspr SPRN_IAMR,r4
> + 564 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> 565 1:
> 566 /*
>
> I have a patchset to handle both AMOR and IAMR,
> need to test it on power8 before posting.
If you think that's better, go ahead.
>
>
> > ld r4,PACAKMSR(r13)
> > ld r5,_NIP(r1)
> > ld r6,_CCR(r1)
> > --
> > 2.20.1
> >
prev parent reply other threads:[~2019-02-20 11:25 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-06 6:28 [PATCH] powerpc/powernv/idle: Restore IAMR after idle Russell Currey
2019-02-07 4:29 ` Michael Ellerman
2019-02-07 6:28 ` Russell Currey
2019-02-07 5:08 ` Nicholas Piggin
2019-02-07 6:33 ` Russell Currey
2019-02-07 16:37 ` Thiago Jung Bauermann
2019-02-07 22:38 ` Russell Currey
2019-02-08 1:04 ` Michael Ellerman
2019-02-19 4:21 ` Nicholas Piggin
2019-02-20 6:04 ` Akshay Adiga
2019-02-20 11:18 ` Russell Currey
2019-02-20 7:15 ` Akshay Adiga
2019-02-20 11:25 ` Russell Currey
2019-02-20 8:58 ` Akshay Adiga
2019-02-20 11:20 ` Russell Currey [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=86ab88c86afeb99f8f4c8e1c2c590bf7541e7e40.camel@russell.cc \
--to=ruscur@russell.cc \
--cc=akshay.adiga@linux.vnet.ibm.com \
--cc=linuxppc-dev@lists.ozlabs.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.