[Qemu-devel] [PATCH] target/ppc: Set PSSCR_EC on cpu halt to prevent spurious wakeup

Suraj Jitindar Singh posted 1 patch 5 days ago
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190516005744.24366-1-sjitindarsingh@gmail.com
Maintainers: David Gibson <david@gibson.dropbear.id.au>
hw/ppc/spapr_cpu_core.c | 2 ++
hw/ppc/spapr_rtas.c     | 6 +++++-
2 files changed, 7 insertions(+), 1 deletion(-)

[Qemu-devel] [PATCH] target/ppc: Set PSSCR_EC on cpu halt to prevent spurious wakeup

Posted by Suraj Jitindar Singh 5 days ago
The processor stop status and control register (PSSCR) is used to
control the power saving facilities of the thread. The exit criterion
bit (EC) is used to specify whether the thread should be woken by any
interrupt (EC == 0) or only an interrupt enabled in the LPCR to wake the
thread (EC == 1).

The rtas facilities start-cpu and self-stop are used to transition a
vcpu between the stopped and running states. When a vcpu is stopped it
may only be started again by the start-cpu rtas call.

Currently a vcpu in the stopped state will start again whenever an
interrupt comes along due to PSSCR_EC being cleared, and while this is
architecturally correct for a hardware thread, a vcpu is expected to
only be woken by calling start-cpu. This means when performing a reboot
on a tcg machine that the secondary threads will restart while the
primary is still in slof, this is unsupported and causes call traces
like:

SLOF **********************************************************************
QEMU Starting
 Build Date = Jan 14 2019 18:00:39
 FW Version = git-a5b428e1c1eae703
 Press "s" to enter Open Firmware.

qemu: fatal: Trying to deliver HV exception (MSR) 70 with no HV support

NIP 6d61676963313230   LR 000000003dbe0308 CTR 6d61676963313233 XER 0000000000000000 CPU#1
MSR 0000000000000000 HID0 0000000000000000  HF 0000000000000000 iidx 3 didx 3
TB 00000026 115746031956 DECR 18446744073326238463
GPR00 000000003dbe0308 000000003e669fe0 000000003dc10700 0000000000000003
GPR04 000000003dc62198 000000003dc62178 000000003dc0ea48 0000000000000030
GPR08 000000003dc621a8 0000000000000018 000000003e466008 000000003dc50700
GPR12 c00000000093a4e0 c00000003ffff300 c00000003e533f90 0000000000000000
GPR16 0000000000000000 0000000000000000 000000003e466010 000000003dc0b040
GPR20 0000000000008000 000000000000f003 0000000000000006 000000003e66a050
GPR24 000000003dc06400 000000003dc0ae70 0000000000000003 000000000000f001
GPR28 000000003e66a060 ffffffffffffffff 6d61676963313233 0000000000000028
CR 28000222  [ E  L  -  -  -  E  E  E  ]             RES ffffffffffffffff
FPR00 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR08 0000000000000000 0000000000000000 0000000000000000 00000000311825e0
FPR12 00000000311825e0 0000000000000000 0000000000000000 0000000000000000
FPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000
FPSCR 0000000000000000
 SRR0 000000003dbe06b0  SRR1 0000000000080000    PVR 00000000004e1200 VRSAVE 0000000000000000
SPRG0 000000003dbe0308 SPRG1 000000003e669fe0  SPRG2 00000000000000d8  SPRG3 000000003dbe0308
SPRG4 0000000000000000 SPRG5 0000000000000000  SPRG6 0000000000000000  SPRG7 0000000000000000
HSRR0 6d61676963313230 HSRR1 0000000000000000
 CFAR 000000003dbe3e64
 LPCR 0000000004020008
 PTCR 0000000000000000   DAR 0000000000000000  DSISR 0000000000000000
Aborted (core dumped)

To fix this, set the PSSCR_EC bit when a vcpu is stopped to disable it
from coming back online until the start-cpu rtas call is made.

Fixes: 21c0d66a9c99 ("target/ppc: Fix support for "STOP light" states on POWER9")

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 hw/ppc/spapr_cpu_core.c | 2 ++
 hw/ppc/spapr_rtas.c     | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index f04e06cdf6..5621fb9a3d 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -58,9 +58,11 @@ static void spapr_cpu_reset(void *opaque)
      *
      * Disable Power-saving mode Exit Cause exceptions for the CPU, so
      * we don't get spurious wakups before an RTAS start-cpu call.
+     * For the same reason, set PSSCR_EC.
      */
     lpcr &= ~(LPCR_VPM0 | LPCR_VPM1 | LPCR_ISL | LPCR_KBV | pcc->lpcr_pm);
     lpcr |= LPCR_LPES0 | LPCR_LPES1;
+    env->spr[SPR_PSSCR] |= PSSCR_EC;
 
     /* Set RMLS to the max (ie, 16G) */
     lpcr &= ~LPCR_RMLS;
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index ee24212765..5bc1a93271 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -177,6 +177,7 @@ static void rtas_start_cpu(PowerPCCPU *callcpu, SpaprMachineState *spapr,
         } else {
             lpcr &= ~(LPCR_UPRT | LPCR_GTSE | LPCR_HR);
         }
+        env->spr[SPR_PSSCR] &= ~PSSCR_EC;
     }
     ppc_store_lpcr(newcpu, lpcr);
 
@@ -205,8 +206,11 @@ static void rtas_stop_self(PowerPCCPU *cpu, SpaprMachineState *spapr,
 
     /* Disable Power-saving mode Exit Cause exceptions for the CPU.
      * This could deliver an interrupt on a dying CPU and crash the
-     * guest */
+     * guest.
+     * For the same reason, set PSSCR_EC.
+     */
     ppc_store_lpcr(cpu, env->spr[SPR_LPCR] & ~pcc->lpcr_pm);
+    env->spr[SPR_PSSCR] |= PSSCR_EC;
     cs->halted = 1;
     kvmppc_set_reg_ppc_online(cpu, 0);
     qemu_cpu_kick(cs);
-- 
2.13.6


Re: [Qemu-devel] [PATCH] target/ppc: Set PSSCR_EC on cpu halt to prevent spurious wakeup

Posted by David Gibson 5 days ago
On Thu, May 16, 2019 at 10:57:44AM +1000, Suraj Jitindar Singh wrote:
> The processor stop status and control register (PSSCR) is used to
> control the power saving facilities of the thread. The exit criterion
> bit (EC) is used to specify whether the thread should be woken by any
> interrupt (EC == 0) or only an interrupt enabled in the LPCR to wake the
> thread (EC == 1).
> 
> The rtas facilities start-cpu and self-stop are used to transition a
> vcpu between the stopped and running states. When a vcpu is stopped it
> may only be started again by the start-cpu rtas call.
> 
> Currently a vcpu in the stopped state will start again whenever an
> interrupt comes along due to PSSCR_EC being cleared, and while this is
> architecturally correct for a hardware thread, a vcpu is expected to
> only be woken by calling start-cpu. This means when performing a reboot
> on a tcg machine that the secondary threads will restart while the
> primary is still in slof, this is unsupported and causes call traces
> like:
> 
> SLOF **********************************************************************
> QEMU Starting
>  Build Date = Jan 14 2019 18:00:39
>  FW Version = git-a5b428e1c1eae703
>  Press "s" to enter Open Firmware.
> 
> qemu: fatal: Trying to deliver HV exception (MSR) 70 with no HV support
> 
> NIP 6d61676963313230   LR 000000003dbe0308 CTR 6d61676963313233 XER 0000000000000000 CPU#1
> MSR 0000000000000000 HID0 0000000000000000  HF 0000000000000000 iidx 3 didx 3
> TB 00000026 115746031956 DECR 18446744073326238463
> GPR00 000000003dbe0308 000000003e669fe0 000000003dc10700 0000000000000003
> GPR04 000000003dc62198 000000003dc62178 000000003dc0ea48 0000000000000030
> GPR08 000000003dc621a8 0000000000000018 000000003e466008 000000003dc50700
> GPR12 c00000000093a4e0 c00000003ffff300 c00000003e533f90 0000000000000000
> GPR16 0000000000000000 0000000000000000 000000003e466010 000000003dc0b040
> GPR20 0000000000008000 000000000000f003 0000000000000006 000000003e66a050
> GPR24 000000003dc06400 000000003dc0ae70 0000000000000003 000000000000f001
> GPR28 000000003e66a060 ffffffffffffffff 6d61676963313233 0000000000000028
> CR 28000222  [ E  L  -  -  -  E  E  E  ]             RES ffffffffffffffff
> FPR00 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPR08 0000000000000000 0000000000000000 0000000000000000 00000000311825e0
> FPR12 00000000311825e0 0000000000000000 0000000000000000 0000000000000000
> FPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> FPSCR 0000000000000000
>  SRR0 000000003dbe06b0  SRR1 0000000000080000    PVR 00000000004e1200 VRSAVE 0000000000000000
> SPRG0 000000003dbe0308 SPRG1 000000003e669fe0  SPRG2 00000000000000d8  SPRG3 000000003dbe0308
> SPRG4 0000000000000000 SPRG5 0000000000000000  SPRG6 0000000000000000  SPRG7 0000000000000000
> HSRR0 6d61676963313230 HSRR1 0000000000000000
>  CFAR 000000003dbe3e64
>  LPCR 0000000004020008
>  PTCR 0000000000000000   DAR 0000000000000000  DSISR 0000000000000000
> Aborted (core dumped)
> 
> To fix this, set the PSSCR_EC bit when a vcpu is stopped to disable it
> from coming back online until the start-cpu rtas call is made.
> 
> Fixes: 21c0d66a9c99 ("target/ppc: Fix support for "STOP light" states on POWER9")
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

Applied, thanks.

> ---
>  hw/ppc/spapr_cpu_core.c | 2 ++
>  hw/ppc/spapr_rtas.c     | 6 +++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index f04e06cdf6..5621fb9a3d 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -58,9 +58,11 @@ static void spapr_cpu_reset(void *opaque)
>       *
>       * Disable Power-saving mode Exit Cause exceptions for the CPU, so
>       * we don't get spurious wakups before an RTAS start-cpu call.
> +     * For the same reason, set PSSCR_EC.
>       */
>      lpcr &= ~(LPCR_VPM0 | LPCR_VPM1 | LPCR_ISL | LPCR_KBV | pcc->lpcr_pm);
>      lpcr |= LPCR_LPES0 | LPCR_LPES1;
> +    env->spr[SPR_PSSCR] |= PSSCR_EC;
>  
>      /* Set RMLS to the max (ie, 16G) */
>      lpcr &= ~LPCR_RMLS;
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index ee24212765..5bc1a93271 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -177,6 +177,7 @@ static void rtas_start_cpu(PowerPCCPU *callcpu, SpaprMachineState *spapr,
>          } else {
>              lpcr &= ~(LPCR_UPRT | LPCR_GTSE | LPCR_HR);
>          }
> +        env->spr[SPR_PSSCR] &= ~PSSCR_EC;
>      }
>      ppc_store_lpcr(newcpu, lpcr);
>  
> @@ -205,8 +206,11 @@ static void rtas_stop_self(PowerPCCPU *cpu, SpaprMachineState *spapr,
>  
>      /* Disable Power-saving mode Exit Cause exceptions for the CPU.
>       * This could deliver an interrupt on a dying CPU and crash the
> -     * guest */
> +     * guest.
> +     * For the same reason, set PSSCR_EC.
> +     */
>      ppc_store_lpcr(cpu, env->spr[SPR_LPCR] & ~pcc->lpcr_pm);
> +    env->spr[SPR_PSSCR] |= PSSCR_EC;
>      cs->halted = 1;
>      kvmppc_set_reg_ppc_online(cpu, 0);
>      qemu_cpu_kick(cs);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson