[PATCH] powerpc: Invalidate ERAT on powersave wakeup for POWER9

[PATCH] powerpc: Invalidate ERAT on powersave wakeup for POWER9

[Michael Neuling]

On POWER9 the ERAT may be incorrect on wakeup from some stop states
that lose state. This causes random segvs and illegal instructions
when these stop states are enabled.

This patch invalidates the ERAT on wakeup on POWER9 to prevent this
from causing a problem.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 arch/powerpc/kernel/idle_book3s.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 1ea14b96f1..ace2ad50c8 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -793,6 +793,9 @@ fastsleep_workaround_at_exit:
  */
 .global pnv_wakeup_loss
 pnv_wakeup_loss:
+BEGIN_FTR_SECTION
+	PPC_INVALIDATE_ERAT
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
 	ld	r1,PACAR1(r13)
 BEGIN_FTR_SECTION
 	CHECK_HMI_INTERRUPT
-- 
2.11.0

Re: [PATCH] powerpc: Invalidate ERAT on powersave wakeup for POWER9

[Stewart Smith]

Michael Neuling <mikey@neuling.org> writes:
> On POWER9 the ERAT may be incorrect on wakeup from some stop states
> that lose state. This causes random segvs and illegal instructions
> when these stop states are enabled.
>
> This patch invalidates the ERAT on wakeup on POWER9 to prevent this
> from causing a problem.
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>

CC: stable?

-- 
Stewart Smith
OPAL Architect, IBM.

Re: [PATCH] powerpc: Invalidate ERAT on powersave wakeup for POWER9

[Michael Neuling]

[-- Attachment #1: Type: text/plain, Size: 569 bytes --]

Yes please... Would be helpful


On 22 Jun. 2017 7:50 pm, "Stewart Smith" <stewart@linux.vnet.ibm.com> wrote:

> Michael Neuling <mikey@neuling.org> writes:
> > On POWER9 the ERAT may be incorrect on wakeup from some stop states
> > that lose state. This causes random segvs and illegal instructions
> > when these stop states are enabled.
> >
> > This patch invalidates the ERAT on wakeup on POWER9 to prevent this
> > from causing a problem.
> >
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
>
> CC: stable?
>
> --
> Stewart Smith
> OPAL Architect, IBM.
>
>

[-- Attachment #2: Type: text/html, Size: 1045 bytes --]

Re: [PATCH] powerpc: Invalidate ERAT on powersave wakeup for POWER9

[Michael Neuling]

On Fri, 2017-06-23 at 10:50 +1000, Stewart Smith wrote:
> Michael Neuling <mikey@neuling.org> writes:
> > On POWER9 the ERAT may be incorrect on wakeup from some stop states
> > that lose state. This causes random segvs and illegal instructions
> > when these stop states are enabled.
> >=20
> > This patch invalidates the ERAT on wakeup on POWER9 to prevent this
> > from causing a problem.
> >=20
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
>=20
> CC: stable?

Yes please!

Mikey

Re: [PATCH] powerpc: Invalidate ERAT on powersave wakeup for POWER9

[Michael Ellerman]

Michael Neuling <mikey@neuling.org> writes:

> On POWER9 the ERAT may be incorrect on wakeup from some stop states
> that lose state. This causes random segvs and illegal instructions
> when these stop states are enabled.

Incorrect how?

Because with the ERAT flush where you've put it, there's still a good
amount of code executed prior to the flush isn't there?

ie. we come in at 0x100, do some of the prolog, do IDLE_TEST which takes
us to pnv_powersave_wakeup, which then restores state from the paca
(memory), that returns and then we check KVM ... and then finally we end
up at pnv_wakeup_loss.

Or is there some other path? Or is the ERAT incorrect in some specific
way which means we only need to flush there?

cheers

> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 1ea14b96f1..ace2ad50c8 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -793,6 +793,9 @@ fastsleep_workaround_at_exit:
>   */
>  .global pnv_wakeup_loss
>  pnv_wakeup_loss:
> +BEGIN_FTR_SECTION
> +	PPC_INVALIDATE_ERAT
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
>  	ld	r1,PACAR1(r13)
>  BEGIN_FTR_SECTION
>  	CHECK_HMI_INTERRUPT
> -- 
> 2.11.0

Re: [PATCH] powerpc: Invalidate ERAT on powersave wakeup for POWER9

[Nicholas Piggin]

On Fri, 23 Jun 2017 19:33:23 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Michael Neuling <mikey@neuling.org> writes:
> 
> > On POWER9 the ERAT may be incorrect on wakeup from some stop states
> > that lose state. This causes random segvs and illegal instructions
> > when these stop states are enabled.  
> 
> Incorrect how?

It can have stale ERAT entries from another idle thread.

> 
> Because with the ERAT flush where you've put it, there's still a good
> amount of code executed prior to the flush isn't there?
> 
> ie. we come in at 0x100, do some of the prolog, do IDLE_TEST which takes
> us to pnv_powersave_wakeup, which then restores state from the paca
> (memory), that returns and then we check KVM ... and then finally we end
> up at pnv_wakeup_loss.

In the case of an HMI, we could call into OPAL as well.

> Or is there some other path? Or is the ERAT incorrect in some specific
> way which means we only need to flush there?

I think we're in real mode until returning from pnv_wakeup_loss so those
ERATs should be the same.

Except KVM, which can go to guest and switch on the MMU. My bad, I
suggested putting it into pnv_wakeup_loss.

Flushing at the start of pnv_powersave_wakeup should be safest. I guess
we can avoid it for non-state-loss wakeups if cr3 is lt.

Thanks,
Nick

Re: [PATCH] powerpc: Invalidate ERAT on powersave wakeup for POWER9

[Benjamin Herrenschmidt]

On Fri, 2017-06-23 at 19:33 +1000, Michael Ellerman wrote:
> Michael Neuling <mikey@neuling.org> writes:
> 
> > On POWER9 the ERAT may be incorrect on wakeup from some stop states
> > that lose state. This causes random segvs and illegal instructions
> > when these stop states are enabled.
> 
> Incorrect how?

As in stale. Not sure about the details.

> Because with the ERAT flush where you've put it, there's still a good
> amount of code executed prior to the flush isn't there?

In real mode, should be ok.

> ie. we come in at 0x100, do some of the prolog, do IDLE_TEST which takes
> us to pnv_powersave_wakeup, which then restores state from the paca
> (memory), that returns and then we check KVM ... and then finally we end
> up at pnv_wakeup_loss.
> 
> Or is there some other path? Or is the ERAT incorrect in some specific
> way which means we only need to flush there?

I think real mode translations are ok but I'll ask around.

Cheers,
Ben.

> cheers
> 
> > diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> > index 1ea14b96f1..ace2ad50c8 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S
> > @@ -793,6 +793,9 @@ fastsleep_workaround_at_exit:
> >   */
> >  .global pnv_wakeup_loss
> >  pnv_wakeup_loss:
> > +BEGIN_FTR_SECTION
> > +	PPC_INVALIDATE_ERAT
> > +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> >  	ld	r1,PACAR1(r13)
> >  BEGIN_FTR_SECTION
> >  	CHECK_HMI_INTERRUPT
> > -- 
> > 2.11.0

Re: [PATCH] powerpc: Invalidate ERAT on powersave wakeup for POWER9

[Anshuman Khandual]

On 06/22/2017 10:56 PM, Michael Neuling wrote:
> On POWER9 the ERAT may be incorrect on wakeup from some stop states
> that lose state. This causes random segvs and illegal instructions
> when these stop states are enabled.
> 
> This patch invalidates the ERAT on wakeup on POWER9 to prevent this
> from causing a problem.

Cant there be any real genuine ERAT error on the wake up path
from these states ? Just being curious.

[PATCH v2] powerpc: Invalidate ERAT on powersave wakeup for POWER9

[Benjamin Herrenschmidt]

On POWER9 the ERAT may be incorrect on wakeup from some stop states
that lose state. This causes random segvs and illegal instructions
when these stop states are enabled.

This patch invalidates the ERAT on wakeup on POWER9 to prevent this
from causing a problem.

Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

v2. [BenH] Move to a place before we branch off to KVM if the
           core was in a guest. Also add a comment about the
           SRR1 bit extraction.
---
 arch/powerpc/kernel/exceptions-64s.S | 4 +++-
 arch/powerpc/kernel/idle_book3s.S    | 7 +++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae418b85c17c..b4b2c3a344c4 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -99,7 +99,9 @@ EXC_VIRT_NONE(0x4000, 0x100)
 #ifdef CONFIG_PPC_P7_NAP
 	/*
 	 * If running native on arch 2.06 or later, check if we are waking up
-	 * from nap/sleep/winkle, and branch to idle handler.
+	 * from nap/sleep/winkle, and branch to idle handler. This tests
+	 * SRR1 bits 46:47. A non-0 value indicates that we are coming from
+	 * a power saving state.
 	 */
 #define IDLETEST(n)							\
 	BEGIN_FTR_SECTION ;						\
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 4898d676dcae..3fd65739e105 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -489,6 +489,13 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
  */
 pnv_restore_hyp_resource_arch300:
 	/*
+	 * Workaround for POWER9, if we lost resources, the ERAT
+	 * might have been mixed up and needs flushing.
+	 */
+	blt	cr3,1f
+	PPC_INVALIDATE_ERAT
+1:
+	/*
 	 * POWER ISA 3. Use PSSCR to determine if we
 	 * are waking up from deep idle state
 	 */

Re: [PATCH v2] powerpc: Invalidate ERAT on powersave wakeup for POWER9

[Nicholas Piggin]

On Sat, 24 Jun 2017 12:29:01 -0500
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On POWER9 the ERAT may be incorrect on wakeup from some stop states
> that lose state. This causes random segvs and illegal instructions
> when these stop states are enabled.
> 
> This patch invalidates the ERAT on wakeup on POWER9 to prevent this
> from causing a problem.
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> v2. [BenH] Move to a place before we branch off to KVM if the
>            core was in a guest. Also add a comment about the
>            SRR1 bit extraction.

This looks a bit safer to me now (avoiding KVM). My understanding is
the real-mode i- and d-ERAT entries are still valid and usable,
which is why this works.

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
>  arch/powerpc/kernel/exceptions-64s.S | 4 +++-
>  arch/powerpc/kernel/idle_book3s.S    | 7 +++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index ae418b85c17c..b4b2c3a344c4 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -99,7 +99,9 @@ EXC_VIRT_NONE(0x4000, 0x100)
>  #ifdef CONFIG_PPC_P7_NAP
>  	/*
>  	 * If running native on arch 2.06 or later, check if we are waking up
> -	 * from nap/sleep/winkle, and branch to idle handler.
> +	 * from nap/sleep/winkle, and branch to idle handler. This tests
> +	 * SRR1 bits 46:47. A non-0 value indicates that we are coming from
> +	 * a power saving state.
>  	 */
>  #define IDLETEST(n)							\
>  	BEGIN_FTR_SECTION ;						\
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 4898d676dcae..3fd65739e105 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -489,6 +489,13 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
>   */
>  pnv_restore_hyp_resource_arch300:
>  	/*
> +	 * Workaround for POWER9, if we lost resources, the ERAT
> +	 * might have been mixed up and needs flushing.
> +	 */
> +	blt	cr3,1f
> +	PPC_INVALIDATE_ERAT
> +1:
> +	/*
>  	 * POWER ISA 3. Use PSSCR to determine if we
>  	 * are waking up from deep idle state
>  	 */
> 

Re: [v2] powerpc: Invalidate ERAT on powersave wakeup for POWER9

[Michael Ellerman]

On Sat, 2017-06-24 at 17:29:01 UTC, Benjamin Herrenschmidt wrote:
> On POWER9 the ERAT may be incorrect on wakeup from some stop states
> that lose state. This causes random segvs and illegal instructions
> when these stop states are enabled.
> 
> This patch invalidates the ERAT on wakeup on POWER9 to prevent this
> from causing a problem.
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/ba6d334ac230065243a92bb7cb3fd6

cheers