Re: [RESEND][PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Vaidyanathan Srinivasan]

* Benjamin Herrenschmidt  [2018-03-01 08:40:22]:

> On Thu, 2018-03-01 at 01:03 +0530, Akshay Adiga wrote:
> > commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> > states via stop API.") uses stop-api provided by the firmware to restore
> > PSSCR. PSSCR restore is required for handling special wakeup. When special
> > wakeup is completed, the core enters stop state based on restored PSSCR.
> > 
> > Currently PSSCR is restored to deepest available stop state, causing
> > a idle cpu to enter deeper stop state on a special wakeup, which causes
> > the cpu to hang on wakeup.
> > 
> > A "sensors" command which reads temperature (through DTS sensors) on idle
> > cpu can trigger special wakeup.
> > 
> > Failed Scenario :
> > Request restore of PSSCR with RL = 11
> > cpu enters idle state (stop5)
> >   user triggers "sensors" command
> >Assert special wakeup on cpu
> >  Restores PSSCR with RL = 11  < Done by firmware
> >   Read DTS sensor
> >Deassert special wakeup
> >   cpu enters idle state (stop11) <-- Instead of stop5
> > 
> > Cpu hang is caused because cpu ended up in a deeper state than it requested
> > 
> > This patch fixes instability caused by special wakeup when stop11 is
> > enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
> > Only when offlining cpu, request restore of PSSCR to deepest stop state.
> > On onlining cpu, request restore of PSSCR to deepest stop state used by
> > cpuidle.
> 
> So if we chose a stop state, but somebody else does a special wakeup,
> we'll end up going back into a *deeper* one than the one we came from ?

Unfortunately yes.  This is the current limitation.  If we are in stop4
and above and we had not set a PSSCR to be restored, then the hardware
will default to all bits set (stop15) leading to entry into stop11
after the special wakeup is removed.  The requirement is such that we
need to have a correct PSSCR restore value set using stop-api.

We need to set a restore PSSCR value that represents one in a group
like stop4,5,6,7 will have identical state loss, hence we can either
set a PSSCR of 7 or 4 or 5 for any of this stop state entry and not
have to use stop-api to set exact value of stop4 or 5 at every entry.
 
> I still think this is broken by design. The chip should automatically
> go back to the state it went to after special wakeup, thus the PPE
> controlling the state should override the PSSCR value accordingly
> rather than relying on those SW hoops.

Special wakeup de-assertion and re-entry hits this limitation where we
have lost the original content of PSSCR SPR and hence CME does not know
what was requested.

Additional stop-api calls from software could have been avoided, but
practically we have only cpu hotplug case that uses stop11 and needs
this stop-api.  We can default the system to stop4 or stop5 and then
make stop-api call to explicitly set stop11 and then hotplug out the
cpu. We have to restore the deepest cpuidle state (stop4/5) back
during online.  As such this is not much of software overhead. But we
need an elegant method to control these calls from OPAL flags so that
kernel behaviour can be more closely controlled.

If we want to use stop11 in cpuidle (despite being very slow) for
evaluation reasons, then we will need to make more stop-api call to
choose between stop4/5 vs stop11 since they belong to different group.
Even in this case, since stop11 is the slow path, we would want to set
stop11 before entry and restore to stop4/5 after wakeup.  This way we
still completely avoid stop-api call in fast-path stop4/5 entry/exit.

--Vaidy

Re: [RESEND][PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Vaidyanathan Srinivasan]

* Benjamin Herrenschmidt  [2018-03-01 08:40:22]:

> On Thu, 2018-03-01 at 01:03 +0530, Akshay Adiga wrote:
> > commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> > states via stop API.") uses stop-api provided by the firmware to restore
> > PSSCR. PSSCR restore is required for handling special wakeup. When special
> > wakeup is completed, the core enters stop state based on restored PSSCR.
> > 
> > Currently PSSCR is restored to deepest available stop state, causing
> > a idle cpu to enter deeper stop state on a special wakeup, which causes
> > the cpu to hang on wakeup.
> > 
> > A "sensors" command which reads temperature (through DTS sensors) on idle
> > cpu can trigger special wakeup.
> > 
> > Failed Scenario :
> > Request restore of PSSCR with RL = 11
> > cpu enters idle state (stop5)
> >   user triggers "sensors" command
> >Assert special wakeup on cpu
> >  Restores PSSCR with RL = 11  < Done by firmware
> >   Read DTS sensor
> >Deassert special wakeup
> >   cpu enters idle state (stop11) <-- Instead of stop5
> > 
> > Cpu hang is caused because cpu ended up in a deeper state than it requested
> > 
> > This patch fixes instability caused by special wakeup when stop11 is
> > enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
> > Only when offlining cpu, request restore of PSSCR to deepest stop state.
> > On onlining cpu, request restore of PSSCR to deepest stop state used by
> > cpuidle.
> 
> So if we chose a stop state, but somebody else does a special wakeup,
> we'll end up going back into a *deeper* one than the one we came from ?

Unfortunately yes.  This is the current limitation.  If we are in stop4
and above and we had not set a PSSCR to be restored, then the hardware
will default to all bits set (stop15) leading to entry into stop11
after the special wakeup is removed.  The requirement is such that we
need to have a correct PSSCR restore value set using stop-api.

We need to set a restore PSSCR value that represents one in a group
like stop4,5,6,7 will have identical state loss, hence we can either
set a PSSCR of 7 or 4 or 5 for any of this stop state entry and not
have to use stop-api to set exact value of stop4 or 5 at every entry.
 
> I still think this is broken by design. The chip should automatically
> go back to the state it went to after special wakeup, thus the PPE
> controlling the state should override the PSSCR value accordingly
> rather than relying on those SW hoops.

Special wakeup de-assertion and re-entry hits this limitation where we
have lost the original content of PSSCR SPR and hence CME does not know
what was requested.

Additional stop-api calls from software could have been avoided, but
practically we have only cpu hotplug case that uses stop11 and needs
this stop-api.  We can default the system to stop4 or stop5 and then
make stop-api call to explicitly set stop11 and then hotplug out the
cpu. We have to restore the deepest cpuidle state (stop4/5) back
during online.  As such this is not much of software overhead. But we
need an elegant method to control these calls from OPAL flags so that
kernel behaviour can be more closely controlled.

If we want to use stop11 in cpuidle (despite being very slow) for
evaluation reasons, then we will need to make more stop-api call to
choose between stop4/5 vs stop11 since they belong to different group.
Even in this case, since stop11 is the slow path, we would want to set
stop11 before entry and restore to stop4/5 after wakeup.  This way we
still completely avoid stop-api call in fast-path stop4/5 entry/exit.

--Vaidy

Re: [PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Michael Ellerman]

Stewart Smith  writes:

> Akshay Adiga  writes:
>> commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
>> states via stop API.") uses stop-api provided by the firmware to restore
>> PSSCR. PSSCR restore is required for handling special wakeup. When special
>> wakeup is completed, the core enters stop state based on restored PSSCR.
>>
>> Currently PSSCR is restored to deepest available stop state, causing
>> a idle cpu to enter deeper stop state on a special wakeup, which causes
>> the cpu to hang on wakeup.
>>
>> A "sensors" command which reads temperature (through DTS sensors) on idle
>> cpu can trigger special wakeup.
>>
>> Failed Scenario :
>> Request restore of PSSCR with RL = 11
>> cpu enters idle state (stop5)
>>   user triggers "sensors" command
>>Assert special wakeup on cpu
>>  Restores PSSCR with RL = 11  < Done by firmware
>>   Read DTS sensor
>>Deassert special wakeup
>>   cpu enters idle state (stop11) <-- Instead of stop5
>>
>> Cpu hang is caused because cpu ended up in a deeper state than it requested
>>
>> This patch fixes instability caused by special wakeup when stop11 is
>> enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
>> Only when offlining cpu, request restore of PSSCR to deepest stop state.
>> On onlining cpu, request restore of PSSCR to deepest stop state used by
>> cpuidle.
>>
>> Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
>> states via stop API.")
>
> This should CC stable ?
>
> We'll need this to enable stop11 in firmware and not break things, right?

But is Cc'ing this to stable even sufficient to enable stop11?

If firmware starts enabling stop11 then every existing kernel will be
broken, so eg. bisecting anything prior to now will be impossible.

cheers

Re: [PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Michael Ellerman]

Stewart Smith  writes:

> Akshay Adiga  writes:
>> commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
>> states via stop API.") uses stop-api provided by the firmware to restore
>> PSSCR. PSSCR restore is required for handling special wakeup. When special
>> wakeup is completed, the core enters stop state based on restored PSSCR.
>>
>> Currently PSSCR is restored to deepest available stop state, causing
>> a idle cpu to enter deeper stop state on a special wakeup, which causes
>> the cpu to hang on wakeup.
>>
>> A "sensors" command which reads temperature (through DTS sensors) on idle
>> cpu can trigger special wakeup.
>>
>> Failed Scenario :
>> Request restore of PSSCR with RL = 11
>> cpu enters idle state (stop5)
>>   user triggers "sensors" command
>>Assert special wakeup on cpu
>>  Restores PSSCR with RL = 11  < Done by firmware
>>   Read DTS sensor
>>Deassert special wakeup
>>   cpu enters idle state (stop11) <-- Instead of stop5
>>
>> Cpu hang is caused because cpu ended up in a deeper state than it requested
>>
>> This patch fixes instability caused by special wakeup when stop11 is
>> enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
>> Only when offlining cpu, request restore of PSSCR to deepest stop state.
>> On onlining cpu, request restore of PSSCR to deepest stop state used by
>> cpuidle.
>>
>> Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
>> states via stop API.")
>
> This should CC stable ?
>
> We'll need this to enable stop11 in firmware and not break things, right?

But is Cc'ing this to stable even sufficient to enable stop11?

If firmware starts enabling stop11 then every existing kernel will be
broken, so eg. bisecting anything prior to now will be impossible.

cheers

Re: [PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Michael Ellerman]

Akshay Adiga  writes:

> On Mon, Feb 26, 2018 at 03:47:12PM +1100, Stewart Smith wrote:
>> Akshay Adiga  writes:
>> > commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
>> > states via stop API.") uses stop-api provided by the firmware to restore
>> > PSSCR. PSSCR restore is required for handling special wakeup. When special
>> > wakeup is completed, the core enters stop state based on restored PSSCR.
>> >
>> > Currently PSSCR is restored to deepest available stop state, causing
>> > a idle cpu to enter deeper stop state on a special wakeup, which causes
>> > the cpu to hang on wakeup.
>> >
>> > A "sensors" command which reads temperature (through DTS sensors) on idle
>> > cpu can trigger special wakeup.
>> >
>> > Failed Scenario :
>> > Request restore of PSSCR with RL = 11
>> > cpu enters idle state (stop5)
>> >   user triggers "sensors" command
>> >Assert special wakeup on cpu
>> >  Restores PSSCR with RL = 11  < Done by firmware
>> >   Read DTS sensor
>> >Deassert special wakeup
>> >   cpu enters idle state (stop11) <-- Instead of stop5
>> >
>> > Cpu hang is caused because cpu ended up in a deeper state than it requested
>> >
>> > This patch fixes instability caused by special wakeup when stop11 is
>> > enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
>> > Only when offlining cpu, request restore of PSSCR to deepest stop state.
>> > On onlining cpu, request restore of PSSCR to deepest stop state used by
>> > cpuidle.
>> >
>> > Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
>> > states via stop API.")
>> 
>> This should CC stable ?
>> 
>> We'll need this to enable stop11 in firmware and not break things, right?
>
> Yes I will resend and CC it to stable.

That's not how patches get to stable.

You tag it with "Cc: stable@vger ...", you don't actually email it to
sta...@vger.kernel.org.

cheers

Re: [PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Michael Ellerman]

Akshay Adiga  writes:

> On Mon, Feb 26, 2018 at 03:47:12PM +1100, Stewart Smith wrote:
>> Akshay Adiga  writes:
>> > commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
>> > states via stop API.") uses stop-api provided by the firmware to restore
>> > PSSCR. PSSCR restore is required for handling special wakeup. When special
>> > wakeup is completed, the core enters stop state based on restored PSSCR.
>> >
>> > Currently PSSCR is restored to deepest available stop state, causing
>> > a idle cpu to enter deeper stop state on a special wakeup, which causes
>> > the cpu to hang on wakeup.
>> >
>> > A "sensors" command which reads temperature (through DTS sensors) on idle
>> > cpu can trigger special wakeup.
>> >
>> > Failed Scenario :
>> > Request restore of PSSCR with RL = 11
>> > cpu enters idle state (stop5)
>> >   user triggers "sensors" command
>> >Assert special wakeup on cpu
>> >  Restores PSSCR with RL = 11  < Done by firmware
>> >   Read DTS sensor
>> >Deassert special wakeup
>> >   cpu enters idle state (stop11) <-- Instead of stop5
>> >
>> > Cpu hang is caused because cpu ended up in a deeper state than it requested
>> >
>> > This patch fixes instability caused by special wakeup when stop11 is
>> > enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
>> > Only when offlining cpu, request restore of PSSCR to deepest stop state.
>> > On onlining cpu, request restore of PSSCR to deepest stop state used by
>> > cpuidle.
>> >
>> > Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
>> > states via stop API.")
>> 
>> This should CC stable ?
>> 
>> We'll need this to enable stop11 in firmware and not break things, right?
>
> Yes I will resend and CC it to stable.

That's not how patches get to stable.

You tag it with "Cc: stable@vger ...", you don't actually email it to
sta...@vger.kernel.org.

cheers

Re: [RESEND][PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Benjamin Herrenschmidt]

On Thu, 2018-03-01 at 01:03 +0530, Akshay Adiga wrote:
> commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> states via stop API.") uses stop-api provided by the firmware to restore
> PSSCR. PSSCR restore is required for handling special wakeup. When special
> wakeup is completed, the core enters stop state based on restored PSSCR.
> 
> Currently PSSCR is restored to deepest available stop state, causing
> a idle cpu to enter deeper stop state on a special wakeup, which causes
> the cpu to hang on wakeup.
> 
> A "sensors" command which reads temperature (through DTS sensors) on idle
> cpu can trigger special wakeup.
> 
> Failed Scenario :
> Request restore of PSSCR with RL = 11
> cpu enters idle state (stop5)
>   user triggers "sensors" command
>Assert special wakeup on cpu
>  Restores PSSCR with RL = 11  < Done by firmware
>   Read DTS sensor
>Deassert special wakeup
>   cpu enters idle state (stop11) <-- Instead of stop5
> 
> Cpu hang is caused because cpu ended up in a deeper state than it requested
> 
> This patch fixes instability caused by special wakeup when stop11 is
> enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
> Only when offlining cpu, request restore of PSSCR to deepest stop state.
> On onlining cpu, request restore of PSSCR to deepest stop state used by
> cpuidle.

So if we chose a stop state, but somebody else does a special wakeup,
we'll end up going back into a *deeper* one than the one we came from ?

I still think this is broken by design. The chip should automatically
go back to the state it went to after special wakeup, thus the PPE
controlling the state should override the PSSCR value accordingly
rather than relying on those SW hoops.

> 
> Cc:  # v4.14+
> Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> states via stop API.")
> Reported-by: Pridhiviraj Paidipeddi 
> Signed-off-by: Akshay Adiga 
> ---
>  arch/powerpc/include/asm/cpuidle.h|  2 ++
>  arch/powerpc/platforms/powernv/idle.c | 46 
> ---
>  drivers/cpuidle/cpuidle-powernv.c |  1 -
>  3 files changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/cpuidle.h 
> b/arch/powerpc/include/asm/cpuidle.h
> index e210a83..f52e9f1 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -67,6 +67,8 @@
>  #define ERR_EC_ESL_MISMATCH  -1
>  #define ERR_DEEP_STATE_ESL_MISMATCH  -2
>  
> +#define POWERNV_THRESHOLD_LATENCY_NS 20
> +
>  #ifndef __ASSEMBLY__
>  /* Additional SPRs that need to be saved/restored during stop */
>  struct stop_sprs {
> diff --git a/arch/powerpc/platforms/powernv/idle.c 
> b/arch/powerpc/platforms/powernv/idle.c
> index 443d5ca..4b0c7d24 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -56,8 +56,11 @@ u64 pnv_first_deep_stop_state = MAX_STOP_STATE;
>   */
>  static u64 pnv_deepest_stop_psscr_val;
>  static u64 pnv_deepest_stop_psscr_mask;
> +static u64 pnv_deepest_cpuidle_psscr_val;
> +static u64 pnv_deepest_cpuidle_psscr_mask;
>  static u64 pnv_deepest_stop_flag;
>  static bool deepest_stop_found;
> +static bool deepest_cpuidle_found;
>  
>  static int pnv_save_sprs_for_deep_states(void)
>  {
> @@ -76,7 +79,14 @@ static int pnv_save_sprs_for_deep_states(void)
>   uint64_t hid5_val = mfspr(SPRN_HID5);
>   uint64_t hmeer_val = mfspr(SPRN_HMEER);
>   uint64_t msr_val = MSR_IDLE;
> - uint64_t psscr_val = pnv_deepest_stop_psscr_val;
> +
> + /*
> +  * Pick deepest cpuidle psscr as the value to be
> +  * restored through wakeup engine.
> +  * We will request a deeper state to be restored
> +  * in hotplug path
> +  */
> + uint64_t psscr_val = pnv_deepest_cpuidle_psscr_val;
>  
>   for_each_possible_cpu(cpu) {
>   uint64_t pir = get_hard_smp_processor_id(cpu);
> @@ -409,7 +419,7 @@ static void pnv_program_cpu_hotplug_lpcr(unsigned int 
> cpu, u64 lpcr_val)
>   */
>  unsigned long pnv_cpu_offline(unsigned int cpu)
>  {
> - unsigned long srr1;
> + u64 srr1;
>   u32 idle_states = pnv_get_supported_cpuidle_states();
>   u64 lpcr_val;
>  
> @@ -429,12 +439,18 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
>   __ppc64_runlatch_off();
>  
>   if (cpu_has_feature(CPU_FTR_ARCH_300) && deepest_stop_found) {
> - unsigned long psscr;
> + u64 psscr;
> + u64 pir = get_hard_smp_processor_id(cpu);
>  
>   psscr = mfspr(SPRN_PSSCR);
>   psscr = (psscr & ~pnv_deepest_stop_psscr_mask) |
>   pnv_deepest_stop_psscr_val;
> + if (pnv_deepest_stop_psscr_val != pnv_deepest_cpuidle_psscr_val)
> + opal_slw_set_reg(pir, P9_STOP_SPR_PSSCR, psscr);
>   srr1 = 

Re: [RESEND][PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Benjamin Herrenschmidt]

On Thu, 2018-03-01 at 01:03 +0530, Akshay Adiga wrote:
> commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> states via stop API.") uses stop-api provided by the firmware to restore
> PSSCR. PSSCR restore is required for handling special wakeup. When special
> wakeup is completed, the core enters stop state based on restored PSSCR.
> 
> Currently PSSCR is restored to deepest available stop state, causing
> a idle cpu to enter deeper stop state on a special wakeup, which causes
> the cpu to hang on wakeup.
> 
> A "sensors" command which reads temperature (through DTS sensors) on idle
> cpu can trigger special wakeup.
> 
> Failed Scenario :
> Request restore of PSSCR with RL = 11
> cpu enters idle state (stop5)
>   user triggers "sensors" command
>Assert special wakeup on cpu
>  Restores PSSCR with RL = 11  < Done by firmware
>   Read DTS sensor
>Deassert special wakeup
>   cpu enters idle state (stop11) <-- Instead of stop5
> 
> Cpu hang is caused because cpu ended up in a deeper state than it requested
> 
> This patch fixes instability caused by special wakeup when stop11 is
> enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
> Only when offlining cpu, request restore of PSSCR to deepest stop state.
> On onlining cpu, request restore of PSSCR to deepest stop state used by
> cpuidle.

So if we chose a stop state, but somebody else does a special wakeup,
we'll end up going back into a *deeper* one than the one we came from ?

I still think this is broken by design. The chip should automatically
go back to the state it went to after special wakeup, thus the PPE
controlling the state should override the PSSCR value accordingly
rather than relying on those SW hoops.

> 
> Cc:  # v4.14+
> Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> states via stop API.")
> Reported-by: Pridhiviraj Paidipeddi 
> Signed-off-by: Akshay Adiga 
> ---
>  arch/powerpc/include/asm/cpuidle.h|  2 ++
>  arch/powerpc/platforms/powernv/idle.c | 46 
> ---
>  drivers/cpuidle/cpuidle-powernv.c |  1 -
>  3 files changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/cpuidle.h 
> b/arch/powerpc/include/asm/cpuidle.h
> index e210a83..f52e9f1 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -67,6 +67,8 @@
>  #define ERR_EC_ESL_MISMATCH  -1
>  #define ERR_DEEP_STATE_ESL_MISMATCH  -2
>  
> +#define POWERNV_THRESHOLD_LATENCY_NS 20
> +
>  #ifndef __ASSEMBLY__
>  /* Additional SPRs that need to be saved/restored during stop */
>  struct stop_sprs {
> diff --git a/arch/powerpc/platforms/powernv/idle.c 
> b/arch/powerpc/platforms/powernv/idle.c
> index 443d5ca..4b0c7d24 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -56,8 +56,11 @@ u64 pnv_first_deep_stop_state = MAX_STOP_STATE;
>   */
>  static u64 pnv_deepest_stop_psscr_val;
>  static u64 pnv_deepest_stop_psscr_mask;
> +static u64 pnv_deepest_cpuidle_psscr_val;
> +static u64 pnv_deepest_cpuidle_psscr_mask;
>  static u64 pnv_deepest_stop_flag;
>  static bool deepest_stop_found;
> +static bool deepest_cpuidle_found;
>  
>  static int pnv_save_sprs_for_deep_states(void)
>  {
> @@ -76,7 +79,14 @@ static int pnv_save_sprs_for_deep_states(void)
>   uint64_t hid5_val = mfspr(SPRN_HID5);
>   uint64_t hmeer_val = mfspr(SPRN_HMEER);
>   uint64_t msr_val = MSR_IDLE;
> - uint64_t psscr_val = pnv_deepest_stop_psscr_val;
> +
> + /*
> +  * Pick deepest cpuidle psscr as the value to be
> +  * restored through wakeup engine.
> +  * We will request a deeper state to be restored
> +  * in hotplug path
> +  */
> + uint64_t psscr_val = pnv_deepest_cpuidle_psscr_val;
>  
>   for_each_possible_cpu(cpu) {
>   uint64_t pir = get_hard_smp_processor_id(cpu);
> @@ -409,7 +419,7 @@ static void pnv_program_cpu_hotplug_lpcr(unsigned int 
> cpu, u64 lpcr_val)
>   */
>  unsigned long pnv_cpu_offline(unsigned int cpu)
>  {
> - unsigned long srr1;
> + u64 srr1;
>   u32 idle_states = pnv_get_supported_cpuidle_states();
>   u64 lpcr_val;
>  
> @@ -429,12 +439,18 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
>   __ppc64_runlatch_off();
>  
>   if (cpu_has_feature(CPU_FTR_ARCH_300) && deepest_stop_found) {
> - unsigned long psscr;
> + u64 psscr;
> + u64 pir = get_hard_smp_processor_id(cpu);
>  
>   psscr = mfspr(SPRN_PSSCR);
>   psscr = (psscr & ~pnv_deepest_stop_psscr_mask) |
>   pnv_deepest_stop_psscr_val;
> + if (pnv_deepest_stop_psscr_val != pnv_deepest_cpuidle_psscr_val)
> + opal_slw_set_reg(pir, P9_STOP_SPR_PSSCR, psscr);
>   srr1 = power9_idle_stop(psscr);
> + psscr = (psscr & ~pnv_deepest_cpuidle_psscr_mask) |

[RESEND][PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Akshay Adiga]

commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
states via stop API.") uses stop-api provided by the firmware to restore
PSSCR. PSSCR restore is required for handling special wakeup. When special
wakeup is completed, the core enters stop state based on restored PSSCR.

Currently PSSCR is restored to deepest available stop state, causing
a idle cpu to enter deeper stop state on a special wakeup, which causes
the cpu to hang on wakeup.

A "sensors" command which reads temperature (through DTS sensors) on idle
cpu can trigger special wakeup.

Failed Scenario :
Request restore of PSSCR with RL = 11
cpu enters idle state (stop5)
  user triggers "sensors" command
   Assert special wakeup on cpu
 Restores PSSCR with RL = 11  < Done by firmware
  Read DTS sensor
   Deassert special wakeup
  cpu enters idle state (stop11) <-- Instead of stop5

Cpu hang is caused because cpu ended up in a deeper state than it requested

This patch fixes instability caused by special wakeup when stop11 is
enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
Only when offlining cpu, request restore of PSSCR to deepest stop state.
On onlining cpu, request restore of PSSCR to deepest stop state used by
cpuidle.

Cc:  # v4.14+
Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
states via stop API.")
Reported-by: Pridhiviraj Paidipeddi 
Signed-off-by: Akshay Adiga 
---
 arch/powerpc/include/asm/cpuidle.h|  2 ++
 arch/powerpc/platforms/powernv/idle.c | 46 ---
 drivers/cpuidle/cpuidle-powernv.c |  1 -
 3 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/cpuidle.h 
b/arch/powerpc/include/asm/cpuidle.h
index e210a83..f52e9f1 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -67,6 +67,8 @@
 #define ERR_EC_ESL_MISMATCH-1
 #define ERR_DEEP_STATE_ESL_MISMATCH-2
 
+#define POWERNV_THRESHOLD_LATENCY_NS 20
+
 #ifndef __ASSEMBLY__
 /* Additional SPRs that need to be saved/restored during stop */
 struct stop_sprs {
diff --git a/arch/powerpc/platforms/powernv/idle.c 
b/arch/powerpc/platforms/powernv/idle.c
index 443d5ca..4b0c7d24 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -56,8 +56,11 @@ u64 pnv_first_deep_stop_state = MAX_STOP_STATE;
  */
 static u64 pnv_deepest_stop_psscr_val;
 static u64 pnv_deepest_stop_psscr_mask;
+static u64 pnv_deepest_cpuidle_psscr_val;
+static u64 pnv_deepest_cpuidle_psscr_mask;
 static u64 pnv_deepest_stop_flag;
 static bool deepest_stop_found;
+static bool deepest_cpuidle_found;
 
 static int pnv_save_sprs_for_deep_states(void)
 {
@@ -76,7 +79,14 @@ static int pnv_save_sprs_for_deep_states(void)
uint64_t hid5_val = mfspr(SPRN_HID5);
uint64_t hmeer_val = mfspr(SPRN_HMEER);
uint64_t msr_val = MSR_IDLE;
-   uint64_t psscr_val = pnv_deepest_stop_psscr_val;
+
+   /*
+* Pick deepest cpuidle psscr as the value to be
+* restored through wakeup engine.
+* We will request a deeper state to be restored
+* in hotplug path
+*/
+   uint64_t psscr_val = pnv_deepest_cpuidle_psscr_val;
 
for_each_possible_cpu(cpu) {
uint64_t pir = get_hard_smp_processor_id(cpu);
@@ -409,7 +419,7 @@ static void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, 
u64 lpcr_val)
  */
 unsigned long pnv_cpu_offline(unsigned int cpu)
 {
-   unsigned long srr1;
+   u64 srr1;
u32 idle_states = pnv_get_supported_cpuidle_states();
u64 lpcr_val;
 
@@ -429,12 +439,18 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
__ppc64_runlatch_off();
 
if (cpu_has_feature(CPU_FTR_ARCH_300) && deepest_stop_found) {
-   unsigned long psscr;
+   u64 psscr;
+   u64 pir = get_hard_smp_processor_id(cpu);
 
psscr = mfspr(SPRN_PSSCR);
psscr = (psscr & ~pnv_deepest_stop_psscr_mask) |
pnv_deepest_stop_psscr_val;
+   if (pnv_deepest_stop_psscr_val != pnv_deepest_cpuidle_psscr_val)
+   opal_slw_set_reg(pir, P9_STOP_SPR_PSSCR, psscr);
srr1 = power9_idle_stop(psscr);
+   psscr = (psscr & ~pnv_deepest_cpuidle_psscr_mask) |
+   pnv_deepest_cpuidle_psscr_val;
+   opal_slw_set_reg(pir, P9_STOP_SPR_PSSCR, psscr);
 
} else if ((idle_states & OPAL_PM_WINKLE_ENABLED) &&
   (idle_states & OPAL_PM_LOSE_FULL_CONTEXT)) {
@@ -555,6 +571,7 @@ static int __init pnv_power9_idle_init(struct device_node 
*np, u32 *flags,
u64 *psscr_val = NULL;
u64 *psscr_mask = NULL;
u32 *residency_ns = NULL;
+   u32 *latency_ns = NULL;
u64 

[RESEND][PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Akshay Adiga]

commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
states via stop API.") uses stop-api provided by the firmware to restore
PSSCR. PSSCR restore is required for handling special wakeup. When special
wakeup is completed, the core enters stop state based on restored PSSCR.

Currently PSSCR is restored to deepest available stop state, causing
a idle cpu to enter deeper stop state on a special wakeup, which causes
the cpu to hang on wakeup.

A "sensors" command which reads temperature (through DTS sensors) on idle
cpu can trigger special wakeup.

Failed Scenario :
Request restore of PSSCR with RL = 11
cpu enters idle state (stop5)
  user triggers "sensors" command
   Assert special wakeup on cpu
 Restores PSSCR with RL = 11  < Done by firmware
  Read DTS sensor
   Deassert special wakeup
  cpu enters idle state (stop11) <-- Instead of stop5

Cpu hang is caused because cpu ended up in a deeper state than it requested

This patch fixes instability caused by special wakeup when stop11 is
enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
Only when offlining cpu, request restore of PSSCR to deepest stop state.
On onlining cpu, request restore of PSSCR to deepest stop state used by
cpuidle.

Cc:  # v4.14+
Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
states via stop API.")
Reported-by: Pridhiviraj Paidipeddi 
Signed-off-by: Akshay Adiga 
---
 arch/powerpc/include/asm/cpuidle.h|  2 ++
 arch/powerpc/platforms/powernv/idle.c | 46 ---
 drivers/cpuidle/cpuidle-powernv.c |  1 -
 3 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/cpuidle.h 
b/arch/powerpc/include/asm/cpuidle.h
index e210a83..f52e9f1 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -67,6 +67,8 @@
 #define ERR_EC_ESL_MISMATCH-1
 #define ERR_DEEP_STATE_ESL_MISMATCH-2
 
+#define POWERNV_THRESHOLD_LATENCY_NS 20
+
 #ifndef __ASSEMBLY__
 /* Additional SPRs that need to be saved/restored during stop */
 struct stop_sprs {
diff --git a/arch/powerpc/platforms/powernv/idle.c 
b/arch/powerpc/platforms/powernv/idle.c
index 443d5ca..4b0c7d24 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -56,8 +56,11 @@ u64 pnv_first_deep_stop_state = MAX_STOP_STATE;
  */
 static u64 pnv_deepest_stop_psscr_val;
 static u64 pnv_deepest_stop_psscr_mask;
+static u64 pnv_deepest_cpuidle_psscr_val;
+static u64 pnv_deepest_cpuidle_psscr_mask;
 static u64 pnv_deepest_stop_flag;
 static bool deepest_stop_found;
+static bool deepest_cpuidle_found;
 
 static int pnv_save_sprs_for_deep_states(void)
 {
@@ -76,7 +79,14 @@ static int pnv_save_sprs_for_deep_states(void)
uint64_t hid5_val = mfspr(SPRN_HID5);
uint64_t hmeer_val = mfspr(SPRN_HMEER);
uint64_t msr_val = MSR_IDLE;
-   uint64_t psscr_val = pnv_deepest_stop_psscr_val;
+
+   /*
+* Pick deepest cpuidle psscr as the value to be
+* restored through wakeup engine.
+* We will request a deeper state to be restored
+* in hotplug path
+*/
+   uint64_t psscr_val = pnv_deepest_cpuidle_psscr_val;
 
for_each_possible_cpu(cpu) {
uint64_t pir = get_hard_smp_processor_id(cpu);
@@ -409,7 +419,7 @@ static void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, 
u64 lpcr_val)
  */
 unsigned long pnv_cpu_offline(unsigned int cpu)
 {
-   unsigned long srr1;
+   u64 srr1;
u32 idle_states = pnv_get_supported_cpuidle_states();
u64 lpcr_val;
 
@@ -429,12 +439,18 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
__ppc64_runlatch_off();
 
if (cpu_has_feature(CPU_FTR_ARCH_300) && deepest_stop_found) {
-   unsigned long psscr;
+   u64 psscr;
+   u64 pir = get_hard_smp_processor_id(cpu);
 
psscr = mfspr(SPRN_PSSCR);
psscr = (psscr & ~pnv_deepest_stop_psscr_mask) |
pnv_deepest_stop_psscr_val;
+   if (pnv_deepest_stop_psscr_val != pnv_deepest_cpuidle_psscr_val)
+   opal_slw_set_reg(pir, P9_STOP_SPR_PSSCR, psscr);
srr1 = power9_idle_stop(psscr);
+   psscr = (psscr & ~pnv_deepest_cpuidle_psscr_mask) |
+   pnv_deepest_cpuidle_psscr_val;
+   opal_slw_set_reg(pir, P9_STOP_SPR_PSSCR, psscr);
 
} else if ((idle_states & OPAL_PM_WINKLE_ENABLED) &&
   (idle_states & OPAL_PM_LOSE_FULL_CONTEXT)) {
@@ -555,6 +571,7 @@ static int __init pnv_power9_idle_init(struct device_node 
*np, u32 *flags,
u64 *psscr_val = NULL;
u64 *psscr_mask = NULL;
u32 *residency_ns = NULL;
+   u32 *latency_ns = NULL;
u64 max_residency_ns = 0;
int rc = 0, i;
 
@@ -562,6 +579,8 @@ static int __init 

Re: [PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Akshay Adiga]

On Mon, Feb 26, 2018 at 03:47:12PM +1100, Stewart Smith wrote:
> Akshay Adiga  writes:
> > commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> > states via stop API.") uses stop-api provided by the firmware to restore
> > PSSCR. PSSCR restore is required for handling special wakeup. When special
> > wakeup is completed, the core enters stop state based on restored PSSCR.
> >
> > Currently PSSCR is restored to deepest available stop state, causing
> > a idle cpu to enter deeper stop state on a special wakeup, which causes
> > the cpu to hang on wakeup.
> >
> > A "sensors" command which reads temperature (through DTS sensors) on idle
> > cpu can trigger special wakeup.
> >
> > Failed Scenario :
> > Request restore of PSSCR with RL = 11
> > cpu enters idle state (stop5)
> >   user triggers "sensors" command
> >Assert special wakeup on cpu
> >  Restores PSSCR with RL = 11  < Done by firmware
> >   Read DTS sensor
> >Deassert special wakeup
> >   cpu enters idle state (stop11) <-- Instead of stop5
> >
> > Cpu hang is caused because cpu ended up in a deeper state than it requested
> >
> > This patch fixes instability caused by special wakeup when stop11 is
> > enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
> > Only when offlining cpu, request restore of PSSCR to deepest stop state.
> > On onlining cpu, request restore of PSSCR to deepest stop state used by
> > cpuidle.
> >
> > Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> > states via stop API.")
> 
> This should CC stable ?
> 
> We'll need this to enable stop11 in firmware and not break things, right?

Yes I will resend and CC it to stable.
> 
> -- 
> Stewart Smith
> OPAL Architect, IBM.
> 

Re: [PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Akshay Adiga]

On Mon, Feb 26, 2018 at 03:47:12PM +1100, Stewart Smith wrote:
> Akshay Adiga  writes:
> > commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> > states via stop API.") uses stop-api provided by the firmware to restore
> > PSSCR. PSSCR restore is required for handling special wakeup. When special
> > wakeup is completed, the core enters stop state based on restored PSSCR.
> >
> > Currently PSSCR is restored to deepest available stop state, causing
> > a idle cpu to enter deeper stop state on a special wakeup, which causes
> > the cpu to hang on wakeup.
> >
> > A "sensors" command which reads temperature (through DTS sensors) on idle
> > cpu can trigger special wakeup.
> >
> > Failed Scenario :
> > Request restore of PSSCR with RL = 11
> > cpu enters idle state (stop5)
> >   user triggers "sensors" command
> >Assert special wakeup on cpu
> >  Restores PSSCR with RL = 11  < Done by firmware
> >   Read DTS sensor
> >Deassert special wakeup
> >   cpu enters idle state (stop11) <-- Instead of stop5
> >
> > Cpu hang is caused because cpu ended up in a deeper state than it requested
> >
> > This patch fixes instability caused by special wakeup when stop11 is
> > enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
> > Only when offlining cpu, request restore of PSSCR to deepest stop state.
> > On onlining cpu, request restore of PSSCR to deepest stop state used by
> > cpuidle.
> >
> > Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> > states via stop API.")
> 
> This should CC stable ?
> 
> We'll need this to enable stop11 in firmware and not break things, right?

Yes I will resend and CC it to stable.
> 
> -- 
> Stewart Smith
> OPAL Architect, IBM.
> 

Re: [PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Stewart Smith]

Akshay Adiga  writes:
> commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> states via stop API.") uses stop-api provided by the firmware to restore
> PSSCR. PSSCR restore is required for handling special wakeup. When special
> wakeup is completed, the core enters stop state based on restored PSSCR.
>
> Currently PSSCR is restored to deepest available stop state, causing
> a idle cpu to enter deeper stop state on a special wakeup, which causes
> the cpu to hang on wakeup.
>
> A "sensors" command which reads temperature (through DTS sensors) on idle
> cpu can trigger special wakeup.
>
> Failed Scenario :
> Request restore of PSSCR with RL = 11
> cpu enters idle state (stop5)
>   user triggers "sensors" command
>Assert special wakeup on cpu
>  Restores PSSCR with RL = 11  < Done by firmware
>   Read DTS sensor
>Deassert special wakeup
>   cpu enters idle state (stop11) <-- Instead of stop5
>
> Cpu hang is caused because cpu ended up in a deeper state than it requested
>
> This patch fixes instability caused by special wakeup when stop11 is
> enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
> Only when offlining cpu, request restore of PSSCR to deepest stop state.
> On onlining cpu, request restore of PSSCR to deepest stop state used by
> cpuidle.
>
> Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> states via stop API.")

This should CC stable ?

We'll need this to enable stop11 in firmware and not break things, right?

-- 
Stewart Smith
OPAL Architect, IBM.

Re: [PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Stewart Smith]

Akshay Adiga  writes:
> commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> states via stop API.") uses stop-api provided by the firmware to restore
> PSSCR. PSSCR restore is required for handling special wakeup. When special
> wakeup is completed, the core enters stop state based on restored PSSCR.
>
> Currently PSSCR is restored to deepest available stop state, causing
> a idle cpu to enter deeper stop state on a special wakeup, which causes
> the cpu to hang on wakeup.
>
> A "sensors" command which reads temperature (through DTS sensors) on idle
> cpu can trigger special wakeup.
>
> Failed Scenario :
> Request restore of PSSCR with RL = 11
> cpu enters idle state (stop5)
>   user triggers "sensors" command
>Assert special wakeup on cpu
>  Restores PSSCR with RL = 11  < Done by firmware
>   Read DTS sensor
>Deassert special wakeup
>   cpu enters idle state (stop11) <-- Instead of stop5
>
> Cpu hang is caused because cpu ended up in a deeper state than it requested
>
> This patch fixes instability caused by special wakeup when stop11 is
> enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
> Only when offlining cpu, request restore of PSSCR to deepest stop state.
> On onlining cpu, request restore of PSSCR to deepest stop state used by
> cpuidle.
>
> Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> states via stop API.")

This should CC stable ?

We'll need this to enable stop11 in firmware and not break things, right?

-- 
Stewart Smith
OPAL Architect, IBM.

[PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Akshay Adiga]

commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
states via stop API.") uses stop-api provided by the firmware to restore
PSSCR. PSSCR restore is required for handling special wakeup. When special
wakeup is completed, the core enters stop state based on restored PSSCR.

Currently PSSCR is restored to deepest available stop state, causing
a idle cpu to enter deeper stop state on a special wakeup, which causes
the cpu to hang on wakeup.

A "sensors" command which reads temperature (through DTS sensors) on idle
cpu can trigger special wakeup.

Failed Scenario :
Request restore of PSSCR with RL = 11
cpu enters idle state (stop5)
  user triggers "sensors" command
   Assert special wakeup on cpu
 Restores PSSCR with RL = 11  < Done by firmware
  Read DTS sensor
   Deassert special wakeup
  cpu enters idle state (stop11) <-- Instead of stop5

Cpu hang is caused because cpu ended up in a deeper state than it requested

This patch fixes instability caused by special wakeup when stop11 is
enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
Only when offlining cpu, request restore of PSSCR to deepest stop state.
On onlining cpu, request restore of PSSCR to deepest stop state used by
cpuidle.

Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
states via stop API.")
Reported-by: Pridhiviraj Paidipeddi 
Signed-off-by: Akshay Adiga 
---
 arch/powerpc/include/asm/cpuidle.h|  2 ++
 arch/powerpc/platforms/powernv/idle.c | 46 ---
 drivers/cpuidle/cpuidle-powernv.c |  1 -
 3 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/cpuidle.h 
b/arch/powerpc/include/asm/cpuidle.h
index e210a83..f52e9f1 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -67,6 +67,8 @@
 #define ERR_EC_ESL_MISMATCH-1
 #define ERR_DEEP_STATE_ESL_MISMATCH-2
 
+#define POWERNV_THRESHOLD_LATENCY_NS 20
+
 #ifndef __ASSEMBLY__
 /* Additional SPRs that need to be saved/restored during stop */
 struct stop_sprs {
diff --git a/arch/powerpc/platforms/powernv/idle.c 
b/arch/powerpc/platforms/powernv/idle.c
index 443d5ca..4b0c7d24 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -56,8 +56,11 @@ u64 pnv_first_deep_stop_state = MAX_STOP_STATE;
  */
 static u64 pnv_deepest_stop_psscr_val;
 static u64 pnv_deepest_stop_psscr_mask;
+static u64 pnv_deepest_cpuidle_psscr_val;
+static u64 pnv_deepest_cpuidle_psscr_mask;
 static u64 pnv_deepest_stop_flag;
 static bool deepest_stop_found;
+static bool deepest_cpuidle_found;
 
 static int pnv_save_sprs_for_deep_states(void)
 {
@@ -76,7 +79,14 @@ static int pnv_save_sprs_for_deep_states(void)
uint64_t hid5_val = mfspr(SPRN_HID5);
uint64_t hmeer_val = mfspr(SPRN_HMEER);
uint64_t msr_val = MSR_IDLE;
-   uint64_t psscr_val = pnv_deepest_stop_psscr_val;
+
+   /*
+* Pick deepest cpuidle psscr as the value to be
+* restored through wakeup engine.
+* We will request a deeper state to be restored
+* in hotplug path
+*/
+   uint64_t psscr_val = pnv_deepest_cpuidle_psscr_val;
 
for_each_possible_cpu(cpu) {
uint64_t pir = get_hard_smp_processor_id(cpu);
@@ -409,7 +419,7 @@ static void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, 
u64 lpcr_val)
  */
 unsigned long pnv_cpu_offline(unsigned int cpu)
 {
-   unsigned long srr1;
+   u64 srr1;
u32 idle_states = pnv_get_supported_cpuidle_states();
u64 lpcr_val;
 
@@ -429,12 +439,18 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
__ppc64_runlatch_off();
 
if (cpu_has_feature(CPU_FTR_ARCH_300) && deepest_stop_found) {
-   unsigned long psscr;
+   u64 psscr;
+   u64 pir = get_hard_smp_processor_id(cpu);
 
psscr = mfspr(SPRN_PSSCR);
psscr = (psscr & ~pnv_deepest_stop_psscr_mask) |
pnv_deepest_stop_psscr_val;
+   if (pnv_deepest_stop_psscr_val != pnv_deepest_cpuidle_psscr_val)
+   opal_slw_set_reg(pir, P9_STOP_SPR_PSSCR, psscr);
srr1 = power9_idle_stop(psscr);
+   psscr = (psscr & ~pnv_deepest_cpuidle_psscr_mask) |
+   pnv_deepest_cpuidle_psscr_val;
+   opal_slw_set_reg(pir, P9_STOP_SPR_PSSCR, psscr);
 
} else if ((idle_states & OPAL_PM_WINKLE_ENABLED) &&
   (idle_states & OPAL_PM_LOSE_FULL_CONTEXT)) {
@@ -555,6 +571,7 @@ static int __init pnv_power9_idle_init(struct device_node 
*np, u32 *flags,
u64 *psscr_val = NULL;
u64 *psscr_mask = NULL;
u32 *residency_ns = NULL;
+   u32 *latency_ns = NULL;
u64 max_residency_ns = 0;
int rc = 0, i;
 
@@ 

[PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug

[Akshay Adiga]

commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
states via stop API.") uses stop-api provided by the firmware to restore
PSSCR. PSSCR restore is required for handling special wakeup. When special
wakeup is completed, the core enters stop state based on restored PSSCR.

Currently PSSCR is restored to deepest available stop state, causing
a idle cpu to enter deeper stop state on a special wakeup, which causes
the cpu to hang on wakeup.

A "sensors" command which reads temperature (through DTS sensors) on idle
cpu can trigger special wakeup.

Failed Scenario :
Request restore of PSSCR with RL = 11
cpu enters idle state (stop5)
  user triggers "sensors" command
   Assert special wakeup on cpu
 Restores PSSCR with RL = 11  < Done by firmware
  Read DTS sensor
   Deassert special wakeup
  cpu enters idle state (stop11) <-- Instead of stop5

Cpu hang is caused because cpu ended up in a deeper state than it requested

This patch fixes instability caused by special wakeup when stop11 is
enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
Only when offlining cpu, request restore of PSSCR to deepest stop state.
On onlining cpu, request restore of PSSCR to deepest stop state used by
cpuidle.

Fixes : 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
states via stop API.")
Reported-by: Pridhiviraj Paidipeddi 
Signed-off-by: Akshay Adiga 
---
 arch/powerpc/include/asm/cpuidle.h|  2 ++
 arch/powerpc/platforms/powernv/idle.c | 46 ---
 drivers/cpuidle/cpuidle-powernv.c |  1 -
 3 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/cpuidle.h 
b/arch/powerpc/include/asm/cpuidle.h
index e210a83..f52e9f1 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -67,6 +67,8 @@
 #define ERR_EC_ESL_MISMATCH-1
 #define ERR_DEEP_STATE_ESL_MISMATCH-2
 
+#define POWERNV_THRESHOLD_LATENCY_NS 20
+
 #ifndef __ASSEMBLY__
 /* Additional SPRs that need to be saved/restored during stop */
 struct stop_sprs {
diff --git a/arch/powerpc/platforms/powernv/idle.c 
b/arch/powerpc/platforms/powernv/idle.c
index 443d5ca..4b0c7d24 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -56,8 +56,11 @@ u64 pnv_first_deep_stop_state = MAX_STOP_STATE;
  */
 static u64 pnv_deepest_stop_psscr_val;
 static u64 pnv_deepest_stop_psscr_mask;
+static u64 pnv_deepest_cpuidle_psscr_val;
+static u64 pnv_deepest_cpuidle_psscr_mask;
 static u64 pnv_deepest_stop_flag;
 static bool deepest_stop_found;
+static bool deepest_cpuidle_found;
 
 static int pnv_save_sprs_for_deep_states(void)
 {
@@ -76,7 +79,14 @@ static int pnv_save_sprs_for_deep_states(void)
uint64_t hid5_val = mfspr(SPRN_HID5);
uint64_t hmeer_val = mfspr(SPRN_HMEER);
uint64_t msr_val = MSR_IDLE;
-   uint64_t psscr_val = pnv_deepest_stop_psscr_val;
+
+   /*
+* Pick deepest cpuidle psscr as the value to be
+* restored through wakeup engine.
+* We will request a deeper state to be restored
+* in hotplug path
+*/
+   uint64_t psscr_val = pnv_deepest_cpuidle_psscr_val;
 
for_each_possible_cpu(cpu) {
uint64_t pir = get_hard_smp_processor_id(cpu);
@@ -409,7 +419,7 @@ static void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, 
u64 lpcr_val)
  */
 unsigned long pnv_cpu_offline(unsigned int cpu)
 {
-   unsigned long srr1;
+   u64 srr1;
u32 idle_states = pnv_get_supported_cpuidle_states();
u64 lpcr_val;
 
@@ -429,12 +439,18 @@ unsigned long pnv_cpu_offline(unsigned int cpu)
__ppc64_runlatch_off();
 
if (cpu_has_feature(CPU_FTR_ARCH_300) && deepest_stop_found) {
-   unsigned long psscr;
+   u64 psscr;
+   u64 pir = get_hard_smp_processor_id(cpu);
 
psscr = mfspr(SPRN_PSSCR);
psscr = (psscr & ~pnv_deepest_stop_psscr_mask) |
pnv_deepest_stop_psscr_val;
+   if (pnv_deepest_stop_psscr_val != pnv_deepest_cpuidle_psscr_val)
+   opal_slw_set_reg(pir, P9_STOP_SPR_PSSCR, psscr);
srr1 = power9_idle_stop(psscr);
+   psscr = (psscr & ~pnv_deepest_cpuidle_psscr_mask) |
+   pnv_deepest_cpuidle_psscr_val;
+   opal_slw_set_reg(pir, P9_STOP_SPR_PSSCR, psscr);
 
} else if ((idle_states & OPAL_PM_WINKLE_ENABLED) &&
   (idle_states & OPAL_PM_LOSE_FULL_CONTEXT)) {
@@ -555,6 +571,7 @@ static int __init pnv_power9_idle_init(struct device_node 
*np, u32 *flags,
u64 *psscr_val = NULL;
u64 *psscr_mask = NULL;
u32 *residency_ns = NULL;
+   u32 *latency_ns = NULL;
u64 max_residency_ns = 0;
int rc = 0, i;
 
@@ -562,6 +579,8 @@ static int __init pnv_power9_idle_init(struct