* [PATCH v3 0/4] powerpc/64s: idle POWER9 stop improvements
@ 2017-08-25 4:30 ` Nicholas Piggin
0 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-25 4:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, kvm-ppc, Paul Mackerras
These are rebased patches leftover from the unmerged bit of
the idle series.
Based on feedback, I dropped one of the KVM patches, and reworked
the code a bit so it is easier to restore the ability for KVM to
grab secondaries into real mode.
I did a bit more benchmarking, and all up these patches improve 2
CPU ping-pong context switch benchmark on a POWER9 by around 4-6%
(depending on what CPUs and idle states are used).
Nicholas Piggin (4):
KVM: PPC: Book3S HV: POWER9 does not require secondary thread
management
powerpc/64s: idle POWER9 can execute stop without a sync sequence
powerpc/64s: idle POWER9 can execute stop in virtual mode
powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
arch/powerpc/include/asm/cpuidle.h | 16 -----
arch/powerpc/include/asm/kvm_book3s_asm.h | 4 ++
arch/powerpc/kernel/idle_book3s.S | 103 ++++++++++++++++++++++--------
arch/powerpc/kvm/book3s_hv.c | 14 +++-
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 8 +++
5 files changed, 101 insertions(+), 44 deletions(-)
--
2.13.3
^ permalink raw reply [flat|nested] 38+ messages in thread* [PATCH v3 1/4] KVM: PPC: Book3S HV: POWER9 does not require secondary thread management
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-25 4:30 ` Nicholas Piggin
-1 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-25 4:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, kvm-ppc, Paul Mackerras
POWER9 CPUs have independent MMU contexts per thread, so KVM does not
need to quiesce secondary threads, so the hwthread_req/hwthread_state
protocol does not have to be used. So patch it away on POWER9, and patch
away the branch from the Linux idle wakeup to kvm_start_guest that is
never used.
Add a warning and error out of kvmppc_grab_hwthread in case it is ever
called on POWER9.
This avoids a hwsync in the idle wakeup path on POWER9.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/kvm_book3s_asm.h | 4 ++++
arch/powerpc/kernel/idle_book3s.S | 35 +++++++++++++++++++++----------
arch/powerpc/kvm/book3s_hv.c | 14 ++++++++++++-
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 8 +++++++
4 files changed, 49 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s_asm.h b/arch/powerpc/include/asm/kvm_book3s_asm.h
index 7cea76f11c26..83596f32f50b 100644
--- a/arch/powerpc/include/asm/kvm_book3s_asm.h
+++ b/arch/powerpc/include/asm/kvm_book3s_asm.h
@@ -104,6 +104,10 @@ struct kvmppc_host_state {
u8 napping;
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+ /*
+ * hwthread_req/hwthread_state pair is used to pull sibling threads
+ * out of guest on pre-ISAv3.0B CPUs where threads share MMU.
+ */
u8 hwthread_req;
u8 hwthread_state;
u8 host_ipi;
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index bfbf0976fc09..4924647d964d 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -296,13 +296,20 @@ 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 */
+power_enter_stop_kvm_rm:
+ /*
+ * This is currently unused because POWER9 KVM does not have to
+ * gather secondary threads into sibling mode, but the code is
+ * here in case that function is required.
+ *
+ * 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
+power_enter_stop:
/*
* Check if we are executing the lite variant with ESL=EC=0
*/
@@ -465,6 +472,18 @@ pnv_powersave_wakeup_mce:
b pnv_powersave_wakeup
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+kvm_start_guest_check:
+ li r0,KVM_HWTHREAD_IN_KERNEL
+ stb r0,HSTATE_HWTHREAD_STATE(r13)
+ /* Order setting hwthread_state vs. testing hwthread_req */
+ sync
+ lbz r0,HSTATE_HWTHREAD_REQ(r13)
+ cmpwi r0,0
+ beqlr
+ b kvm_start_guest
+#endif
+
/*
* Called from reset vector for powersave wakeups.
* cr3 - set to gt if waking up with partial/complete hypervisor state loss
@@ -489,15 +508,9 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
mr r3,r12
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
- li r0,KVM_HWTHREAD_IN_KERNEL
- stb r0,HSTATE_HWTHREAD_STATE(r13)
- /* Order setting hwthread_state vs. testing hwthread_req */
- sync
- lbz r0,HSTATE_HWTHREAD_REQ(r13)
- cmpwi r0,0
- beq 1f
- b kvm_start_guest
-1:
+BEGIN_FTR_SECTION
+ bl kvm_start_guest_check
+END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
#endif
/* Return SRR1 from power7_nap() */
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 359c79cdf0cc..e34cd6fb947b 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2111,6 +2111,16 @@ static int kvmppc_grab_hwthread(int cpu)
struct paca_struct *tpaca;
long timeout = 10000;
+ /*
+ * ISA v3.0 idle routines do not set hwthread_state or test
+ * hwthread_req, so they can not grab idle threads.
+ */
+ if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+ WARN_ON(1);
+ pr_err("KVM: can not control sibling threads\n");
+ return -EBUSY;
+ }
+
tpaca = &paca[cpu];
/* Ensure the thread won't go into the kernel if it wakes */
@@ -2145,10 +2155,12 @@ static void kvmppc_release_hwthread(int cpu)
struct paca_struct *tpaca;
tpaca = &paca[cpu];
- tpaca->kvm_hstate.hwthread_req = 0;
tpaca->kvm_hstate.kvm_vcpu = NULL;
tpaca->kvm_hstate.kvm_vcore = NULL;
tpaca->kvm_hstate.kvm_split_mode = NULL;
+ if (!cpu_has_feature(CPU_FTR_ARCH_300))
+ tpaca->kvm_hstate.hwthread_req = 0;
+
}
static void radix_flush_cpu(struct kvm *kvm, int cpu, struct kvm_vcpu *vcpu)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index c52184a8efdf..3e024fd71fe8 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -149,9 +149,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
subf r4, r4, r3
mtspr SPRN_DEC, r4
+BEGIN_FTR_SECTION
/* hwthread_req may have got set by cede or no vcpu, so clear it */
li r0, 0
stb r0, HSTATE_HWTHREAD_REQ(r13)
+END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
/*
* For external interrupts we need to call the Linux
@@ -314,6 +316,7 @@ kvm_novcpu_exit:
* Relocation is off and most register values are lost.
* r13 points to the PACA.
* r3 contains the SRR1 wakeup value, SRR1 is trashed.
+ * This is not used by ISAv3.0B processors.
*/
.globl kvm_start_guest
kvm_start_guest:
@@ -432,6 +435,9 @@ kvm_secondary_got_guest:
* While waiting we also need to check if we get given a vcpu to run.
*/
kvm_no_guest:
+BEGIN_FTR_SECTION
+ twi 31,0,0
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
lbz r3, HSTATE_HWTHREAD_REQ(r13)
cmpwi r3, 0
bne 53f
@@ -2509,8 +2515,10 @@ kvm_do_nap:
clrrdi r0, r0, 1
mtspr SPRN_CTRLT, r0
+BEGIN_FTR_SECTION
li r0,1
stb r0,HSTATE_HWTHREAD_REQ(r13)
+END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
mfspr r5,SPRN_LPCR
ori r5,r5,LPCR_PECE0 | LPCR_PECE1
BEGIN_FTR_SECTION
--
2.13.3
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v3 1/4] KVM: PPC: Book3S HV: POWER9 does not require secondary thread management
@ 2017-08-25 4:30 ` Nicholas Piggin
0 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-25 4:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, kvm-ppc, Paul Mackerras
POWER9 CPUs have independent MMU contexts per thread, so KVM does not
need to quiesce secondary threads, so the hwthread_req/hwthread_state
protocol does not have to be used. So patch it away on POWER9, and patch
away the branch from the Linux idle wakeup to kvm_start_guest that is
never used.
Add a warning and error out of kvmppc_grab_hwthread in case it is ever
called on POWER9.
This avoids a hwsync in the idle wakeup path on POWER9.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/kvm_book3s_asm.h | 4 ++++
arch/powerpc/kernel/idle_book3s.S | 35 +++++++++++++++++++++----------
arch/powerpc/kvm/book3s_hv.c | 14 ++++++++++++-
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 8 +++++++
4 files changed, 49 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s_asm.h b/arch/powerpc/include/asm/kvm_book3s_asm.h
index 7cea76f11c26..83596f32f50b 100644
--- a/arch/powerpc/include/asm/kvm_book3s_asm.h
+++ b/arch/powerpc/include/asm/kvm_book3s_asm.h
@@ -104,6 +104,10 @@ struct kvmppc_host_state {
u8 napping;
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+ /*
+ * hwthread_req/hwthread_state pair is used to pull sibling threads
+ * out of guest on pre-ISAv3.0B CPUs where threads share MMU.
+ */
u8 hwthread_req;
u8 hwthread_state;
u8 host_ipi;
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index bfbf0976fc09..4924647d964d 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -296,13 +296,20 @@ 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 */
+power_enter_stop_kvm_rm:
+ /*
+ * This is currently unused because POWER9 KVM does not have to
+ * gather secondary threads into sibling mode, but the code is
+ * here in case that function is required.
+ *
+ * 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
+power_enter_stop:
/*
* Check if we are executing the lite variant with ESLì=0
*/
@@ -465,6 +472,18 @@ pnv_powersave_wakeup_mce:
b pnv_powersave_wakeup
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+kvm_start_guest_check:
+ li r0,KVM_HWTHREAD_IN_KERNEL
+ stb r0,HSTATE_HWTHREAD_STATE(r13)
+ /* Order setting hwthread_state vs. testing hwthread_req */
+ sync
+ lbz r0,HSTATE_HWTHREAD_REQ(r13)
+ cmpwi r0,0
+ beqlr
+ b kvm_start_guest
+#endif
+
/*
* Called from reset vector for powersave wakeups.
* cr3 - set to gt if waking up with partial/complete hypervisor state loss
@@ -489,15 +508,9 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
mr r3,r12
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
- li r0,KVM_HWTHREAD_IN_KERNEL
- stb r0,HSTATE_HWTHREAD_STATE(r13)
- /* Order setting hwthread_state vs. testing hwthread_req */
- sync
- lbz r0,HSTATE_HWTHREAD_REQ(r13)
- cmpwi r0,0
- beq 1f
- b kvm_start_guest
-1:
+BEGIN_FTR_SECTION
+ bl kvm_start_guest_check
+END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
#endif
/* Return SRR1 from power7_nap() */
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 359c79cdf0cc..e34cd6fb947b 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2111,6 +2111,16 @@ static int kvmppc_grab_hwthread(int cpu)
struct paca_struct *tpaca;
long timeout = 10000;
+ /*
+ * ISA v3.0 idle routines do not set hwthread_state or test
+ * hwthread_req, so they can not grab idle threads.
+ */
+ if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+ WARN_ON(1);
+ pr_err("KVM: can not control sibling threads\n");
+ return -EBUSY;
+ }
+
tpaca = &paca[cpu];
/* Ensure the thread won't go into the kernel if it wakes */
@@ -2145,10 +2155,12 @@ static void kvmppc_release_hwthread(int cpu)
struct paca_struct *tpaca;
tpaca = &paca[cpu];
- tpaca->kvm_hstate.hwthread_req = 0;
tpaca->kvm_hstate.kvm_vcpu = NULL;
tpaca->kvm_hstate.kvm_vcore = NULL;
tpaca->kvm_hstate.kvm_split_mode = NULL;
+ if (!cpu_has_feature(CPU_FTR_ARCH_300))
+ tpaca->kvm_hstate.hwthread_req = 0;
+
}
static void radix_flush_cpu(struct kvm *kvm, int cpu, struct kvm_vcpu *vcpu)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index c52184a8efdf..3e024fd71fe8 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -149,9 +149,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
subf r4, r4, r3
mtspr SPRN_DEC, r4
+BEGIN_FTR_SECTION
/* hwthread_req may have got set by cede or no vcpu, so clear it */
li r0, 0
stb r0, HSTATE_HWTHREAD_REQ(r13)
+END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
/*
* For external interrupts we need to call the Linux
@@ -314,6 +316,7 @@ kvm_novcpu_exit:
* Relocation is off and most register values are lost.
* r13 points to the PACA.
* r3 contains the SRR1 wakeup value, SRR1 is trashed.
+ * This is not used by ISAv3.0B processors.
*/
.globl kvm_start_guest
kvm_start_guest:
@@ -432,6 +435,9 @@ kvm_secondary_got_guest:
* While waiting we also need to check if we get given a vcpu to run.
*/
kvm_no_guest:
+BEGIN_FTR_SECTION
+ twi 31,0,0
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
lbz r3, HSTATE_HWTHREAD_REQ(r13)
cmpwi r3, 0
bne 53f
@@ -2509,8 +2515,10 @@ kvm_do_nap:
clrrdi r0, r0, 1
mtspr SPRN_CTRLT, r0
+BEGIN_FTR_SECTION
li r0,1
stb r0,HSTATE_HWTHREAD_REQ(r13)
+END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
mfspr r5,SPRN_LPCR
ori r5,r5,LPCR_PECE0 | LPCR_PECE1
BEGIN_FTR_SECTION
--
2.13.3
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v3 1/4] KVM: PPC: Book3S HV: POWER9 does not require secondary thread management
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-28 11:49 ` Michael Ellerman
-1 siblings, 0 replies; 38+ messages in thread
From: Michael Ellerman @ 2017-08-28 11:49 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev, Paul Mackerras; +Cc: kvm-ppc, Nicholas Piggin
Nicholas Piggin <npiggin@gmail.com> writes:
> POWER9 CPUs have independent MMU contexts per thread, so KVM does not
> need to quiesce secondary threads, so the hwthread_req/hwthread_state
> protocol does not have to be used. So patch it away on POWER9, and patch
> away the branch from the Linux idle wakeup to kvm_start_guest that is
> never used.
>
> Add a warning and error out of kvmppc_grab_hwthread in case it is ever
> called on POWER9.
>
> This avoids a hwsync in the idle wakeup path on POWER9.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/kvm_book3s_asm.h | 4 ++++
> arch/powerpc/kernel/idle_book3s.S | 35 +++++++++++++++++++++----------
> arch/powerpc/kvm/book3s_hv.c | 14 ++++++++++++-
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 8 +++++++
> 4 files changed, 49 insertions(+), 12 deletions(-)
Paulus do you mind acking this and I'll put it in my ppc-kvm topic
branch.
cheers
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 1/4] KVM: PPC: Book3S HV: POWER9 does not require secondary thread management
@ 2017-08-28 11:49 ` Michael Ellerman
0 siblings, 0 replies; 38+ messages in thread
From: Michael Ellerman @ 2017-08-28 11:49 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev, Paul Mackerras; +Cc: kvm-ppc, Nicholas Piggin
Nicholas Piggin <npiggin@gmail.com> writes:
> POWER9 CPUs have independent MMU contexts per thread, so KVM does not
> need to quiesce secondary threads, so the hwthread_req/hwthread_state
> protocol does not have to be used. So patch it away on POWER9, and patch
> away the branch from the Linux idle wakeup to kvm_start_guest that is
> never used.
>
> Add a warning and error out of kvmppc_grab_hwthread in case it is ever
> called on POWER9.
>
> This avoids a hwsync in the idle wakeup path on POWER9.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/kvm_book3s_asm.h | 4 ++++
> arch/powerpc/kernel/idle_book3s.S | 35 +++++++++++++++++++++----------
> arch/powerpc/kvm/book3s_hv.c | 14 ++++++++++++-
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 8 +++++++
> 4 files changed, 49 insertions(+), 12 deletions(-)
Paulus do you mind acking this and I'll put it in my ppc-kvm topic
branch.
cheers
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 1/4] KVM: PPC: Book3S HV: POWER9 does not require secondary thread management
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-29 0:13 ` Paul Mackerras
-1 siblings, 0 replies; 38+ messages in thread
From: Paul Mackerras @ 2017-08-29 0:13 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
On Fri, Aug 25, 2017 at 02:30:33PM +1000, Nicholas Piggin wrote:
> POWER9 CPUs have independent MMU contexts per thread, so KVM does not
> need to quiesce secondary threads, so the hwthread_req/hwthread_state
> protocol does not have to be used. So patch it away on POWER9, and patch
> away the branch from the Linux idle wakeup to kvm_start_guest that is
> never used.
>
> Add a warning and error out of kvmppc_grab_hwthread in case it is ever
> called on POWER9.
>
> This avoids a hwsync in the idle wakeup path on POWER9.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Some of the feature sections seem a little unnecessary except for
documentation purposes (e.g. the second one added after the
kvm_no_guest label), but they won't actually hurt, so:
Acked-by: Paul Mackerras <paulus@ozlabs.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 1/4] KVM: PPC: Book3S HV: POWER9 does not require secondary thread management
@ 2017-08-29 0:13 ` Paul Mackerras
0 siblings, 0 replies; 38+ messages in thread
From: Paul Mackerras @ 2017-08-29 0:13 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
On Fri, Aug 25, 2017 at 02:30:33PM +1000, Nicholas Piggin wrote:
> POWER9 CPUs have independent MMU contexts per thread, so KVM does not
> need to quiesce secondary threads, so the hwthread_req/hwthread_state
> protocol does not have to be used. So patch it away on POWER9, and patch
> away the branch from the Linux idle wakeup to kvm_start_guest that is
> never used.
>
> Add a warning and error out of kvmppc_grab_hwthread in case it is ever
> called on POWER9.
>
> This avoids a hwsync in the idle wakeup path on POWER9.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Some of the feature sections seem a little unnecessary except for
documentation purposes (e.g. the second one added after the
kvm_no_guest label), but they won't actually hurt, so:
Acked-by: Paul Mackerras <paulus@ozlabs.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v3, 1/4] KVM: PPC: Book3S HV: POWER9 does not require secondary thread management
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-31 11:36 ` Michael Ellerman
-1 siblings, 0 replies; 38+ messages in thread
From: Michael Ellerman @ 2017-08-31 11:36 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: kvm-ppc, Nicholas Piggin
On Fri, 2017-08-25 at 04:30:33 UTC, Nicholas Piggin wrote:
> POWER9 CPUs have independent MMU contexts per thread, so KVM does not
> need to quiesce secondary threads, so the hwthread_req/hwthread_state
> protocol does not have to be used. So patch it away on POWER9, and patch
> away the branch from the Linux idle wakeup to kvm_start_guest that is
> never used.
>
> Add a warning and error out of kvmppc_grab_hwthread in case it is ever
> called on POWER9.
>
> This avoids a hwsync in the idle wakeup path on POWER9.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Acked-by: Paul Mackerras <paulus@ozlabs.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/94a04bc25a2c6296bd0c5e82c10e82
cheers
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v3, 1/4] KVM: PPC: Book3S HV: POWER9 does not require secondary thread management
@ 2017-08-31 11:36 ` Michael Ellerman
0 siblings, 0 replies; 38+ messages in thread
From: Michael Ellerman @ 2017-08-31 11:36 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: kvm-ppc, Nicholas Piggin
On Fri, 2017-08-25 at 04:30:33 UTC, Nicholas Piggin wrote:
> POWER9 CPUs have independent MMU contexts per thread, so KVM does not
> need to quiesce secondary threads, so the hwthread_req/hwthread_state
> protocol does not have to be used. So patch it away on POWER9, and patch
> away the branch from the Linux idle wakeup to kvm_start_guest that is
> never used.
>
> Add a warning and error out of kvmppc_grab_hwthread in case it is ever
> called on POWER9.
>
> This avoids a hwsync in the idle wakeup path on POWER9.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Acked-by: Paul Mackerras <paulus@ozlabs.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/94a04bc25a2c6296bd0c5e82c10e82
cheers
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v3 2/4] powerpc/64s: idle POWER9 can execute stop without a sync sequence
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-25 4:30 ` Nicholas Piggin
-1 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-25 4:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, kvm-ppc, Paul Mackerras
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/cpuidle.h | 16 ----------------
arch/powerpc/kernel/idle_book3s.S | 26 ++++++++++++++++++++------
2 files changed, 20 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
index 8a174cba5567..eb43b5c3a7b5 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -101,20 +101,4 @@ static inline void report_invalid_psscr_val(u64 psscr_val, int err)
#endif
-/* Idle state entry routines */
-#ifdef CONFIG_PPC_P7_NAP
-#define IDLE_STATE_ENTER_SEQ(IDLE_INST) \
- /* Magic NAP/SLEEP/WINKLE mode enter sequence */ \
- std r0,0(r1); \
- ptesync; \
- ld r0,0(r1); \
-236: cmpd cr0,r0,r0; \
- bne 236b; \
- IDLE_INST; \
-
-#define IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST) \
- IDLE_STATE_ENTER_SEQ(IDLE_INST) \
- b .
-#endif /* CONFIG_PPC_P7_NAP */
-
#endif
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 4924647d964d..14e97f442167 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -205,6 +205,19 @@ pnv_powersave_common:
mtmsrd r7,0
bctr
+/*
+ * This is the sequence required to execute idle instructions, as
+ * specified in ISA v2.07. MSR[IR] and MSR[DR] must be 0.
+ */
+#define ARCH207_IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST) \
+ /* Magic NAP/SLEEP/WINKLE mode enter sequence */ \
+ std r0,0(r1); \
+ ptesync; \
+ ld r0,0(r1); \
+236: cmpd cr0,r0,r0; \
+ bne 236b; \
+ IDLE_INST;
+
.globl pnv_enter_arch207_idle_mode
pnv_enter_arch207_idle_mode:
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
@@ -230,7 +243,7 @@ pnv_enter_arch207_idle_mode:
stb r3,PACA_THREAD_IDLE_STATE(r13)
cmpwi cr3,r3,PNV_THREAD_SLEEP
bge cr3,2f
- IDLE_STATE_ENTER_SEQ_NORET(PPC_NAP)
+ ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_NAP)
/* No return */
2:
/* Sleep or winkle */
@@ -269,7 +282,7 @@ pnv_fastsleep_workaround_at_entry:
common_enter: /* common code for all the threads entering sleep or winkle */
bgt cr3,enter_winkle
- IDLE_STATE_ENTER_SEQ_NORET(PPC_SLEEP)
+ ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_SLEEP)
fastsleep_workaround_at_entry:
oris r15,r15,PNV_CORE_IDLE_LOCK_BIT@h
@@ -291,7 +304,7 @@ fastsleep_workaround_at_entry:
enter_winkle:
bl save_sprs_to_stack
- IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
+ ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
/*
* r3 - PSSCR value corresponding to the requested stop state.
@@ -316,7 +329,7 @@ power_enter_stop:
andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
bne .Lhandle_esl_ec_set
- IDLE_STATE_ENTER_SEQ(PPC_STOP)
+ PPC_STOP
li r3,0 /* Since we didn't lose state, return 0 */
/*
@@ -349,7 +362,8 @@ power_enter_stop:
ld r4,ADDROFF(pnv_first_deep_stop_state)(r5)
cmpd r3,r4
bge .Lhandle_deep_stop
- IDLE_STATE_ENTER_SEQ_NORET(PPC_STOP)
+ PPC_STOP /* Does not return (system reset interrupt) */
+
.Lhandle_deep_stop:
/*
* Entering deep idle state.
@@ -371,7 +385,7 @@ lwarx_loop_stop:
bl save_sprs_to_stack
- IDLE_STATE_ENTER_SEQ_NORET(PPC_STOP)
+ PPC_STOP /* Does not return (system reset interrupt) */
/*
* Entered with MSR[EE]=0 and no soft-masked interrupts pending.
--
2.13.3
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 2/4] powerpc/64s: idle POWER9 can execute stop without a sync sequence
@ 2017-08-25 4:30 ` Nicholas Piggin
0 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-25 4:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, kvm-ppc, Paul Mackerras
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/cpuidle.h | 16 ----------------
arch/powerpc/kernel/idle_book3s.S | 26 ++++++++++++++++++++------
2 files changed, 20 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
index 8a174cba5567..eb43b5c3a7b5 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -101,20 +101,4 @@ static inline void report_invalid_psscr_val(u64 psscr_val, int err)
#endif
-/* Idle state entry routines */
-#ifdef CONFIG_PPC_P7_NAP
-#define IDLE_STATE_ENTER_SEQ(IDLE_INST) \
- /* Magic NAP/SLEEP/WINKLE mode enter sequence */ \
- std r0,0(r1); \
- ptesync; \
- ld r0,0(r1); \
-236: cmpd cr0,r0,r0; \
- bne 236b; \
- IDLE_INST; \
-
-#define IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST) \
- IDLE_STATE_ENTER_SEQ(IDLE_INST) \
- b .
-#endif /* CONFIG_PPC_P7_NAP */
-
#endif
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 4924647d964d..14e97f442167 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -205,6 +205,19 @@ pnv_powersave_common:
mtmsrd r7,0
bctr
+/*
+ * This is the sequence required to execute idle instructions, as
+ * specified in ISA v2.07. MSR[IR] and MSR[DR] must be 0.
+ */
+#define ARCH207_IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST) \
+ /* Magic NAP/SLEEP/WINKLE mode enter sequence */ \
+ std r0,0(r1); \
+ ptesync; \
+ ld r0,0(r1); \
+236: cmpd cr0,r0,r0; \
+ bne 236b; \
+ IDLE_INST;
+
.globl pnv_enter_arch207_idle_mode
pnv_enter_arch207_idle_mode:
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
@@ -230,7 +243,7 @@ pnv_enter_arch207_idle_mode:
stb r3,PACA_THREAD_IDLE_STATE(r13)
cmpwi cr3,r3,PNV_THREAD_SLEEP
bge cr3,2f
- IDLE_STATE_ENTER_SEQ_NORET(PPC_NAP)
+ ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_NAP)
/* No return */
2:
/* Sleep or winkle */
@@ -269,7 +282,7 @@ pnv_fastsleep_workaround_at_entry:
common_enter: /* common code for all the threads entering sleep or winkle */
bgt cr3,enter_winkle
- IDLE_STATE_ENTER_SEQ_NORET(PPC_SLEEP)
+ ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_SLEEP)
fastsleep_workaround_at_entry:
oris r15,r15,PNV_CORE_IDLE_LOCK_BIT@h
@@ -291,7 +304,7 @@ fastsleep_workaround_at_entry:
enter_winkle:
bl save_sprs_to_stack
- IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
+ ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
/*
* r3 - PSSCR value corresponding to the requested stop state.
@@ -316,7 +329,7 @@ power_enter_stop:
andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
bne .Lhandle_esl_ec_set
- IDLE_STATE_ENTER_SEQ(PPC_STOP)
+ PPC_STOP
li r3,0 /* Since we didn't lose state, return 0 */
/*
@@ -349,7 +362,8 @@ power_enter_stop:
ld r4,ADDROFF(pnv_first_deep_stop_state)(r5)
cmpd r3,r4
bge .Lhandle_deep_stop
- IDLE_STATE_ENTER_SEQ_NORET(PPC_STOP)
+ PPC_STOP /* Does not return (system reset interrupt) */
+
.Lhandle_deep_stop:
/*
* Entering deep idle state.
@@ -371,7 +385,7 @@ lwarx_loop_stop:
bl save_sprs_to_stack
- IDLE_STATE_ENTER_SEQ_NORET(PPC_STOP)
+ PPC_STOP /* Does not return (system reset interrupt) */
/*
* Entered with MSR[EE]=0 and no soft-masked interrupts pending.
--
2.13.3
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v3 2/4] powerpc/64s: idle POWER9 can execute stop without a sync sequence
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-29 0:11 ` Paul Mackerras
-1 siblings, 0 replies; 38+ messages in thread
From: Paul Mackerras @ 2017-08-29 0:11 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
On Fri, Aug 25, 2017 at 02:30:34PM +1000, Nicholas Piggin wrote:
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/cpuidle.h | 16 ----------------
> arch/powerpc/kernel/idle_book3s.S | 26 ++++++++++++++++++++------
> 2 files changed, 20 insertions(+), 22 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
> index 8a174cba5567..eb43b5c3a7b5 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -101,20 +101,4 @@ static inline void report_invalid_psscr_val(u64 psscr_val, int err)
>
> #endif
>
> -/* Idle state entry routines */
> -#ifdef CONFIG_PPC_P7_NAP
> -#define IDLE_STATE_ENTER_SEQ(IDLE_INST) \
> - /* Magic NAP/SLEEP/WINKLE mode enter sequence */ \
> - std r0,0(r1); \
> - ptesync; \
> - ld r0,0(r1); \
> -236: cmpd cr0,r0,r0; \
> - bne 236b; \
> - IDLE_INST; \
> -
> -#define IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST) \
> - IDLE_STATE_ENTER_SEQ(IDLE_INST) \
> - b .
> -#endif /* CONFIG_PPC_P7_NAP */
> -
> #endif
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 4924647d964d..14e97f442167 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -205,6 +205,19 @@ pnv_powersave_common:
> mtmsrd r7,0
> bctr
>
> +/*
> + * This is the sequence required to execute idle instructions, as
> + * specified in ISA v2.07. MSR[IR] and MSR[DR] must be 0.
> + */
> +#define ARCH207_IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST) \
We had to do this sequence on POWER7 also, which is architecture
v2.06. Thus the comments and the naming (ARCH207_*) are a bit
misleading here. The actual code change looks OK.
Paul.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 2/4] powerpc/64s: idle POWER9 can execute stop without a sync sequence
@ 2017-08-29 0:11 ` Paul Mackerras
0 siblings, 0 replies; 38+ messages in thread
From: Paul Mackerras @ 2017-08-29 0:11 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
On Fri, Aug 25, 2017 at 02:30:34PM +1000, Nicholas Piggin wrote:
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/cpuidle.h | 16 ----------------
> arch/powerpc/kernel/idle_book3s.S | 26 ++++++++++++++++++++------
> 2 files changed, 20 insertions(+), 22 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
> index 8a174cba5567..eb43b5c3a7b5 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -101,20 +101,4 @@ static inline void report_invalid_psscr_val(u64 psscr_val, int err)
>
> #endif
>
> -/* Idle state entry routines */
> -#ifdef CONFIG_PPC_P7_NAP
> -#define IDLE_STATE_ENTER_SEQ(IDLE_INST) \
> - /* Magic NAP/SLEEP/WINKLE mode enter sequence */ \
> - std r0,0(r1); \
> - ptesync; \
> - ld r0,0(r1); \
> -236: cmpd cr0,r0,r0; \
> - bne 236b; \
> - IDLE_INST; \
> -
> -#define IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST) \
> - IDLE_STATE_ENTER_SEQ(IDLE_INST) \
> - b .
> -#endif /* CONFIG_PPC_P7_NAP */
> -
> #endif
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 4924647d964d..14e97f442167 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -205,6 +205,19 @@ pnv_powersave_common:
> mtmsrd r7,0
> bctr
>
> +/*
> + * This is the sequence required to execute idle instructions, as
> + * specified in ISA v2.07. MSR[IR] and MSR[DR] must be 0.
> + */
> +#define ARCH207_IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST) \
We had to do this sequence on POWER7 also, which is architecture
v2.06. Thus the comments and the naming (ARCH207_*) are a bit
misleading here. The actual code change looks OK.
Paul.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 2/4] powerpc/64s: idle POWER9 can execute stop without a sync sequence
2017-08-29 0:11 ` Paul Mackerras
@ 2017-08-29 10:30 ` Michael Ellerman
-1 siblings, 0 replies; 38+ messages in thread
From: Michael Ellerman @ 2017-08-29 10:30 UTC (permalink / raw)
To: Paul Mackerras, Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
Paul Mackerras <paulus@ozlabs.org> writes:
> On Fri, Aug 25, 2017 at 02:30:34PM +1000, Nicholas Piggin wrote:
>> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
>> index 4924647d964d..14e97f442167 100644
>> --- a/arch/powerpc/kernel/idle_book3s.S
>> +++ b/arch/powerpc/kernel/idle_book3s.S
>> @@ -205,6 +205,19 @@ pnv_powersave_common:
>> mtmsrd r7,0
>> bctr
>>
>> +/*
>> + * This is the sequence required to execute idle instructions, as
>> + * specified in ISA v2.07. MSR[IR] and MSR[DR] must be 0.
>> + */
>> +#define ARCH207_IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST) \
>
> We had to do this sequence on POWER7 also, which is architecture
> v2.06. Thus the comments and the naming (ARCH207_*) are a bit
> misleading here. The actual code change looks OK.
I'll just drop the name change, I don't think it's crucial. That makes
P9 the special case.
We can come up with a better name or something in future.
Unless Nick objects?
cheers
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 2/4] powerpc/64s: idle POWER9 can execute stop without a sync sequence
@ 2017-08-29 10:30 ` Michael Ellerman
0 siblings, 0 replies; 38+ messages in thread
From: Michael Ellerman @ 2017-08-29 10:30 UTC (permalink / raw)
To: Paul Mackerras, Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
Paul Mackerras <paulus@ozlabs.org> writes:
> On Fri, Aug 25, 2017 at 02:30:34PM +1000, Nicholas Piggin wrote:
>> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
>> index 4924647d964d..14e97f442167 100644
>> --- a/arch/powerpc/kernel/idle_book3s.S
>> +++ b/arch/powerpc/kernel/idle_book3s.S
>> @@ -205,6 +205,19 @@ pnv_powersave_common:
>> mtmsrd r7,0
>> bctr
>>
>> +/*
>> + * This is the sequence required to execute idle instructions, as
>> + * specified in ISA v2.07. MSR[IR] and MSR[DR] must be 0.
>> + */
>> +#define ARCH207_IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST) \
>
> We had to do this sequence on POWER7 also, which is architecture
> v2.06. Thus the comments and the naming (ARCH207_*) are a bit
> misleading here. The actual code change looks OK.
I'll just drop the name change, I don't think it's crucial. That makes
P9 the special case.
We can come up with a better name or something in future.
Unless Nick objects?
cheers
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v3, 2/4] powerpc/64s: idle POWER9 can execute stop without a sync sequence
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-31 11:36 ` Michael Ellerman
-1 siblings, 0 replies; 38+ messages in thread
From: Michael Ellerman @ 2017-08-31 11:36 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: kvm-ppc, Nicholas Piggin
On Fri, 2017-08-25 at 04:30:34 UTC, Nicholas Piggin wrote:
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/56ee52408ed0bd4af400c04ad60f98
cheers
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v3 3/4] powerpc/64s: idle POWER9 can execute stop in virtual mode
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-25 4:30 ` Nicholas Piggin
-1 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-25 4:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, kvm-ppc, Paul Mackerras
The hardware can execute stop in any context, and KVM does not
require real mode because siblings do not share MMU state. This
saves a switch to real-mode when going idle.
Acked-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/idle_book3s.S | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 14e97f442167..32d65ee323a0 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -195,7 +195,16 @@ pnv_powersave_common:
std r5,_CCR(r1)
std r1,PACAR1(r13)
+BEGIN_FTR_SECTION
+ /*
+ * POWER9 does not require real mode to stop, and presently does not
+ * set hwthread_state for KVM (threads don't share MMU context), so
+ * we can remain in virtual mode for this.
+ */
+ bctr
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
/*
+ * POWER8
* Go to real mode to do the nap, as required by the architecture.
* Also, we need to be in real mode before setting hwthread_state,
* because as soon as we do that, another thread can switch
--
2.13.3
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 3/4] powerpc/64s: idle POWER9 can execute stop in virtual mode
@ 2017-08-25 4:30 ` Nicholas Piggin
0 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-25 4:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, kvm-ppc, Paul Mackerras
The hardware can execute stop in any context, and KVM does not
require real mode because siblings do not share MMU state. This
saves a switch to real-mode when going idle.
Acked-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/idle_book3s.S | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 14e97f442167..32d65ee323a0 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -195,7 +195,16 @@ pnv_powersave_common:
std r5,_CCR(r1)
std r1,PACAR1(r13)
+BEGIN_FTR_SECTION
+ /*
+ * POWER9 does not require real mode to stop, and presently does not
+ * set hwthread_state for KVM (threads don't share MMU context), so
+ * we can remain in virtual mode for this.
+ */
+ bctr
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
/*
+ * POWER8
* Go to real mode to do the nap, as required by the architecture.
* Also, we need to be in real mode before setting hwthread_state,
* because as soon as we do that, another thread can switch
--
2.13.3
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v3 3/4] powerpc/64s: idle POWER9 can execute stop in virtual mode
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-29 0:14 ` Paul Mackerras
-1 siblings, 0 replies; 38+ messages in thread
From: Paul Mackerras @ 2017-08-29 0:14 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
On Fri, Aug 25, 2017 at 02:30:35PM +1000, Nicholas Piggin wrote:
> The hardware can execute stop in any context, and KVM does not
> require real mode because siblings do not share MMU state. This
> saves a switch to real-mode when going idle.
>
> Acked-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/idle_book3s.S | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 14e97f442167..32d65ee323a0 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -195,7 +195,16 @@ pnv_powersave_common:
> std r5,_CCR(r1)
> std r1,PACAR1(r13)
>
> +BEGIN_FTR_SECTION
> + /*
> + * POWER9 does not require real mode to stop, and presently does not
> + * set hwthread_state for KVM (threads don't share MMU context), so
> + * we can remain in virtual mode for this.
> + */
> + bctr
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> /*
> + * POWER8
... and POWER7 too.
Since that's just a comment,
Reviewed-by: Paul Mackerras <paulus@ozlabs.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 3/4] powerpc/64s: idle POWER9 can execute stop in virtual mode
@ 2017-08-29 0:14 ` Paul Mackerras
0 siblings, 0 replies; 38+ messages in thread
From: Paul Mackerras @ 2017-08-29 0:14 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
On Fri, Aug 25, 2017 at 02:30:35PM +1000, Nicholas Piggin wrote:
> The hardware can execute stop in any context, and KVM does not
> require real mode because siblings do not share MMU state. This
> saves a switch to real-mode when going idle.
>
> Acked-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/idle_book3s.S | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 14e97f442167..32d65ee323a0 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -195,7 +195,16 @@ pnv_powersave_common:
> std r5,_CCR(r1)
> std r1,PACAR1(r13)
>
> +BEGIN_FTR_SECTION
> + /*
> + * POWER9 does not require real mode to stop, and presently does not
> + * set hwthread_state for KVM (threads don't share MMU context), so
> + * we can remain in virtual mode for this.
> + */
> + bctr
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> /*
> + * POWER8
... and POWER7 too.
Since that's just a comment,
Reviewed-by: Paul Mackerras <paulus@ozlabs.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [v3, 3/4] powerpc/64s: idle POWER9 can execute stop in virtual mode
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-31 11:36 ` Michael Ellerman
-1 siblings, 0 replies; 38+ messages in thread
From: Michael Ellerman @ 2017-08-31 11:36 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: kvm-ppc, Nicholas Piggin
On Fri, 2017-08-25 at 04:30:35 UTC, Nicholas Piggin wrote:
> The hardware can execute stop in any context, and KVM does not
> require real mode because siblings do not share MMU state. This
> saves a switch to real-mode when going idle.
>
> Acked-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Reviewed-by: Paul Mackerras <paulus@ozlabs.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/72b0d51d973beab5a06c97279b61a0
cheers
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-25 4:30 ` Nicholas Piggin
-1 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-25 4:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, kvm-ppc, 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 "lite" sleep states out to power9_idle_stop.
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 32d65ee323a0..fa56120bd0bc 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -315,9 +315,6 @@ enter_winkle:
ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
-/*
- * r3 - PSSCR value corresponding to the requested stop state.
- */
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
power_enter_stop_kvm_rm:
/*
@@ -330,14 +327,11 @@ power_enter_stop_kvm_rm:
li r4,KVM_HWTHREAD_IN_IDLE
/* DO THIS IN REAL MODE! See comment above. */
stb r4,HSTATE_HWTHREAD_STATE(r13)
-#endif
-power_enter_stop:
/*
* Check if we are executing the lite variant with ESL=EC=0
*/
- andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
- clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
- bne .Lhandle_esl_ec_set
+ andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
+ bne power_enter_stop_esl
PPC_STOP
li r3,0 /* Since we didn't lose state, return 0 */
@@ -354,8 +348,13 @@ power_enter_stop:
*/
li r12, 0
b pnv_wakeup_noloss
+#endif
-.Lhandle_esl_ec_set:
+/*
+ * r3 - PSSCR value corresponding to the requested stop state.
+ */
+power_enter_stop_esl:
+ clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
/*
* POWER9 DD2 can incorrectly set PMAO when waking up after a
* state-loss idle. Saving and restoring MMCR0 over idle is a
@@ -428,9 +427,23 @@ 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)
+
+ /*
+ * Check if we are executing the lite variant with ESL=EC=0
+ * This case resumes execution after the stop instruction without
+ * losing any state, so nothing has to be saved. The following
+ * instructions up to the blr must be skipped if we want to
+ * use power_enter_stop_kvm_rm.
+ */
+ 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: /* state-loss idle */
+ std r3, PACA_REQ_PSSCR(r13)
+ LOAD_REG_ADDR(r4,power_enter_stop_esl)
b pnv_powersave_common
/* No return */
--
2.13.3
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
@ 2017-08-25 4:30 ` Nicholas Piggin
0 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-25 4:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, kvm-ppc, 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 "lite" sleep states out to power9_idle_stop.
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 32d65ee323a0..fa56120bd0bc 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -315,9 +315,6 @@ enter_winkle:
ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
-/*
- * r3 - PSSCR value corresponding to the requested stop state.
- */
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
power_enter_stop_kvm_rm:
/*
@@ -330,14 +327,11 @@ power_enter_stop_kvm_rm:
li r4,KVM_HWTHREAD_IN_IDLE
/* DO THIS IN REAL MODE! See comment above. */
stb r4,HSTATE_HWTHREAD_STATE(r13)
-#endif
-power_enter_stop:
/*
* Check if we are executing the lite variant with ESLì=0
*/
- andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
- clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
- bne .Lhandle_esl_ec_set
+ andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
+ bne power_enter_stop_esl
PPC_STOP
li r3,0 /* Since we didn't lose state, return 0 */
@@ -354,8 +348,13 @@ power_enter_stop:
*/
li r12, 0
b pnv_wakeup_noloss
+#endif
-.Lhandle_esl_ec_set:
+/*
+ * r3 - PSSCR value corresponding to the requested stop state.
+ */
+power_enter_stop_esl:
+ clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
/*
* POWER9 DD2 can incorrectly set PMAO when waking up after a
* state-loss idle. Saving and restoring MMCR0 over idle is a
@@ -428,9 +427,23 @@ 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)
+
+ /*
+ * Check if we are executing the lite variant with ESLì=0
+ * This case resumes execution after the stop instruction without
+ * losing any state, so nothing has to be saved. The following
+ * instructions up to the blr must be skipped if we want to
+ * use power_enter_stop_kvm_rm.
+ */
+ 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: /* state-loss idle */
+ std r3, PACA_REQ_PSSCR(r13)
+ LOAD_REG_ADDR(r4,power_enter_stop_esl)
b pnv_powersave_common
/* No return */
--
2.13.3
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-29 0:20 ` Paul Mackerras
-1 siblings, 0 replies; 38+ messages in thread
From: Paul Mackerras @ 2017-08-29 0:20 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
On Fri, Aug 25, 2017 at 02:30:36PM +1000, Nicholas Piggin wrote:
> 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 "lite" sleep states out to power9_idle_stop.
>
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 32d65ee323a0..fa56120bd0bc 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -315,9 +315,6 @@ enter_winkle:
>
> ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
>
> -/*
> - * r3 - PSSCR value corresponding to the requested stop state.
> - */
> #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> power_enter_stop_kvm_rm:
> /*
> @@ -330,14 +327,11 @@ power_enter_stop_kvm_rm:
> li r4,KVM_HWTHREAD_IN_IDLE
> /* DO THIS IN REAL MODE! See comment above. */
> stb r4,HSTATE_HWTHREAD_STATE(r13)
> -#endif
> -power_enter_stop:
> /*
> * Check if we are executing the lite variant with ESL=EC=0
> */
> - andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
> - clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
> - bne .Lhandle_esl_ec_set
> + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
> + bne power_enter_stop_esl
> PPC_STOP
> li r3,0 /* Since we didn't lose state, return 0 */
>
> @@ -354,8 +348,13 @@ power_enter_stop:
> */
> li r12, 0
> b pnv_wakeup_noloss
> +#endif
>
> -.Lhandle_esl_ec_set:
> +/*
> + * r3 - PSSCR value corresponding to the requested stop state.
> + */
> +power_enter_stop_esl:
> + clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
> /*
> * POWER9 DD2 can incorrectly set PMAO when waking up after a
> * state-loss idle. Saving and restoring MMCR0 over idle is a
> @@ -428,9 +427,23 @@ 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)
> +
> + /*
> + * Check if we are executing the lite variant with ESL=EC=0
> + * This case resumes execution after the stop instruction without
> + * losing any state, so nothing has to be saved. The following
> + * instructions up to the blr must be skipped if we want to
> + * use power_enter_stop_kvm_rm.
> + */
> + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
I realize you're just moving existing code, but I think this would be
clearer (to me, anyway) as
andis. r4, r3, (PSSCR_EC | PSSCR_ESL)@h
Apart from that very minor nit,
Reviewed-by: Paul Mackerras <paulus@ozlabs.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
@ 2017-08-29 0:20 ` Paul Mackerras
0 siblings, 0 replies; 38+ messages in thread
From: Paul Mackerras @ 2017-08-29 0:20 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
On Fri, Aug 25, 2017 at 02:30:36PM +1000, Nicholas Piggin wrote:
> 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 "lite" sleep states out to power9_idle_stop.
>
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 32d65ee323a0..fa56120bd0bc 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -315,9 +315,6 @@ enter_winkle:
>
> ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
>
> -/*
> - * r3 - PSSCR value corresponding to the requested stop state.
> - */
> #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> power_enter_stop_kvm_rm:
> /*
> @@ -330,14 +327,11 @@ power_enter_stop_kvm_rm:
> li r4,KVM_HWTHREAD_IN_IDLE
> /* DO THIS IN REAL MODE! See comment above. */
> stb r4,HSTATE_HWTHREAD_STATE(r13)
> -#endif
> -power_enter_stop:
> /*
> * Check if we are executing the lite variant with ESLì=0
> */
> - andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
> - clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
> - bne .Lhandle_esl_ec_set
> + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
> + bne power_enter_stop_esl
> PPC_STOP
> li r3,0 /* Since we didn't lose state, return 0 */
>
> @@ -354,8 +348,13 @@ power_enter_stop:
> */
> li r12, 0
> b pnv_wakeup_noloss
> +#endif
>
> -.Lhandle_esl_ec_set:
> +/*
> + * r3 - PSSCR value corresponding to the requested stop state.
> + */
> +power_enter_stop_esl:
> + clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
> /*
> * POWER9 DD2 can incorrectly set PMAO when waking up after a
> * state-loss idle. Saving and restoring MMCR0 over idle is a
> @@ -428,9 +427,23 @@ 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)
> +
> + /*
> + * Check if we are executing the lite variant with ESLì=0
> + * This case resumes execution after the stop instruction without
> + * losing any state, so nothing has to be saved. The following
> + * instructions up to the blr must be skipped if we want to
> + * use power_enter_stop_kvm_rm.
> + */
> + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
I realize you're just moving existing code, but I think this would be
clearer (to me, anyway) as
andis. r4, r3, (PSSCR_EC | PSSCR_ESL)@h
Apart from that very minor nit,
Reviewed-by: Paul Mackerras <paulus@ozlabs.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
2017-08-29 0:20 ` Paul Mackerras
@ 2017-08-29 1:39 ` Nicholas Piggin
-1 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-29 1:39 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, Gautham R. Shenoy
On Tue, 29 Aug 2017 10:20:48 +1000
Paul Mackerras <paulus@ozlabs.org> wrote:
> On Fri, Aug 25, 2017 at 02:30:36PM +1000, Nicholas Piggin wrote:
> > 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 "lite" sleep states out to power9_idle_stop.
> >
> > Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
> > 1 file changed, 24 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> > index 32d65ee323a0..fa56120bd0bc 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S
> > @@ -315,9 +315,6 @@ enter_winkle:
> >
> > ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
> >
> > -/*
> > - * r3 - PSSCR value corresponding to the requested stop state.
> > - */
> > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> > power_enter_stop_kvm_rm:
> > /*
> > @@ -330,14 +327,11 @@ power_enter_stop_kvm_rm:
> > li r4,KVM_HWTHREAD_IN_IDLE
> > /* DO THIS IN REAL MODE! See comment above. */
> > stb r4,HSTATE_HWTHREAD_STATE(r13)
> > -#endif
> > -power_enter_stop:
> > /*
> > * Check if we are executing the lite variant with ESL=EC=0
> > */
> > - andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
> > - clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
> > - bne .Lhandle_esl_ec_set
> > + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
> > + bne power_enter_stop_esl
> > PPC_STOP
> > li r3,0 /* Since we didn't lose state, return 0 */
> >
> > @@ -354,8 +348,13 @@ power_enter_stop:
> > */
> > li r12, 0
> > b pnv_wakeup_noloss
> > +#endif
> >
> > -.Lhandle_esl_ec_set:
> > +/*
> > + * r3 - PSSCR value corresponding to the requested stop state.
> > + */
> > +power_enter_stop_esl:
> > + clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
> > /*
> > * POWER9 DD2 can incorrectly set PMAO when waking up after a
> > * state-loss idle. Saving and restoring MMCR0 over idle is a
> > @@ -428,9 +427,23 @@ 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)
> > +
> > + /*
> > + * Check if we are executing the lite variant with ESL=EC=0
> > + * This case resumes execution after the stop instruction without
> > + * losing any state, so nothing has to be saved. The following
> > + * instructions up to the blr must be skipped if we want to
> > + * use power_enter_stop_kvm_rm.
> > + */
> > + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
>
> I realize you're just moving existing code, but I think this would be
> clearer (to me, anyway) as
>
> andis. r4, r3, (PSSCR_EC | PSSCR_ESL)@h
Agreed, it's not a very helpful extra indirection. Perhaps we could
do a cleanup patch to fix that (and your other points like the 206/207
confusion).
Thanks for the reviews.
> Apart from that very minor nit,
>
> Reviewed-by: Paul Mackerras <paulus@ozlabs.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
@ 2017-08-29 1:39 ` Nicholas Piggin
0 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-29 1:39 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, Gautham R. Shenoy
On Tue, 29 Aug 2017 10:20:48 +1000
Paul Mackerras <paulus@ozlabs.org> wrote:
> On Fri, Aug 25, 2017 at 02:30:36PM +1000, Nicholas Piggin wrote:
> > 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 "lite" sleep states out to power9_idle_stop.
> >
> > Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
> > 1 file changed, 24 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> > index 32d65ee323a0..fa56120bd0bc 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S
> > @@ -315,9 +315,6 @@ enter_winkle:
> >
> > ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
> >
> > -/*
> > - * r3 - PSSCR value corresponding to the requested stop state.
> > - */
> > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> > power_enter_stop_kvm_rm:
> > /*
> > @@ -330,14 +327,11 @@ power_enter_stop_kvm_rm:
> > li r4,KVM_HWTHREAD_IN_IDLE
> > /* DO THIS IN REAL MODE! See comment above. */
> > stb r4,HSTATE_HWTHREAD_STATE(r13)
> > -#endif
> > -power_enter_stop:
> > /*
> > * Check if we are executing the lite variant with ESLì=0
> > */
> > - andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
> > - clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
> > - bne .Lhandle_esl_ec_set
> > + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
> > + bne power_enter_stop_esl
> > PPC_STOP
> > li r3,0 /* Since we didn't lose state, return 0 */
> >
> > @@ -354,8 +348,13 @@ power_enter_stop:
> > */
> > li r12, 0
> > b pnv_wakeup_noloss
> > +#endif
> >
> > -.Lhandle_esl_ec_set:
> > +/*
> > + * r3 - PSSCR value corresponding to the requested stop state.
> > + */
> > +power_enter_stop_esl:
> > + clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
> > /*
> > * POWER9 DD2 can incorrectly set PMAO when waking up after a
> > * state-loss idle. Saving and restoring MMCR0 over idle is a
> > @@ -428,9 +427,23 @@ 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)
> > +
> > + /*
> > + * Check if we are executing the lite variant with ESLì=0
> > + * This case resumes execution after the stop instruction without
> > + * losing any state, so nothing has to be saved. The following
> > + * instructions up to the blr must be skipped if we want to
> > + * use power_enter_stop_kvm_rm.
> > + */
> > + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
>
> I realize you're just moving existing code, but I think this would be
> clearer (to me, anyway) as
>
> andis. r4, r3, (PSSCR_EC | PSSCR_ESL)@h
Agreed, it's not a very helpful extra indirection. Perhaps we could
do a cleanup patch to fix that (and your other points like the 206/207
confusion).
Thanks for the reviews.
> Apart from that very minor nit,
>
> Reviewed-by: Paul Mackerras <paulus@ozlabs.org>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
2017-08-25 4:30 ` Nicholas Piggin
@ 2017-08-30 11:25 ` Michael Ellerman
-1 siblings, 0 replies; 38+ messages in thread
From: Michael Ellerman @ 2017-08-30 11:25 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: kvm-ppc, 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 "lite" sleep states out to power9_idle_stop.
>
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
This is blowing up for me on mambo:
usbcore: registered new interface driver usb-storage
Disabling lock debugging due to kernel taint
Severe Machine check interrupt [Not recovered]
NIP [c0000000002a0c04]: kmem_cache_free+0x64/0x2c0
Initiator: CPU
Error type: Real address [Load/Store (foreign)]
opal: Hardware platform error: Unrecoverable Machine Check exception
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G M 4.13.0-rc2-gcc-6.3.1-00257-g26268bb39bff #543
task: c0000000016b1200 task.stack: c00000000175c000
NIP: c0000000002a0c04 LR: c0000000001128ec CTR: c000000000112800
REGS: c00000003fff7d80 TRAP: 0200 Tainted: G M (4.13.0-rc2-gcc-6.3.1-00257-g26268bb39bff)
MSR: 9000000000209003 <SF,HV,EE,ME,RI,LE> CR: 28002828 XER: 20000000
CFAR: c0000000001128e8 DAR: c00a0000003c3ce0 DSISR: 00000008 SOFTE: 1
GPR00: c0000000001128ec c00000000175f7b0 c000000001760500 c0000000f001a000
GPR04: c0000000f0f37f00 0000000000000003 00000000000220c3 0000000000000000
GPR08: 0000000000000000 00000000003c3cc0 c0000000017e7758 0000000000000000
GPR12: c000000000112800 c00000000fff0000 c000000000112800 c0000000f0f37f90
GPR16: c00000000175c000 c00000000175c000 0000000000000001 c0000000016d7900
GPR20: c00000000179ba98 7fffffffffffffff c00000000175c000 0000000000000000
GPR24: c00000000017a8c0 000000000000000a c0000000016d8a00 c00000000175f8d0
GPR28: c0000000001128ec c0000000f0f37f00 c00a0000003c3cc0 c0000000f001a000
NIP [c0000000002a0c04] kmem_cache_free+0x64/0x2c0
LR [c0000000001128ec] put_cred_rcu+0xec/0x140
Call Trace:
[c00000000175f7b0] [c00000000175f800] init_thread_union+0x3800/0x4000 (unreliable)
[c00000000175f840] [c0000000001128ec] put_cred_rcu+0xec/0x140
[c00000000175f8b0] [c00000000017a908] rcu_process_callbacks+0x438/0x6a0
[c00000000175f980] [c0000000000e5e28] __do_softirq+0x198/0x310
[c00000000175fa70] [c0000000000e6248] irq_exit+0xf8/0x140
[c00000000175fa90] [c000000000023710] timer_interrupt+0xa0/0xe0
[c00000000175fac0] [c000000000008fcc] decrementer_common+0x11c/0x120
--- interrupt: 901 at replay_interrupt_return+0x0/0x4
LR = arch_local_irq_restore.part.1+0x84/0xb0
[c00000000175fdb0] [c00000000175c000] init_thread_union+0x0/0x4000 (unreliable)
[c00000000175fdd0] [c00000000001cde0] arch_cpu_idle+0xe0/0x140
[c00000000175fe00] [c0000000007f4f44] default_idle_call+0x44/0x84
[c00000000175fe20] [c000000000142e54] do_idle+0x254/0x320
[c00000000175fe90] [c000000000143280] cpu_startup_entry+0x30/0x40
[c00000000175fec0] [c00000000000d2d8] rest_init+0x2f8/0x320
[c00000000175ff00] [c000000001000d74] start_kernel+0x510/0x52c
[c00000000175ff90] [c00000000000ab70] start_here_common+0x1c/0x4ac
Instruction dump:
60000000 e9230008 71280100 40820170 2fbf0000 419e013c 3d420008 394a7258
7bbe8502 7bc93664 ebca0000 7fde4a14 <e93e0020> 712a0001 40820234 e93f0008
[ 1.217346467,0] OPAL: Reboot requested due to Platform error.
[ 1.217351582,3] OPAL: failed to log an error
[ 1.217358188,5] OPAL: Reboot request...
I'll just drop it for now.
cheers
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
@ 2017-08-30 11:25 ` Michael Ellerman
0 siblings, 0 replies; 38+ messages in thread
From: Michael Ellerman @ 2017-08-30 11:25 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: kvm-ppc, 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 "lite" sleep states out to power9_idle_stop.
>
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
This is blowing up for me on mambo:
usbcore: registered new interface driver usb-storage
Disabling lock debugging due to kernel taint
Severe Machine check interrupt [Not recovered]
NIP [c0000000002a0c04]: kmem_cache_free+0x64/0x2c0
Initiator: CPU
Error type: Real address [Load/Store (foreign)]
opal: Hardware platform error: Unrecoverable Machine Check exception
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G M 4.13.0-rc2-gcc-6.3.1-00257-g26268bb39bff #543
task: c0000000016b1200 task.stack: c00000000175c000
NIP: c0000000002a0c04 LR: c0000000001128ec CTR: c000000000112800
REGS: c00000003fff7d80 TRAP: 0200 Tainted: G M (4.13.0-rc2-gcc-6.3.1-00257-g26268bb39bff)
MSR: 9000000000209003 <SF,HV,EE,ME,RI,LE> CR: 28002828 XER: 20000000
CFAR: c0000000001128e8 DAR: c00a0000003c3ce0 DSISR: 00000008 SOFTE: 1
GPR00: c0000000001128ec c00000000175f7b0 c000000001760500 c0000000f001a000
GPR04: c0000000f0f37f00 0000000000000003 00000000000220c3 0000000000000000
GPR08: 0000000000000000 00000000003c3cc0 c0000000017e7758 0000000000000000
GPR12: c000000000112800 c00000000fff0000 c000000000112800 c0000000f0f37f90
GPR16: c00000000175c000 c00000000175c000 0000000000000001 c0000000016d7900
GPR20: c00000000179ba98 7fffffffffffffff c00000000175c000 0000000000000000
GPR24: c00000000017a8c0 000000000000000a c0000000016d8a00 c00000000175f8d0
GPR28: c0000000001128ec c0000000f0f37f00 c00a0000003c3cc0 c0000000f001a000
NIP [c0000000002a0c04] kmem_cache_free+0x64/0x2c0
LR [c0000000001128ec] put_cred_rcu+0xec/0x140
Call Trace:
[c00000000175f7b0] [c00000000175f800] init_thread_union+0x3800/0x4000 (unreliable)
[c00000000175f840] [c0000000001128ec] put_cred_rcu+0xec/0x140
[c00000000175f8b0] [c00000000017a908] rcu_process_callbacks+0x438/0x6a0
[c00000000175f980] [c0000000000e5e28] __do_softirq+0x198/0x310
[c00000000175fa70] [c0000000000e6248] irq_exit+0xf8/0x140
[c00000000175fa90] [c000000000023710] timer_interrupt+0xa0/0xe0
[c00000000175fac0] [c000000000008fcc] decrementer_common+0x11c/0x120
--- interrupt: 901 at replay_interrupt_return+0x0/0x4
LR = arch_local_irq_restore.part.1+0x84/0xb0
[c00000000175fdb0] [c00000000175c000] init_thread_union+0x0/0x4000 (unreliable)
[c00000000175fdd0] [c00000000001cde0] arch_cpu_idle+0xe0/0x140
[c00000000175fe00] [c0000000007f4f44] default_idle_call+0x44/0x84
[c00000000175fe20] [c000000000142e54] do_idle+0x254/0x320
[c00000000175fe90] [c000000000143280] cpu_startup_entry+0x30/0x40
[c00000000175fec0] [c00000000000d2d8] rest_init+0x2f8/0x320
[c00000000175ff00] [c000000001000d74] start_kernel+0x510/0x52c
[c00000000175ff90] [c00000000000ab70] start_here_common+0x1c/0x4ac
Instruction dump:
60000000 e9230008 71280100 40820170 2fbf0000 419e013c 3d420008 394a7258
7bbe8502 7bc93664 ebca0000 7fde4a14 <e93e0020> 712a0001 40820234 e93f0008
[ 1.217346467,0] OPAL: Reboot requested due to Platform error.
[ 1.217351582,3] OPAL: failed to log an error
[ 1.217358188,5] OPAL: Reboot request...
I'll just drop it for now.
cheers
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
2017-08-30 11:25 ` Michael Ellerman
@ 2017-08-30 12:10 ` Nicholas Piggin
-1 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-30 12:10 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, kvm-ppc
On Wed, 30 Aug 2017 21:25:59 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:
> 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 "lite" sleep states out to power9_idle_stop.
> >
> > Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
> > 1 file changed, 24 insertions(+), 11 deletions(-)
>
> This is blowing up for me on mambo:
Oh this is a known bug in mambo that does not match the hardware.
You need >= Mambo.7.8.21, or this firmware patch to work around
the issue for old mambos.
mambo.git 58d3162f4d6204fc077ff4a6ba47e4d1e19d5120
https://lists.ozlabs.org/pipermail/skiboot/2017-August/008768.html
Thanks,
Nick
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
@ 2017-08-30 12:10 ` Nicholas Piggin
0 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-08-30 12:10 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, kvm-ppc
On Wed, 30 Aug 2017 21:25:59 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:
> 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 "lite" sleep states out to power9_idle_stop.
> >
> > Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
> > 1 file changed, 24 insertions(+), 11 deletions(-)
>
> This is blowing up for me on mambo:
Oh this is a known bug in mambo that does not match the hardware.
You need >= Mambo.7.8.21, or this firmware patch to work around
the issue for old mambos.
mambo.git 58d3162f4d6204fc077ff4a6ba47e4d1e19d5120
https://lists.ozlabs.org/pipermail/skiboot/2017-August/008768.html
Thanks,
Nick
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
2017-08-30 12:10 ` Nicholas Piggin
@ 2017-09-01 9:39 ` Michael Ellerman
-1 siblings, 0 replies; 38+ messages in thread
From: Michael Ellerman @ 2017-09-01 9:39 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
Nicholas Piggin <npiggin@gmail.com> writes:
> On Wed, 30 Aug 2017 21:25:59 +1000
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>
>> 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 "lite" sleep states out to power9_idle_stop.
>> >
>> > Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
>> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> > ---
>> > arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
>> > 1 file changed, 24 insertions(+), 11 deletions(-)
>>
>> This is blowing up for me on mambo:
>
> Oh this is a known bug in mambo that does not match the hardware.
>
> You need >= Mambo.7.8.21, or this firmware patch to work around
> the issue for old mambos.
As discussed elsewhere this still breaks on new mambo with more than one
CPU. So I've dropped it for now.
cheers
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
@ 2017-09-01 9:39 ` Michael Ellerman
0 siblings, 0 replies; 38+ messages in thread
From: Michael Ellerman @ 2017-09-01 9:39 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
Nicholas Piggin <npiggin@gmail.com> writes:
> On Wed, 30 Aug 2017 21:25:59 +1000
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>
>> 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 "lite" sleep states out to power9_idle_stop.
>> >
>> > Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
>> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> > ---
>> > arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
>> > 1 file changed, 24 insertions(+), 11 deletions(-)
>>
>> This is blowing up for me on mambo:
>
> Oh this is a known bug in mambo that does not match the hardware.
>
> You need >= Mambo.7.8.21, or this firmware patch to work around
> the issue for old mambos.
As discussed elsewhere this still breaks on new mambo with more than one
CPU. So I've dropped it for now.
cheers
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
2017-09-01 9:39 ` Michael Ellerman
@ 2017-09-20 13:56 ` Nicholas Piggin
-1 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-09-20 13:56 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, kvm-ppc
On Fri, 01 Sep 2017 19:39:41 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > On Wed, 30 Aug 2017 21:25:59 +1000
> > Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> >> 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 "lite" sleep states out to power9_idle_stop.
> >> >
> >> > Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> >> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >> > ---
> >> > arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
> >> > 1 file changed, 24 insertions(+), 11 deletions(-)
> >>
> >> This is blowing up for me on mambo:
> >
> > Oh this is a known bug in mambo that does not match the hardware.
> >
> > You need >= Mambo.7.8.21, or this firmware patch to work around
> > the issue for old mambos.
>
> As discussed elsewhere this still breaks on new mambo with more than one
> CPU. So I've dropped it for now.
This now seems to be properly fixed in mambo with commit 11783550ee11.
Skiboot also has a patch merged which disables the problematic state on
mambo so older versions won't crash.
d2a24406a49 ("idle: disable stop*_lite POWER9 idle states for Mambo platform")
So this could be re-applied now.
Thanks,
Nick
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v3 4/4] powerpc/64s: idle ESL=0 stop can avoid MSR and save/restore overhead
@ 2017-09-20 13:56 ` Nicholas Piggin
0 siblings, 0 replies; 38+ messages in thread
From: Nicholas Piggin @ 2017-09-20 13:56 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, kvm-ppc
On Fri, 01 Sep 2017 19:39:41 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > On Wed, 30 Aug 2017 21:25:59 +1000
> > Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> >> 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 "lite" sleep states out to power9_idle_stop.
> >> >
> >> > Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> >> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >> > ---
> >> > arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++-----------
> >> > 1 file changed, 24 insertions(+), 11 deletions(-)
> >>
> >> This is blowing up for me on mambo:
> >
> > Oh this is a known bug in mambo that does not match the hardware.
> >
> > You need >= Mambo.7.8.21, or this firmware patch to work around
> > the issue for old mambos.
>
> As discussed elsewhere this still breaks on new mambo with more than one
> CPU. So I've dropped it for now.
This now seems to be properly fixed in mambo with commit 11783550ee11.
Skiboot also has a patch merged which disables the problematic state on
mambo so older versions won't crash.
d2a24406a49 ("idle: disable stop*_lite POWER9 idle states for Mambo platform")
So this could be re-applied now.
Thanks,
Nick
^ permalink raw reply [flat|nested] 38+ messages in thread