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

2019-05-15 Thread David Gibson
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 3dbe0308 CTR 6d61676963313233 XER 
>  CPU#1
> MSR  HID0   HF  iidx 3 didx 3
> TB 0026 115746031956 DECR 18446744073326238463
> GPR00 3dbe0308 3e669fe0 3dc10700 0003
> GPR04 3dc62198 3dc62178 3dc0ea48 0030
> GPR08 3dc621a8 0018 3e466008 3dc50700
> GPR12 c093a4e0 c0003300 c0003e533f90 
> GPR16   3e466010 3dc0b040
> GPR20 8000 f003 0006 3e66a050
> GPR24 3dc06400 3dc0ae70 0003 f001
> GPR28 3e66a060  6d61676963313233 0028
> CR 28000222  [ E  L  -  -  -  E  E  E  ] RES 
> FPR00    
> FPR04    
> FPR08    311825e0
> FPR12 311825e0   
> FPR16    
> FPR20    
> FPR24    
> FPR28    
> FPSCR 
>  SRR0 3dbe06b0  SRR1 0008PVR 004e1200 VRSAVE 
> 
> SPRG0 3dbe0308 SPRG1 3e669fe0  SPRG2 00d8  SPRG3 
> 3dbe0308
> SPRG4  SPRG5   SPRG6   SPRG7 
> 
> HSRR0 6d61676963313230 HSRR1 
>  CFAR 3dbe3e64
>  LPCR 04020008
>  PTCR    DAR   DSISR 
> 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 

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

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

2019-05-15 Thread Suraj Jitindar Singh
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 3dbe0308 CTR 6d61676963313233 XER 
 CPU#1
MSR  HID0   HF  iidx 3 didx 3
TB 0026 115746031956 DECR 18446744073326238463
GPR00 3dbe0308 3e669fe0 3dc10700 0003
GPR04 3dc62198 3dc62178 3dc0ea48 0030
GPR08 3dc621a8 0018 3e466008 3dc50700
GPR12 c093a4e0 c0003300 c0003e533f90 
GPR16   3e466010 3dc0b040
GPR20 8000 f003 0006 3e66a050
GPR24 3dc06400 3dc0ae70 0003 f001
GPR28 3e66a060  6d61676963313233 0028
CR 28000222  [ E  L  -  -  -  E  E  E  ] RES 
FPR00    
FPR04    
FPR08    311825e0
FPR12 311825e0   
FPR16    
FPR20    
FPR24    
FPR28    
FPSCR 
 SRR0 3dbe06b0  SRR1 0008PVR 004e1200 VRSAVE 

SPRG0 3dbe0308 SPRG1 3e669fe0  SPRG2 00d8  SPRG3 
3dbe0308
SPRG4  SPRG5   SPRG6   SPRG7 

HSRR0 6d61676963313230 HSRR1 
 CFAR 3dbe3e64
 LPCR 04020008
 PTCR    DAR   DSISR 
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 
---
 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