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


Groups > linux.kernel > #1653089

Re: [PATCH 2/6] powernv:idle: Decouple Timebase restore & Per-core SPRs restore

From Gautham R Shenoy <ego@linux.vnet.ibm.com>
Newsgroups linux.kernel
Subject Re: [PATCH 2/6] powernv:idle: Decouple Timebase restore & Per-core SPRs restore
Date 2017-05-30 12:30 +0200
Message-ID <tMMzw-677-23@gated-at.bofh.it> (permalink)
References <tHGuJ-3oA-3@gated-at.bofh.it> <tHGuJ-3oA-1@gated-at.bofh.it> <tMIFA-3sh-29@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Tue, May 30, 2017 at 04:12:38PM +1000, Nicholas Piggin wrote:
> On Tue, 16 May 2017 14:19:44 +0530
> "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> wrote:
> 
> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> > 
> > 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 for handling exit from a idle state 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 in such cases.
> > 
> > But that is no longer the case on POWER9 where we do have stop states
> > in which 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.
> > 
> > Fix this by ensuring that even 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), and if
> > this is the case, we restore the per-core hypervisor state.
> > 
> > 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 4898d67..afd029f 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S
> > @@ -731,13 +731,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
> >  
> >  	/*
> 
> It's more that timebase was not lost, rather than resynced.
> Can we just do a 'bgtl cr3,opal_resync_timebase'?

Yes, that's looks much better. I hadn't thought of bgtl. Will update
this.


> 
> I guess cr4 branch will never be true on POWER9... I think

Yes. Inside pnv_wakeup_tb_loss, cr4 will either have gt or
eq set. This branch applies only to POWER8 where cr4 would be eq only
on winkle, and not on fastsleep (a state that loses timebase but not
hypervisor state).


> pnv_wakeup_tb_loss is going to end up clearer being split
> into two between isa 207 and 300. But that can wait until
> after POWER9 is working properly.

Sure. 

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

--
Thanks and Regards
gautham.

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

Re: [PATCH 2/6] powernv:idle: Decouple Timebase restore & Per-core  SPRs restore Nicholas Piggin <npiggin@gmail.com> - 2017-05-30 08:20 +0200
  Re: [PATCH 2/6] powernv:idle: Decouple Timebase restore & Per-core  SPRs restore Gautham R Shenoy <ego@linux.vnet.ibm.com> - 2017-05-30 12:30 +0200

csiph-web