* [PATCH 0/3] one more try at idle improvements
@ 2017-11-17 14:08 Nicholas Piggin
2017-11-17 14:08 ` [PATCH 1/3] powerpc/64s/idle: POWER9 implement a separate idle stop function for hotplug Nicholas Piggin
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Nicholas Piggin @ 2017-11-17 14:08 UTC (permalink / raw)
To: linuxppc-dev
Cc: Nicholas Piggin, Vaidyanathan Srinivasan, Gautham R . Shenoy,
Paul Mackerras
The recent KVM work clashed with a couple of my patches, so they were
reverted/dropped for this merge window.
I've made another attempt at them, hopefully done in a way that is
more compatible with KVM requirements and in a bit nicer way (POWER8
contex switching should speed up a bit too). I'm not completely sure I
got patch 2 right. It works on POWER8, but haven't been able to get
POWER9 running with hash on radix.
Thanks,
Nick
Nicholas Piggin (3):
powerpc/64s/idle: POWER9 implement a separate idle stop function for
hotplug
powerpc/64s/idle: avoid sync for KVM state when waking from idle
powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead
arch/powerpc/include/asm/processor.h | 1 +
arch/powerpc/kernel/idle_book3s.S | 62 +++++++++++++++++------------------
arch/powerpc/platforms/powernv/idle.c | 9 +++--
3 files changed, 38 insertions(+), 34 deletions(-)
--
2.15.0
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/3] powerpc/64s/idle: POWER9 implement a separate idle stop function for hotplug 2017-11-17 14:08 [PATCH 0/3] one more try at idle improvements Nicholas Piggin @ 2017-11-17 14:08 ` Nicholas Piggin 2018-02-28 18:24 ` Vaidyanathan Srinivasan 2018-04-03 16:03 ` [1/3] " Michael Ellerman 2017-11-17 14:08 ` [PATCH 2/3] powerpc/64s/idle: avoid sync for KVM state when waking from idle Nicholas Piggin 2017-11-17 14:08 ` [PATCH 3/3] powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead Nicholas Piggin 2 siblings, 2 replies; 16+ messages in thread From: Nicholas Piggin @ 2017-11-17 14:08 UTC (permalink / raw) To: linuxppc-dev Cc: Nicholas Piggin, Vaidyanathan Srinivasan, Gautham R . Shenoy, Paul Mackerras Implement a new function to invoke stop, power9_offline_stop, which is like power9_idle_stop but used by the cpu hotplug code. Move KVM secondary state manipulation code to the offline case. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/include/asm/processor.h | 1 + arch/powerpc/kernel/idle_book3s.S | 24 ++++++++++++++++++------ arch/powerpc/platforms/powernv/idle.c | 2 +- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h index bdab3b74eb98..0ebdb58460ce 100644 --- a/arch/powerpc/include/asm/processor.h +++ b/arch/powerpc/include/asm/processor.h @@ -500,6 +500,7 @@ extern int powersave_nap; /* set if nap mode can be used in idle loop */ extern unsigned long power7_idle_insn(unsigned long type); /* PNV_THREAD_NAP/etc*/ extern void power7_idle_type(unsigned long type); extern unsigned long power9_idle_stop(unsigned long psscr_val); +extern unsigned long power9_offline_stop(unsigned long psscr_val); extern void power9_idle_type(unsigned long stop_psscr_val, unsigned long stop_psscr_mask); diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S index 01e1c1997893..2f8364e7b489 100644 --- a/arch/powerpc/kernel/idle_book3s.S +++ b/arch/powerpc/kernel/idle_book3s.S @@ -325,12 +325,6 @@ enter_winkle: * r3 - PSSCR value corresponding to the requested stop state. */ power_enter_stop: -#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE - /* Tell KVM we're entering idle */ - li r4,KVM_HWTHREAD_IN_IDLE - /* DO THIS IN REAL MODE! See comment above. */ - stb r4,HSTATE_HWTHREAD_STATE(r13) -#endif /* * Check if we are executing the lite variant with ESL=EC=0 */ @@ -435,6 +429,24 @@ _GLOBAL(power9_idle_stop) b pnv_powersave_common /* No return */ +/* + * Entered with MSR[EE]=0 and no soft-masked interrupts pending. + * r3 contains desired PSSCR register value. + */ +_GLOBAL(power9_offline_stop) + std r3, PACA_REQ_PSSCR(r13) + mtspr SPRN_PSSCR,r3 +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE + /* Tell KVM we're entering idle */ + li r4,KVM_HWTHREAD_IN_IDLE + /* DO THIS IN REAL MODE! See comment above. */ + stb r4,HSTATE_HWTHREAD_STATE(r13) +#endif + LOAD_REG_ADDR(r4,power_enter_stop) + b pnv_powersave_common + /* No return */ + + /* * On waking up from stop 0,1,2 with ESL=1 on POWER9 DD1, * HSPRG0 will be set to the HSPRG0 value of one of the diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c index 443d5ca71995..a921d5428d76 100644 --- a/arch/powerpc/platforms/powernv/idle.c +++ b/arch/powerpc/platforms/powernv/idle.c @@ -434,7 +434,7 @@ unsigned long pnv_cpu_offline(unsigned int cpu) psscr = mfspr(SPRN_PSSCR); psscr = (psscr & ~pnv_deepest_stop_psscr_mask) | pnv_deepest_stop_psscr_val; - srr1 = power9_idle_stop(psscr); + srr1 = power9_offline_stop(psscr); } else if ((idle_states & OPAL_PM_WINKLE_ENABLED) && (idle_states & OPAL_PM_LOSE_FULL_CONTEXT)) { -- 2.15.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] powerpc/64s/idle: POWER9 implement a separate idle stop function for hotplug 2017-11-17 14:08 ` [PATCH 1/3] powerpc/64s/idle: POWER9 implement a separate idle stop function for hotplug Nicholas Piggin @ 2018-02-28 18:24 ` Vaidyanathan Srinivasan 2018-04-03 16:03 ` [1/3] " Michael Ellerman 1 sibling, 0 replies; 16+ messages in thread From: Vaidyanathan Srinivasan @ 2018-02-28 18:24 UTC (permalink / raw) To: Nicholas Piggin; +Cc: linuxppc-dev, Gautham R . Shenoy, Paul Mackerras * Nicholas Piggin <npiggin@gmail.com> [2017-11-18 00:08:05]: > Implement a new function to invoke stop, power9_offline_stop, which is > like power9_idle_stop but used by the cpu hotplug code. > > Move KVM secondary state manipulation code to the offline case. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> > --- > arch/powerpc/include/asm/processor.h | 1 + > arch/powerpc/kernel/idle_book3s.S | 24 ++++++++++++++++++------ > arch/powerpc/platforms/powernv/idle.c | 2 +- > 3 files changed, 20 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h > index bdab3b74eb98..0ebdb58460ce 100644 > --- a/arch/powerpc/include/asm/processor.h > +++ b/arch/powerpc/include/asm/processor.h > @@ -500,6 +500,7 @@ extern int powersave_nap; /* set if nap mode can be used in idle loop */ > extern unsigned long power7_idle_insn(unsigned long type); /* PNV_THREAD_NAP/etc*/ > extern void power7_idle_type(unsigned long type); > extern unsigned long power9_idle_stop(unsigned long psscr_val); > +extern unsigned long power9_offline_stop(unsigned long psscr_val); > extern void power9_idle_type(unsigned long stop_psscr_val, > unsigned long stop_psscr_mask); > > diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S > index 01e1c1997893..2f8364e7b489 100644 > --- a/arch/powerpc/kernel/idle_book3s.S > +++ b/arch/powerpc/kernel/idle_book3s.S > @@ -325,12 +325,6 @@ enter_winkle: > * r3 - PSSCR value corresponding to the requested stop state. > */ > power_enter_stop: > -#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > - /* Tell KVM we're entering idle */ > - li r4,KVM_HWTHREAD_IN_IDLE > - /* DO THIS IN REAL MODE! See comment above. */ > - stb r4,HSTATE_HWTHREAD_STATE(r13) > -#endif > /* > * Check if we are executing the lite variant with ESL=EC=0 > */ > @@ -435,6 +429,24 @@ _GLOBAL(power9_idle_stop) > b pnv_powersave_common > /* No return */ > > +/* > + * Entered with MSR[EE]=0 and no soft-masked interrupts pending. > + * r3 contains desired PSSCR register value. > + */ > +_GLOBAL(power9_offline_stop) > + std r3, PACA_REQ_PSSCR(r13) > + mtspr SPRN_PSSCR,r3 > +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > + /* Tell KVM we're entering idle */ > + li r4,KVM_HWTHREAD_IN_IDLE > + /* DO THIS IN REAL MODE! See comment above. */ > + stb r4,HSTATE_HWTHREAD_STATE(r13) > +#endif > + LOAD_REG_ADDR(r4,power_enter_stop) > + b pnv_powersave_common > + /* No return */ > + Good optimization. This code is needed only when an offline thread is wokenup at 0x100 to get into guest. Can be skipped in idle wakeup case. > + > /* > * On waking up from stop 0,1,2 with ESL=1 on POWER9 DD1, > * HSPRG0 will be set to the HSPRG0 value of one of the > diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c > index 443d5ca71995..a921d5428d76 100644 > --- a/arch/powerpc/platforms/powernv/idle.c > +++ b/arch/powerpc/platforms/powernv/idle.c > @@ -434,7 +434,7 @@ unsigned long pnv_cpu_offline(unsigned int cpu) > psscr = mfspr(SPRN_PSSCR); > psscr = (psscr & ~pnv_deepest_stop_psscr_mask) | > pnv_deepest_stop_psscr_val; > - srr1 = power9_idle_stop(psscr); > + srr1 = power9_offline_stop(psscr); > > } else if ((idle_states & OPAL_PM_WINKLE_ENABLED) && > (idle_states & OPAL_PM_LOSE_FULL_CONTEXT)) { > -- > 2.15.0 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [1/3] powerpc/64s/idle: POWER9 implement a separate idle stop function for hotplug 2017-11-17 14:08 ` [PATCH 1/3] powerpc/64s/idle: POWER9 implement a separate idle stop function for hotplug Nicholas Piggin 2018-02-28 18:24 ` Vaidyanathan Srinivasan @ 2018-04-03 16:03 ` Michael Ellerman 2018-04-03 17:13 ` Nicholas Piggin 1 sibling, 1 reply; 16+ messages in thread From: Michael Ellerman @ 2018-04-03 16:03 UTC (permalink / raw) To: Nicholas Piggin, linuxppc-dev; +Cc: Gautham R . Shenoy, Nicholas Piggin On Fri, 2017-11-17 at 14:08:05 UTC, Nicholas Piggin wrote: > Implement a new function to invoke stop, power9_offline_stop, which is > like power9_idle_stop but used by the cpu hotplug code. > > Move KVM secondary state manipulation code to the offline case. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/3d4fbffdd703d2b968db443911f214 cheers ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [1/3] powerpc/64s/idle: POWER9 implement a separate idle stop function for hotplug 2018-04-03 16:03 ` [1/3] " Michael Ellerman @ 2018-04-03 17:13 ` Nicholas Piggin 2018-04-04 12:52 ` Michael Ellerman 0 siblings, 1 reply; 16+ messages in thread From: Nicholas Piggin @ 2018-04-03 17:13 UTC (permalink / raw) To: Michael Ellerman; +Cc: linuxppc-dev, Gautham R . Shenoy On Wed, 4 Apr 2018 02:03:18 +1000 (AEST) Michael Ellerman <patch-notifications@ellerman.id.au> wrote: > On Fri, 2017-11-17 at 14:08:05 UTC, Nicholas Piggin wrote: > > Implement a new function to invoke stop, power9_offline_stop, which is > > like power9_idle_stop but used by the cpu hotplug code. > > > > Move KVM secondary state manipulation code to the offline case. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> > > Applied to powerpc next, thanks. > > https://git.kernel.org/powerpc/c/3d4fbffdd703d2b968db443911f214 > > cheers I sent out a new series for this which is rebased and a bit cleaner. I probably wasn't clear enough about the change, sorry. This patch will need to be adjusted for the force SMT4 change. The end result should look like this, https://patchwork.ozlabs.org/patch/893948/ Thanks, Nick ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [1/3] powerpc/64s/idle: POWER9 implement a separate idle stop function for hotplug 2018-04-03 17:13 ` Nicholas Piggin @ 2018-04-04 12:52 ` Michael Ellerman 0 siblings, 0 replies; 16+ messages in thread From: Michael Ellerman @ 2018-04-04 12:52 UTC (permalink / raw) To: Nicholas Piggin, Michael Ellerman; +Cc: linuxppc-dev, Gautham R . Shenoy Nicholas Piggin <npiggin@gmail.com> writes: > On Wed, 4 Apr 2018 02:03:18 +1000 (AEST) > Michael Ellerman <patch-notifications@ellerman.id.au> wrote: >> On Fri, 2017-11-17 at 14:08:05 UTC, Nicholas Piggin wrote: >> > Implement a new function to invoke stop, power9_offline_stop, which is >> > like power9_idle_stop but used by the cpu hotplug code. >> > >> > Move KVM secondary state manipulation code to the offline case. >> > >> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> > Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> >> >> Applied to powerpc next, thanks. >> >> https://git.kernel.org/powerpc/c/3d4fbffdd703d2b968db443911f214 > > I sent out a new series for this which is rebased and a bit cleaner. > I probably wasn't clear enough about the change, sorry. No that's OK, I just already had the previous series applied (partially) in my testing tree and didn't see the new series in time to back it out. > This patch will need to be adjusted for the force SMT4 change. The > end result should look like this, > > https://patchwork.ozlabs.org/patch/893948/ Thanks. I did a patch to get everything in sync, and confirmed the result is the same as applying your new series. cheers ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] powerpc/64s/idle: avoid sync for KVM state when waking from idle 2017-11-17 14:08 [PATCH 0/3] one more try at idle improvements Nicholas Piggin 2017-11-17 14:08 ` [PATCH 1/3] powerpc/64s/idle: POWER9 implement a separate idle stop function for hotplug Nicholas Piggin @ 2017-11-17 14:08 ` Nicholas Piggin 2018-02-28 18:16 ` Vaidyanathan Srinivasan 2018-04-03 16:03 ` [2/3] " Michael Ellerman 2017-11-17 14:08 ` [PATCH 3/3] powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead Nicholas Piggin 2 siblings, 2 replies; 16+ messages in thread From: Nicholas Piggin @ 2017-11-17 14:08 UTC (permalink / raw) To: linuxppc-dev Cc: Nicholas Piggin, Vaidyanathan Srinivasan, Gautham R . Shenoy, Paul Mackerras When waking from a CPU idle instruction (e.g., nap or stop), the sync for ordering the KVM secondary thread state can be avoided if there wakeup is coming from a kernel context rather than KVM context. This improves performance for ping-pong benchmark with the stop0 idle state by 0.46% for 2 threads in the same core, and 1.02% for different cores. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- 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 2f8364e7b489..07a306173c5a 100644 --- a/arch/powerpc/kernel/idle_book3s.S +++ b/arch/powerpc/kernel/idle_book3s.S @@ -532,6 +532,9 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300) mr r3,r12 #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE + lbz r0,HSTATE_HWTHREAD_STATE(r13) + cmpwi r0,KVM_HWTHREAD_IN_KERNEL + beq 1f li r0,KVM_HWTHREAD_IN_KERNEL stb r0,HSTATE_HWTHREAD_STATE(r13) /* Order setting hwthread_state vs. testing hwthread_req */ -- 2.15.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] powerpc/64s/idle: avoid sync for KVM state when waking from idle 2017-11-17 14:08 ` [PATCH 2/3] powerpc/64s/idle: avoid sync for KVM state when waking from idle Nicholas Piggin @ 2018-02-28 18:16 ` Vaidyanathan Srinivasan 2018-03-01 11:38 ` Nicholas Piggin 2018-04-03 16:03 ` [2/3] " Michael Ellerman 1 sibling, 1 reply; 16+ messages in thread From: Vaidyanathan Srinivasan @ 2018-02-28 18:16 UTC (permalink / raw) To: Nicholas Piggin; +Cc: linuxppc-dev, Gautham R . Shenoy, Paul Mackerras * Nicholas Piggin <npiggin@gmail.com> [2017-11-18 00:08:06]: > When waking from a CPU idle instruction (e.g., nap or stop), the sync > for ordering the KVM secondary thread state can be avoided if there > wakeup is coming from a kernel context rather than KVM context. > > This improves performance for ping-pong benchmark with the stop0 idle > state by 0.46% for 2 threads in the same core, and 1.02% for different > cores. Cool, the improvement comes from avoiding the "sync" alone? > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > 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 2f8364e7b489..07a306173c5a 100644 > --- a/arch/powerpc/kernel/idle_book3s.S > +++ b/arch/powerpc/kernel/idle_book3s.S > @@ -532,6 +532,9 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300) > mr r3,r12 > > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > + lbz r0,HSTATE_HWTHREAD_STATE(r13) > + cmpwi r0,KVM_HWTHREAD_IN_KERNEL > + beq 1f > li r0,KVM_HWTHREAD_IN_KERNEL > stb r0,HSTATE_HWTHREAD_STATE(r13) > /* Order setting hwthread_state vs. testing hwthread_req */ With this change, we will not check for HSTATE_HWTHREAD_REQ != 0 condition but unconditionally goto host kernel if HSTATE_HWTHREAD_STATE == KVM_HWTHREAD_IN_KERNEL at wakeup. Host is in ST mode and sibling thread got a wakeup event (door bell) to execute a new vcpu by calling kvm_start_guest, what will HSTATE_HWTHREAD_STATE be? Just to clarify, what will the flags looks like for (a) Host cpu sibling thread is offline and need to execute guest (b) Host cpu sibling thread is idle and need to execute guest --Vaidy ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] powerpc/64s/idle: avoid sync for KVM state when waking from idle 2018-02-28 18:16 ` Vaidyanathan Srinivasan @ 2018-03-01 11:38 ` Nicholas Piggin 0 siblings, 0 replies; 16+ messages in thread From: Nicholas Piggin @ 2018-03-01 11:38 UTC (permalink / raw) To: Vaidyanathan Srinivasan; +Cc: linuxppc-dev, Gautham R . Shenoy, Paul Mackerras On Wed, 28 Feb 2018 23:46:23 +0530 Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote: > * Nicholas Piggin <npiggin@gmail.com> [2017-11-18 00:08:06]: > > > When waking from a CPU idle instruction (e.g., nap or stop), the sync > > for ordering the KVM secondary thread state can be avoided if there > > wakeup is coming from a kernel context rather than KVM context. > > > > This improves performance for ping-pong benchmark with the stop0 idle > > state by 0.46% for 2 threads in the same core, and 1.02% for different > > cores. > > Cool, the improvement comes from avoiding the "sync" alone? Yes, they can be pretty costly. > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > 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 2f8364e7b489..07a306173c5a 100644 > > --- a/arch/powerpc/kernel/idle_book3s.S > > +++ b/arch/powerpc/kernel/idle_book3s.S > > @@ -532,6 +532,9 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300) > > mr r3,r12 > > > > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > > + lbz r0,HSTATE_HWTHREAD_STATE(r13) > > + cmpwi r0,KVM_HWTHREAD_IN_KERNEL > > + beq 1f > > li r0,KVM_HWTHREAD_IN_KERNEL > > stb r0,HSTATE_HWTHREAD_STATE(r13) > > /* Order setting hwthread_state vs. testing hwthread_req */ > > With this change, we will not check for HSTATE_HWTHREAD_REQ != 0 > condition but unconditionally goto host kernel if > HSTATE_HWTHREAD_STATE == KVM_HWTHREAD_IN_KERNEL at wakeup. That's right. > Host is in ST mode and sibling thread got a wakeup event (door bell) to execute > a new vcpu by calling kvm_start_guest, what will HSTATE_HWTHREAD_STATE be? It should be KVM_HWTHREAD_IN_IDLE. > Just to clarify, what will the flags looks like for > > (a) Host cpu sibling thread is offline and need to execute guest > (b) Host cpu sibling thread is idle and need to execute guest In the idle case we are running with independent threads mode and siblings not unplugged, so we should not get KVM wake-up requests come through this path. I'm not fluent in KVM though, so I could be wrong. Thanks, Nick ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [2/3] powerpc/64s/idle: avoid sync for KVM state when waking from idle 2017-11-17 14:08 ` [PATCH 2/3] powerpc/64s/idle: avoid sync for KVM state when waking from idle Nicholas Piggin 2018-02-28 18:16 ` Vaidyanathan Srinivasan @ 2018-04-03 16:03 ` Michael Ellerman 1 sibling, 0 replies; 16+ messages in thread From: Michael Ellerman @ 2018-04-03 16:03 UTC (permalink / raw) To: Nicholas Piggin, linuxppc-dev; +Cc: Gautham R . Shenoy, Nicholas Piggin On Fri, 2017-11-17 at 14:08:06 UTC, Nicholas Piggin wrote: > When waking from a CPU idle instruction (e.g., nap or stop), the sync > for ordering the KVM secondary thread state can be avoided if there > wakeup is coming from a kernel context rather than KVM context. > > This improves performance for ping-pong benchmark with the stop0 idle > state by 0.46% for 2 threads in the same core, and 1.02% for different > cores. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/8c1c7fb0b5ec95c392e9b585a6cf8c cheers ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead 2017-11-17 14:08 [PATCH 0/3] one more try at idle improvements Nicholas Piggin 2017-11-17 14:08 ` [PATCH 1/3] powerpc/64s/idle: POWER9 implement a separate idle stop function for hotplug Nicholas Piggin 2017-11-17 14:08 ` [PATCH 2/3] powerpc/64s/idle: avoid sync for KVM state when waking from idle Nicholas Piggin @ 2017-11-17 14:08 ` Nicholas Piggin 2018-02-28 18:34 ` Vaidyanathan Srinivasan 2018-03-31 11:46 ` Michael Ellerman 2 siblings, 2 replies; 16+ messages in thread From: Nicholas Piggin @ 2017-11-17 14:08 UTC (permalink / raw) To: linuxppc-dev Cc: Nicholas Piggin, Vaidyanathan Srinivasan, Gautham R . Shenoy, Paul Mackerras When stop is executed with EC=ESL=0, it appears to execute like a normal instruction (resuming from NIP when woken by interrupt). So all the save/restore handling can be avoided completely. In particular NV GPRs do not have to be saved, and MSR does not have to be switched back to kernel MSR. So move the test for EC=ESL=0 sleep states out to power9_idle_stop, and return directly to the caller after stop in that case. The mtspr to PSSCR is moved to the top of power9_offline_stop just so it matches power9_idle_stop. This improves performance for ping-pong benchmark with the stop0_lite idle state by 2.54% for 2 threads in the same core, and 2.57% for different cores. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/kernel/idle_book3s.S | 43 +++++++++++------------------------ arch/powerpc/platforms/powernv/idle.c | 7 +++++- 2 files changed, 19 insertions(+), 31 deletions(-) diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S index 07a306173c5a..6243da99b26c 100644 --- a/arch/powerpc/kernel/idle_book3s.S +++ b/arch/powerpc/kernel/idle_book3s.S @@ -324,31 +324,8 @@ enter_winkle: /* * r3 - PSSCR value corresponding to the requested stop state. */ -power_enter_stop: -/* - * Check if we are executing the lite variant with ESL=EC=0 - */ - andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED +power_enter_stop_esl: clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */ - bne .Lhandle_esl_ec_set - PPC_STOP - li r3,0 /* Since we didn't lose state, return 0 */ - - /* - * pnv_wakeup_noloss() expects r12 to contain the SRR1 value so - * it can determine if the wakeup reason is an HMI in - * CHECK_HMI_INTERRUPT. - * - * However, when we wakeup with ESL=0, SRR1 will not contain the wakeup - * reason, so there is no point setting r12 to SRR1. - * - * Further, we clear r12 here, so that we don't accidentally enter the - * HMI in pnv_wakeup_noloss() if the value of r12[42:45] == WAKE_HMI. - */ - li r12, 0 - b pnv_wakeup_noloss - -.Lhandle_esl_ec_set: BEGIN_FTR_SECTION /* * POWER9 DD2.0 or earlier can incorrectly set PMAO when waking up after @@ -423,26 +400,32 @@ ALT_FTR_SECTION_END_NESTED_IFSET(CPU_FTR_ARCH_207S, 66); \ * r3 contains desired PSSCR register value. */ _GLOBAL(power9_idle_stop) - std r3, PACA_REQ_PSSCR(r13) mtspr SPRN_PSSCR,r3 - LOAD_REG_ADDR(r4,power_enter_stop) + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED + bne 1f + PPC_STOP + li r3,0 /* Since we didn't lose state, return 0 */ + blr + +1: std r3, PACA_REQ_PSSCR(r13) + LOAD_REG_ADDR(r4,power_enter_stop_esl) b pnv_powersave_common /* No return */ /* - * Entered with MSR[EE]=0 and no soft-masked interrupts pending. - * r3 contains desired PSSCR register value. + * This is the same as the above, but it sets KVM state for secondaries, + * and it must have PSSCR[EC]=1 */ _GLOBAL(power9_offline_stop) - std r3, PACA_REQ_PSSCR(r13) mtspr SPRN_PSSCR,r3 + std r3, PACA_REQ_PSSCR(r13) #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE /* Tell KVM we're entering idle */ li r4,KVM_HWTHREAD_IN_IDLE /* DO THIS IN REAL MODE! See comment above. */ stb r4,HSTATE_HWTHREAD_STATE(r13) #endif - LOAD_REG_ADDR(r4,power_enter_stop) + LOAD_REG_ADDR(r4,power_enter_stop_esl) b pnv_powersave_common /* No return */ diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c index a921d5428d76..610b1637c16f 100644 --- a/arch/powerpc/platforms/powernv/idle.c +++ b/arch/powerpc/platforms/powernv/idle.c @@ -621,7 +621,12 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags, continue; } - if (max_residency_ns < residency_ns[i]) { + /* + * Deepest stop for unplug must be PSSCR[EC]=1 (wakeup at + * 0x100. + */ + if ((max_residency_ns < residency_ns[i])&& + (psscr_val[i] & PSSCR_EC)) { max_residency_ns = residency_ns[i]; pnv_deepest_stop_psscr_val = psscr_val[i]; pnv_deepest_stop_psscr_mask = psscr_mask[i]; -- 2.15.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead 2017-11-17 14:08 ` [PATCH 3/3] powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead Nicholas Piggin @ 2018-02-28 18:34 ` Vaidyanathan Srinivasan 2018-03-01 11:57 ` Nicholas Piggin 2018-03-31 11:46 ` Michael Ellerman 1 sibling, 1 reply; 16+ messages in thread From: Vaidyanathan Srinivasan @ 2018-02-28 18:34 UTC (permalink / raw) To: Nicholas Piggin; +Cc: linuxppc-dev, Gautham R . Shenoy, Paul Mackerras * Nicholas Piggin <npiggin@gmail.com> [2017-11-18 00:08:07]: > When stop is executed with EC=ESL=0, it appears to execute like a > normal instruction (resuming from NIP when woken by interrupt). So all > the save/restore handling can be avoided completely. In particular NV > GPRs do not have to be saved, and MSR does not have to be switched > back to kernel MSR. > > So move the test for EC=ESL=0 sleep states out to power9_idle_stop, > and return directly to the caller after stop in that case. The mtspr > to PSSCR is moved to the top of power9_offline_stop just so it matches > power9_idle_stop. > > This improves performance for ping-pong benchmark with the stop0_lite > idle state by 2.54% for 2 threads in the same core, and 2.57% for > different cores. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> > --- > arch/powerpc/kernel/idle_book3s.S | 43 +++++++++++------------------------ > arch/powerpc/platforms/powernv/idle.c | 7 +++++- > 2 files changed, 19 insertions(+), 31 deletions(-) > > diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S > index 07a306173c5a..6243da99b26c 100644 > --- a/arch/powerpc/kernel/idle_book3s.S > +++ b/arch/powerpc/kernel/idle_book3s.S > @@ -324,31 +324,8 @@ enter_winkle: > /* > * r3 - PSSCR value corresponding to the requested stop state. > */ > -power_enter_stop: > -/* > - * Check if we are executing the lite variant with ESL=EC=0 > - */ > - andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED > +power_enter_stop_esl: > clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */ > - bne .Lhandle_esl_ec_set > - PPC_STOP > - li r3,0 /* Since we didn't lose state, return 0 */ > - > - /* > - * pnv_wakeup_noloss() expects r12 to contain the SRR1 value so > - * it can determine if the wakeup reason is an HMI in > - * CHECK_HMI_INTERRUPT. > - * > - * However, when we wakeup with ESL=0, SRR1 will not contain the wakeup > - * reason, so there is no point setting r12 to SRR1. > - * > - * Further, we clear r12 here, so that we don't accidentally enter the > - * HMI in pnv_wakeup_noloss() if the value of r12[42:45] == WAKE_HMI. > - */ > - li r12, 0 > - b pnv_wakeup_noloss > - > -.Lhandle_esl_ec_set: > BEGIN_FTR_SECTION > /* > * POWER9 DD2.0 or earlier can incorrectly set PMAO when waking up after > @@ -423,26 +400,32 @@ ALT_FTR_SECTION_END_NESTED_IFSET(CPU_FTR_ARCH_207S, 66); \ > * r3 contains desired PSSCR register value. > */ > _GLOBAL(power9_idle_stop) > - std r3, PACA_REQ_PSSCR(r13) > mtspr SPRN_PSSCR,r3 > - LOAD_REG_ADDR(r4,power_enter_stop) > + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED > + bne 1f > + PPC_STOP > + li r3,0 /* Since we didn't lose state, return 0 */ > + blr > + > +1: std r3, PACA_REQ_PSSCR(r13) > + LOAD_REG_ADDR(r4,power_enter_stop_esl) > b pnv_powersave_common > /* No return */ Good optimization to skip the context save and directly execute stop for ESL=EC=0 case. > /* > - * Entered with MSR[EE]=0 and no soft-masked interrupts pending. > - * r3 contains desired PSSCR register value. > + * This is the same as the above, but it sets KVM state for secondaries, > + * and it must have PSSCR[EC]=1 > */ > _GLOBAL(power9_offline_stop) > - std r3, PACA_REQ_PSSCR(r13) > mtspr SPRN_PSSCR,r3 > + std r3, PACA_REQ_PSSCR(r13) > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > /* Tell KVM we're entering idle */ > li r4,KVM_HWTHREAD_IN_IDLE > /* DO THIS IN REAL MODE! See comment above. */ > stb r4,HSTATE_HWTHREAD_STATE(r13) > #endif > - LOAD_REG_ADDR(r4,power_enter_stop) > + LOAD_REG_ADDR(r4,power_enter_stop_esl) > b pnv_powersave_common > /* No return */ > > diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c > index a921d5428d76..610b1637c16f 100644 > --- a/arch/powerpc/platforms/powernv/idle.c > +++ b/arch/powerpc/platforms/powernv/idle.c > @@ -621,7 +621,12 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags, > continue; > } > > - if (max_residency_ns < residency_ns[i]) { > + /* > + * Deepest stop for unplug must be PSSCR[EC]=1 (wakeup at > + * 0x100. > + */ > + if ((max_residency_ns < residency_ns[i])&& > + (psscr_val[i] & PSSCR_EC)) { > max_residency_ns = residency_ns[i]; > pnv_deepest_stop_psscr_val = psscr_val[i]; > pnv_deepest_stop_psscr_mask = psscr_mask[i]; If firmware did not provide any ESL=EC=1 state, we can still leave threads in stop ESL=0 state. This is just a corner case or random test scenario. Why do we want to enforce that offline cpus really use a ESL=0 state or just spin? --Vaidy ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead 2018-02-28 18:34 ` Vaidyanathan Srinivasan @ 2018-03-01 11:57 ` Nicholas Piggin 2018-03-04 23:01 ` Paul Mackerras 0 siblings, 1 reply; 16+ messages in thread From: Nicholas Piggin @ 2018-03-01 11:57 UTC (permalink / raw) To: Vaidyanathan Srinivasan; +Cc: linuxppc-dev, Gautham R . Shenoy, Paul Mackerras On Thu, 1 Mar 2018 00:04:39 +0530 Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote: > * Nicholas Piggin <npiggin@gmail.com> [2017-11-18 00:08:07]: > > > When stop is executed with EC=ESL=0, it appears to execute like a > > normal instruction (resuming from NIP when woken by interrupt). So all > > the save/restore handling can be avoided completely. In particular NV > > GPRs do not have to be saved, and MSR does not have to be switched > > back to kernel MSR. > > > > So move the test for EC=ESL=0 sleep states out to power9_idle_stop, > > and return directly to the caller after stop in that case. The mtspr > > to PSSCR is moved to the top of power9_offline_stop just so it matches > > power9_idle_stop. > > > > This improves performance for ping-pong benchmark with the stop0_lite > > idle state by 2.54% for 2 threads in the same core, and 2.57% for > > different cores. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> > > > --- > > arch/powerpc/kernel/idle_book3s.S | 43 +++++++++++------------------------ > > arch/powerpc/platforms/powernv/idle.c | 7 +++++- > > 2 files changed, 19 insertions(+), 31 deletions(-) > > > > diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S > > index 07a306173c5a..6243da99b26c 100644 > > --- a/arch/powerpc/kernel/idle_book3s.S > > +++ b/arch/powerpc/kernel/idle_book3s.S > > @@ -324,31 +324,8 @@ enter_winkle: > > /* > > * r3 - PSSCR value corresponding to the requested stop state. > > */ > > -power_enter_stop: > > -/* > > - * Check if we are executing the lite variant with ESL=EC=0 > > - */ > > - andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED > > +power_enter_stop_esl: > > clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */ > > - bne .Lhandle_esl_ec_set > > - PPC_STOP > > - li r3,0 /* Since we didn't lose state, return 0 */ > > - > > - /* > > - * pnv_wakeup_noloss() expects r12 to contain the SRR1 value so > > - * it can determine if the wakeup reason is an HMI in > > - * CHECK_HMI_INTERRUPT. > > - * > > - * However, when we wakeup with ESL=0, SRR1 will not contain the wakeup > > - * reason, so there is no point setting r12 to SRR1. > > - * > > - * Further, we clear r12 here, so that we don't accidentally enter the > > - * HMI in pnv_wakeup_noloss() if the value of r12[42:45] == WAKE_HMI. > > - */ > > - li r12, 0 > > - b pnv_wakeup_noloss > > - > > -.Lhandle_esl_ec_set: > > BEGIN_FTR_SECTION > > /* > > * POWER9 DD2.0 or earlier can incorrectly set PMAO when waking up after > > @@ -423,26 +400,32 @@ ALT_FTR_SECTION_END_NESTED_IFSET(CPU_FTR_ARCH_207S, 66); \ > > * r3 contains desired PSSCR register value. > > */ > > _GLOBAL(power9_idle_stop) > > - std r3, PACA_REQ_PSSCR(r13) > > mtspr SPRN_PSSCR,r3 > > - LOAD_REG_ADDR(r4,power_enter_stop) > > + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED > > + bne 1f > > + PPC_STOP > > + li r3,0 /* Since we didn't lose state, return 0 */ > > + blr > > + > > +1: std r3, PACA_REQ_PSSCR(r13) > > + LOAD_REG_ADDR(r4,power_enter_stop_esl) > > b pnv_powersave_common > > /* No return */ > > Good optimization to skip the context save and directly execute stop > for ESL=EC=0 case. Yep we should now be getting pretty close to optimal for ESL=EC=0. About the only other thing we could do is maintain PSSCR state and only change it when going to a different idle state. May not be worth worrying about but I haven't benchmarked it. > > > /* > > - * Entered with MSR[EE]=0 and no soft-masked interrupts pending. > > - * r3 contains desired PSSCR register value. > > + * This is the same as the above, but it sets KVM state for secondaries, > > + * and it must have PSSCR[EC]=1 > > */ > > _GLOBAL(power9_offline_stop) > > - std r3, PACA_REQ_PSSCR(r13) > > mtspr SPRN_PSSCR,r3 > > + std r3, PACA_REQ_PSSCR(r13) > > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > > /* Tell KVM we're entering idle */ > > li r4,KVM_HWTHREAD_IN_IDLE > > /* DO THIS IN REAL MODE! See comment above. */ > > stb r4,HSTATE_HWTHREAD_STATE(r13) > > #endif > > - LOAD_REG_ADDR(r4,power_enter_stop) > > + LOAD_REG_ADDR(r4,power_enter_stop_esl) > > b pnv_powersave_common > > /* No return */ > > > > diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c > > index a921d5428d76..610b1637c16f 100644 > > --- a/arch/powerpc/platforms/powernv/idle.c > > +++ b/arch/powerpc/platforms/powernv/idle.c > > @@ -621,7 +621,12 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags, > > continue; > > } > > > > - if (max_residency_ns < residency_ns[i]) { > > + /* > > + * Deepest stop for unplug must be PSSCR[EC]=1 (wakeup at > > + * 0x100. > > + */ > > + if ((max_residency_ns < residency_ns[i])&& > > + (psscr_val[i] & PSSCR_EC)) { > > max_residency_ns = residency_ns[i]; > > pnv_deepest_stop_psscr_val = psscr_val[i]; > > pnv_deepest_stop_psscr_mask = psscr_mask[i]; > > If firmware did not provide any ESL=EC=1 state, we can still leave > threads in stop ESL=0 state. This is just a corner case or random > test scenario. Why do we want to enforce that offline cpus really use > a ESL=0 state or just spin? It's because power9_offline_stop only has cases for EC=ESL=1 states now. It actually looks like EC=ESL=0 unplug today is broken KVM, because the wakeup side does not check HWTHREAD_REQ, and yet they do set HWTHREAD_IN_IDLE. That would probably hang in KVM if we run with dependent threads, wouldn't it? I think banning it for now should be okay. Thanks, Nick ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead 2018-03-01 11:57 ` Nicholas Piggin @ 2018-03-04 23:01 ` Paul Mackerras 2018-03-05 9:59 ` Nicholas Piggin 0 siblings, 1 reply; 16+ messages in thread From: Paul Mackerras @ 2018-03-04 23:01 UTC (permalink / raw) To: Nicholas Piggin; +Cc: Vaidyanathan Srinivasan, linuxppc-dev, Gautham R . Shenoy On Thu, Mar 01, 2018 at 09:57:34PM +1000, Nicholas Piggin wrote: > On Thu, 1 Mar 2018 00:04:39 +0530 > Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote: > > > * Nicholas Piggin <npiggin@gmail.com> [2017-11-18 00:08:07]: [snip] > > > diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c > > > index a921d5428d76..610b1637c16f 100644 > > > --- a/arch/powerpc/platforms/powernv/idle.c > > > +++ b/arch/powerpc/platforms/powernv/idle.c > > > @@ -621,7 +621,12 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags, > > > continue; > > > } > > > > > > - if (max_residency_ns < residency_ns[i]) { > > > + /* > > > + * Deepest stop for unplug must be PSSCR[EC]=1 (wakeup at > > > + * 0x100. > > > + */ > > > + if ((max_residency_ns < residency_ns[i])&& > > > + (psscr_val[i] & PSSCR_EC)) { > > > max_residency_ns = residency_ns[i]; > > > pnv_deepest_stop_psscr_val = psscr_val[i]; > > > pnv_deepest_stop_psscr_mask = psscr_mask[i]; > > > > If firmware did not provide any ESL=EC=1 state, we can still leave > > threads in stop ESL=0 state. This is just a corner case or random > > test scenario. Why do we want to enforce that offline cpus really use > > a ESL=0 state or just spin? > > It's because power9_offline_stop only has cases for EC=ESL=1 > states now. > > It actually looks like EC=ESL=0 unplug today is broken KVM, because > the wakeup side does not check HWTHREAD_REQ, and yet they do set > HWTHREAD_IN_IDLE. That would probably hang in KVM if we run with > dependent threads, wouldn't it? Right. KVM with indep_threads_mode=N is broken at the moment if you run with powersave=off or if firmware provides no stop states with EC=ESL=1. I'm not sure what's the best way to fix that. > I think banning it for now should be okay. Banning what exactly? Paul. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead 2018-03-04 23:01 ` Paul Mackerras @ 2018-03-05 9:59 ` Nicholas Piggin 0 siblings, 0 replies; 16+ messages in thread From: Nicholas Piggin @ 2018-03-05 9:59 UTC (permalink / raw) To: Paul Mackerras; +Cc: Vaidyanathan Srinivasan, linuxppc-dev, Gautham R . Shenoy On Mon, 5 Mar 2018 10:01:01 +1100 Paul Mackerras <paulus@ozlabs.org> wrote: > On Thu, Mar 01, 2018 at 09:57:34PM +1000, Nicholas Piggin wrote: > > On Thu, 1 Mar 2018 00:04:39 +0530 > > Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote: > > > > > * Nicholas Piggin <npiggin@gmail.com> [2017-11-18 00:08:07]: > [snip] > > > > diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c > > > > index a921d5428d76..610b1637c16f 100644 > > > > --- a/arch/powerpc/platforms/powernv/idle.c > > > > +++ b/arch/powerpc/platforms/powernv/idle.c > > > > @@ -621,7 +621,12 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags, > > > > continue; > > > > } > > > > > > > > - if (max_residency_ns < residency_ns[i]) { > > > > + /* > > > > + * Deepest stop for unplug must be PSSCR[EC]=1 (wakeup at > > > > + * 0x100. > > > > + */ > > > > + if ((max_residency_ns < residency_ns[i])&& > > > > + (psscr_val[i] & PSSCR_EC)) { > > > > max_residency_ns = residency_ns[i]; > > > > pnv_deepest_stop_psscr_val = psscr_val[i]; > > > > pnv_deepest_stop_psscr_mask = psscr_mask[i]; > > > > > > If firmware did not provide any ESL=EC=1 state, we can still leave > > > threads in stop ESL=0 state. This is just a corner case or random > > > test scenario. Why do we want to enforce that offline cpus really use > > > a ESL=0 state or just spin? > > > > It's because power9_offline_stop only has cases for EC=ESL=1 > > states now. > > > > It actually looks like EC=ESL=0 unplug today is broken KVM, because > > the wakeup side does not check HWTHREAD_REQ, and yet they do set > > HWTHREAD_IN_IDLE. That would probably hang in KVM if we run with > > dependent threads, wouldn't it? > > Right. KVM with indep_threads_mode=N is broken at the moment if you > run with powersave=off or if firmware provides no stop states with > EC=ESL=1. I'm not sure what's the best way to fix that. For EC=ESL=1, would it be enough to do a test and branch right after the stop instruction? > > I think banning it for now should be okay. > > Banning what exactly? power9 CPU unplug using a EC=ESL=0 stop state. Thanks, Nick ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead 2017-11-17 14:08 ` [PATCH 3/3] powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead Nicholas Piggin 2018-02-28 18:34 ` Vaidyanathan Srinivasan @ 2018-03-31 11:46 ` Michael Ellerman 1 sibling, 0 replies; 16+ messages in thread From: Michael Ellerman @ 2018-03-31 11:46 UTC (permalink / raw) To: Nicholas Piggin, linuxppc-dev; +Cc: Gautham R . Shenoy, Nicholas Piggin Nicholas Piggin <npiggin@gmail.com> writes: > When stop is executed with EC=ESL=0, it appears to execute like a > normal instruction (resuming from NIP when woken by interrupt). So all > the save/restore handling can be avoided completely. In particular NV > GPRs do not have to be saved, and MSR does not have to be switched > back to kernel MSR. > > So move the test for EC=ESL=0 sleep states out to power9_idle_stop, > and return directly to the caller after stop in that case. The mtspr > to PSSCR is moved to the top of power9_offline_stop just so it matches > power9_idle_stop. > > This improves performance for ping-pong benchmark with the stop0_lite > idle state by 2.54% for 2 threads in the same core, and 2.57% for > different cores. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > arch/powerpc/kernel/idle_book3s.S | 43 +++++++++++------------------------ > arch/powerpc/platforms/powernv/idle.c | 7 +++++- > 2 files changed, 19 insertions(+), 31 deletions(-) > > diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S > index 07a306173c5a..6243da99b26c 100644 > --- a/arch/powerpc/kernel/idle_book3s.S > +++ b/arch/powerpc/kernel/idle_book3s.S > @@ -324,31 +324,8 @@ enter_winkle: > /* > * r3 - PSSCR value corresponding to the requested stop state. > */ > -power_enter_stop: > -/* > - * Check if we are executing the lite variant with ESL=EC=0 > - */ > - andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED > +power_enter_stop_esl: > clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */ > - bne .Lhandle_esl_ec_set > - PPC_STOP > - li r3,0 /* Since we didn't lose state, return 0 */ Sorry this clashed with Paul's force-SMT4 patches which I needed to merge to enable TM workarounds. Can you rebase? cheers ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-04-04 12:52 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-11-17 14:08 [PATCH 0/3] one more try at idle improvements Nicholas Piggin 2017-11-17 14:08 ` [PATCH 1/3] powerpc/64s/idle: POWER9 implement a separate idle stop function for hotplug Nicholas Piggin 2018-02-28 18:24 ` Vaidyanathan Srinivasan 2018-04-03 16:03 ` [1/3] " Michael Ellerman 2018-04-03 17:13 ` Nicholas Piggin 2018-04-04 12:52 ` Michael Ellerman 2017-11-17 14:08 ` [PATCH 2/3] powerpc/64s/idle: avoid sync for KVM state when waking from idle Nicholas Piggin 2018-02-28 18:16 ` Vaidyanathan Srinivasan 2018-03-01 11:38 ` Nicholas Piggin 2018-04-03 16:03 ` [2/3] " Michael Ellerman 2017-11-17 14:08 ` [PATCH 3/3] powerpc/64s/idle: POWER9 ESL=0 stop avoid save/restore overhead Nicholas Piggin 2018-02-28 18:34 ` Vaidyanathan Srinivasan 2018-03-01 11:57 ` Nicholas Piggin 2018-03-04 23:01 ` Paul Mackerras 2018-03-05 9:59 ` Nicholas Piggin 2018-03-31 11:46 ` Michael Ellerman
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.