The ppc (pnv and spapr) NMI injection code does not go through the
asynchronous interrupt path and set a bit in env->pending_interrupts
and raise an interrupt request that the cpu_exec() loop can see.
Instead it injects the exception directly into registers.

This can lead to cpu_exec() missing that the thread has work to do,
if a NMI is injected while it was idle.

Fix this by clearing halted when injecting the interrupt. Probably
NMI injection should be reworked to use the interrupt request interface,
but this seems to work as a minimal fix.

Fixes: 3431648272d3 ("spapr: Add support for new NMI interface")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/excp_helper.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 70daa5076a..9f811af0a4 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2495,10 +2495,16 @@ static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
     }
 }
 
+/*
+ * system reset is not delivered via normal irq method, so have to set
+ * halted = 0 to resume CPU running if it was halted. Possibly we should
+ * move it over to using PPC_INTERRUPT_RESET rather than async_run_on_cpu.
+ */
 void ppc_cpu_do_system_reset(CPUState *cs)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
 
+    cs->halted = 0;
     powerpc_excp(cpu, POWERPC_EXCP_RESET);
 }
 
@@ -2520,6 +2526,7 @@ void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector)
 
     /* Anything for nested required here? MSR[HV] bit? */
 
+    cs->halted = 0;
     powerpc_set_excp_state(cpu, vector, msr);
 }
 
-- 
2.45.2

Reviewed-by: Glenn Miles <milesg@linux.ibm.com>

On Mon, 2024-11-25 at 23:20 +1000, Nicholas Piggin wrote:
> The ppc (pnv and spapr) NMI injection code does not go through the
> asynchronous interrupt path and set a bit in env->pending_interrupts
> and raise an interrupt request that the cpu_exec() loop can see.
> Instead it injects the exception directly into registers.
> 
> This can lead to cpu_exec() missing that the thread has work to do,
> if a NMI is injected while it was idle.
> 
> Fix this by clearing halted when injecting the interrupt. Probably
> NMI injection should be reworked to use the interrupt request
> interface,
> but this seems to work as a minimal fix.
> 
> Fixes: 3431648272d3 ("spapr: Add support for new NMI interface")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  target/ppc/excp_helper.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 70daa5076a..9f811af0a4 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -2495,10 +2495,16 @@ static void ppc_deliver_interrupt(CPUPPCState
> *env, int interrupt)
>      }
>  }
>  
> +/*
> + * system reset is not delivered via normal irq method, so have to
> set
> + * halted = 0 to resume CPU running if it was halted. Possibly we
> should
> + * move it over to using PPC_INTERRUPT_RESET rather than
> async_run_on_cpu.
> + */
>  void ppc_cpu_do_system_reset(CPUState *cs)
>  {
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
>  
> +    cs->halted = 0;
>      powerpc_excp(cpu, POWERPC_EXCP_RESET);
>  }
>  
> @@ -2520,6 +2526,7 @@ void ppc_cpu_do_fwnmi_machine_check(CPUState
> *cs, target_ulong vector)
>  
>      /* Anything for nested required here? MSR[HV] bit? */
>  
> +    cs->halted = 0;
>      powerpc_set_excp_state(cpu, vector, msr);
>  }
>