Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1622036 > unrolled thread

[PATCH 2/3] powernv:idle: Decouple TB restore & Per-core SPRs restore

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/3] powernv:idle: Decouple TB restore & Per-core SPRs restore "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> - 2017-04-12 13:50 +0200
    Re: [PATCH 2/3] powernv:idle: Decouple TB restore & Per-core SPRs  restore Michael Neuling <mikey@neuling.org> - 2017-04-13 09:00 +0200
      Re: [PATCH 2/3] powernv:idle: Decouple TB restore & Per-core SPRs  restore Gautham R Shenoy <ego@linux.vnet.ibm.com> - 2017-04-13 14:00 +0200

#1622036 — [PATCH 2/3] powernv:idle: Decouple TB restore & Per-core SPRs restore

From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>

The idle-exit code assumes that if Timebase is not lost, then neither
are the per-core hypervisor resources lost. This was true on POWER8
where fast-sleep lost only TB but not per-core resources, and winkle
lost both.

This assumption is not true for POWER9 however, since there can be
states which do not lose timebase but can lose per-core SPRs.

Hence check if we need to restore the per-core hypervisor state even
if timebase is not lost.

Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/idle_book3s.S | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 9b747e9..6a9bd28 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -723,13 +723,14 @@ timebase_resync:
 	 * Use cr3 which indicates that we are waking up with atleast partial
 	 * hypervisor state loss to determine if TIMEBASE RESYNC is needed.
 	 */
-	ble	cr3,clear_lock
+	ble	cr3,.Ltb_resynced
 	/* Time base re-sync */
 	bl	opal_resync_timebase;
 	/*
-	 * If waking up from sleep, per core state is not lost, skip to
-	 * clear_lock.
+	 * If waking up from sleep (POWER8), per core state
+	 * is not lost, skip to clear_lock.
 	 */
+.Ltb_resynced:
 	blt	cr4,clear_lock
 
 	/*
-- 
1.9.4

[toc] | [next] | [standalone]

#1622754 — Re: [PATCH 2/3] powernv:idle: Decouple TB restore & Per-core SPRs restore

On Wed, 2017-04-12 at 17:16 +0530, Gautham R. Shenoy wrote:
> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> 
> The idle-exit code assumes that if Timebase is not lost, then neither
> are the per-core hypervisor resources lost. 

Double negative!  How about:

The idle-exit code assumes that if the timebase is restored, then the
per-core hypervisor resources are also restored.

> This was true on POWER8
> where fast-sleep lost only TB but not per-core resources, and winkle
> lost both.
> 
> This assumption is not true for POWER9 however, since there can be
> states which do not lose timebase but can lose per-core SPRs.
> 
> Hence check if we need to restore the per-core hypervisor state even
> if timebase is not lost.

I think I understand what you're doing, just seems awkwardly worded.

Is this actually what the patch is doing?  It seem to be just changing one
branch.

Mikey

> 
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/idle_book3s.S | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/idle_book3s.S
> b/arch/powerpc/kernel/idle_book3s.S
> index 9b747e9..6a9bd28 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -723,13 +723,14 @@ timebase_resync:
>  	 * Use cr3 which indicates that we are waking up with atleast partial
>  	 * hypervisor state loss to determine if TIMEBASE RESYNC is needed.
>  	 */
> -	ble	cr3,clear_lock
> +	ble	cr3,.Ltb_resynced
>  	/* Time base re-sync */
>  	bl	opal_resync_timebase;
>  	/*
> -	 * If waking up from sleep, per core state is not lost, skip to
> -	 * clear_lock.
> +	 * If waking up from sleep (POWER8), per core state
> +	 * is not lost, skip to clear_lock.
>  	 */
> +.Ltb_resynced:
>  	blt	cr4,clear_lock
>  
>  	/*

[toc] | [prev] | [next] | [standalone]

#1622954 — Re: [PATCH 2/3] powernv:idle: Decouple TB restore & Per-core SPRs restore

On Thu, Apr 13, 2017 at 04:55:45PM +1000, Michael Neuling wrote:
> On Wed, 2017-04-12 at 17:16 +0530, Gautham R. Shenoy wrote:
> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> > 
> > The idle-exit code assumes that if Timebase is not lost, then neither
> > are the per-core hypervisor resources lost. 
> 
> Double negative!  How about:
> 
> The idle-exit code assumes that if the timebase is restored, then the
> per-core hypervisor resources are also restored.

Ok. To be more explicit, this was the intention:

On POWER8, in case of 
   -  nap: both timebase and hypervisor state is retained.
   -  fast-sleep: timebase is lost. But the hypervisor state is retained.
   -  winkle: timebase and hypervisor state is lost.

Hence, the current code assumes that if the timebase value is
retained, then so is the hypervisor state. Thus, the current code
doesn't restore per-core hypervisor state if this is the case.

But that is no longer true on POWER9 where we do have stop states
where timebase value is retained, but the hypervisor state is lost. So
we have to ensure that the per-core hypervisor state gets restored in
such cases.

> 
> > This was true on POWER8
> > where fast-sleep lost only TB but not per-core resources, and winkle
> > lost both.
> > 
> > This assumption is not true for POWER9 however, since there can be
> > states which do not lose timebase but can lose per-core SPRs.
> > 
> > Hence check if we need to restore the per-core hypervisor state even
> > if timebase is not lost.
> 
> I think I understand what you're doing, just seems awkwardly worded.

Will fix the wording.

> 
> Is this actually what the patch is doing?  It seem to be just changing one
> branch.
>

Yes, we change the branch to .Ltb_resynced which will ensure in the
case when timebase is retained, we explicitly check if we are waking
up from a deep stop that loses per-core hypervisor state (indicated by
cr4 being eq or gt).

> Mikey
> 
> > 
> > Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> > ---
> >  arch/powerpc/kernel/idle_book3s.S | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/powerpc/kernel/idle_book3s.S
> > b/arch/powerpc/kernel/idle_book3s.S
> > index 9b747e9..6a9bd28 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S
> > @@ -723,13 +723,14 @@ timebase_resync:
> >  	 * Use cr3 which indicates that we are waking up with atleast partial
> >  	 * hypervisor state loss to determine if TIMEBASE RESYNC is needed.
> >  	 */
> > -	ble	cr3,clear_lock
> > +	ble	cr3,.Ltb_resynced
> >  	/* Time base re-sync */
> >  	bl	opal_resync_timebase;
> >  	/*
> > -	 * If waking up from sleep, per core state is not lost, skip to
> > -	 * clear_lock.
> > +	 * If waking up from sleep (POWER8), per core state
> > +	 * is not lost, skip to clear_lock.
> >  	 */
> > +.Ltb_resynced:
> >  	blt	cr4,clear_lock
> >  
> >  	/*

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web