On Tue, 22 Sep 2026 21:42:27 +0200,
Amit Machhiwal <[email protected]> wrote:
> 
> On 2026/09/03 01:50 PM, Kirill A. Korinsky wrote:
> > OpenBSD writes zero to an edge triggered decrementer, then enters idle
> > expecting its interrupt. QEMU defers the already due edge to a timer at
> > the current virtual time, leaving the guest stalled.
> > 
> > Signed-off-by: Kirill A. Korinsky <[email protected]>
> > ---
> >  hw/ppc/ppc.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> > index b123b4cc1c..d99e47d532 100644
> > --- a/hw/ppc/ppc.c
> > +++ b/hw/ppc/ppc.c
> > @@ -885,6 +885,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, 
> > int64_t now, uint64_t *nextp,
> >       * an edge interrupt, so raise it here too.
> >       */
> >      if (((flags & PPC_DECR_UNDERFLOW_LEVEL) && signed_value < 0) ||
> > +        ((flags & PPC_DECR_UNDERFLOW_TRIGGERED) && value == 0) ||
> 
> The fix is correct for mac99/G3/G4, but the guard flag is too broad and
> introduces a regression on PPE42.
> 
> Commit 17dd1354c1d1 ("target/ppc: Decrementer fix BookE semantics") explicitly
> removed the immediate raise on value==0 with the rationale: "BookE says a
> decrementer interrupt should not be raised on a store of 0, only of a 
> decrement
> from 1."
> 
> The new clause:
> 
>     ((flags & PPC_DECR_UNDERFLOW_TRIGGERED) && value == 0)
> 
> also fires for PPE42, which goes through ppc_booke_timers_init() with
> PPC_TIMER_PPE and gets PPC_DECR_UNDERFLOW_TRIGGERED set - introduced by commit
> 7197f6f7baf2 ("hw/ppc: Support for an IBM PPE42 CPU decrementer").  That 
> commit
> explicitly documents that PPE42 raises an interrupt when DEC[0] transitions 
> from
> 0 to -1 (non-negative to negative) — not on a store of zero.  The new clause
> contradicts this.
> 
> Note that __cpu_ppc_store_decr() is reached by BookE machines too — both on
> direct guest writes to SPR_DECR and via the auto-reload path in 
> booke_decr_cb()
> (which calls cpu_ppc_store_decr(env, DECAR) when TCR_ARE is set; the existing
> DECAR != 0 guard there already prevents a zero value from arriving via that
> route).  So the safety of normal BookE targets (e500, sam460ex, bamboo, 
> virtex)
> is flag-based: they have PPC_DECR_ZERO_TRIGGERED set but not
> PPC_DECR_UNDERFLOW_TRIGGERED, so neither the old clause nor the new one fires
> for a guest write of zero.  That is the correct BookE behaviour per the
> architecture.
> 
> PPC_DECR_ZERO_TRIGGERED already exists in ppc.h (defined as "Decr interrupt
> triggered when the decrementer reaches zero") but is never checked in
> __cpu_ppc_store_decr().  I think the minimal fix is to set it in
> cpu_ppc_tb_init() for the non-64-bit path and guard the new clause on it:
> 
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index b123b4cc1cce..6f99b61a2876 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -885,6 +885,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, int64_t 
> now, uint64_t *nextp,
>       * an edge interrupt, so raise it here too.
>       */
>      if (((flags & PPC_DECR_UNDERFLOW_LEVEL) && signed_value < 0) ||
> +        ((flags & PPC_DECR_ZERO_TRIGGERED) && value == 0) ||
>          ((flags & PPC_DECR_UNDERFLOW_TRIGGERED) && signed_value < 0
>            && signed_decr >= 0)) {
>          (*raise_excp)(cpu);
> @@ -1104,6 +1105,12 @@ void cpu_ppc_tb_init(CPUPPCState *env, uint32_t freq)
>      if (is_book3s_arch2x(env)) {
>          /* All Book3S 64bit CPUs implement level based DEC logic */
>          tb_env->flags |= PPC_DECR_UNDERFLOW_LEVEL;
> +    } else {
> +        /*
> +         * Classic 32-bit edge-triggered decrementer: zero write fires
> +         * immediately
> +         */
> +        tb_env->flags |= PPC_DECR_ZERO_TRIGGERED;
>      }
>      /* Create new timer */
>      tb_env->decr_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> 
> 
> All callers of cpu_ppc_tb_init() that use a 32-bit classic CPU (mac99 with 
> 7400,
> mac_oldworld/750, prep/604, pegasos/7457, amigaone/7457) take the else branch
> and get PPC_DECR_ZERO_TRIGGERED.  64-bit Book3S machines (spapr, pnv, mac99 
> with
> 970fx) take the if branch and are unchanged.  PPE42 goes through
> ppc_booke_timers_init() and never gets PPC_DECR_ZERO_TRIGGERED, so the new
> clause does not fire for it.
> 
> This scopes the fix to the classic 32-bit edge-triggered path (mac99, pegasos,
> prep, mac_oldworld, amigaone) and leaves PPE42 behaviour unchanged.  What do 
> you
> think?
> 

It is sane and I had included your suggested way into v2 of the series.

Thanks!

-- 
wbr, Kirill

Reply via email to