Re: [PATCH 3/4] target/arm: simplify wfxt_timer_cb to only trigger cpu_interrupt

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 30 Oct 2026 at 14:10, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> HALT_WFE will always be cleared when leaving halt via
>> arm_cpu_leaving_halt so we don't need to worry about atomic games in
>> the asynchronous timer callback. Furthermore, a WFET timeout or
>> event-stream expiration is not an event that sets the event register;
>
> No, this is still not true. WFET timeouts don't set the
> event register, but event-stream expiration does.

Oops I missed that comment, how about something like this:

--8<---------------cut here---------------start------------->8---
modified   target/arm/cpu.h
@@ -984,8 +984,12 @@ struct ArchCPU {
      * pmu_op_finish() - it does not need other handling during migration
      */
     QEMUTimer *pmu_timer;
-    /* Timer used for WFxT timeouts OR event stream events */
+    /*
+     * Timer used for WFxT timeouts OR event stream events,
+     * wfxt_triggers_event set depending on what will come first.
+     */
     QEMUTimer *wfxt_timer;
+    bool wfxt_triggers_event;
 
     /* GPIO outputs for generic timer */
     qemu_irq gt_timer_outputs[NUM_GTIMERS];
modified   target/arm/cpu.c
@@ -885,26 +885,17 @@ static void arm_cpu_leaving_halt(CPUState *cs)
 }
 #endif
 
-/*
- * Unlike almost everything else that messes with the halt_reason and
- * event_register details the timer callbacks are not in the vCPU
- * context.
- *
- * To prevent races we atomically consume a HALT_WFE and set the event
- * register. Either way we trigger the an exit event.
- */
 static void arm_wfxt_timer_cb(void *opaque)
 {
     ARMCPU *cpu = opaque;
     CPUState *cs = CPU(cpu);
     CPUARMState *env = &cpu->env;
 
-    if (qatomic_cmpxchg(&env->halt_reason, HALT_WFE, NOT_HALTED)) {
+    if (cpu->wfxt_triggers_event) {
         qatomic_set(&env->event_register, true);
     }
-
     /*
-     * We expect the CPU to be halted; this will cause arm_cpu_is_work()
+     * We expect the CPU to be halted; this will cause arm_cpu_has_work()
      * to return true (so we will come out of halt even with no other
      * pending interrupt), and the TCG accelerator's cpu_exec_interrupt()
      * function auto-clears the CPU_INTERRUPT_EXITTB flag for us.
modified   target/arm/tcg/op_helper.c
@@ -377,6 +377,7 @@ void HELPER(wfi)(CPUARMState *env, uint32_t insn_len)
     return;
 #else
     CPUState *cs = env_cpu(env);
+    ARMCPU *cpu = env_archcpu(env);
     uint32_t excp;
     int target_el = check_wfx_trap(env, false, &excp);
 
@@ -398,6 +399,7 @@ void HELPER(wfi)(CPUARMState *env, uint32_t insn_len)
                         target_el);
     }
 
+    cpu->wfxt_triggers_event = false;
     qatomic_set(&env->halt_reason, HALT_WFI);
     cs->exception_index = EXCP_HLT;
     cs->halted = 1;
@@ -461,6 +463,7 @@ void HELPER(wfit)(CPUARMState *env, uint32_t rd)
     } else {
         timer_mod(cpu->wfxt_timer, nexttick);
     }
+    cpu->wfxt_triggers_event = false;
     qatomic_set(&env->halt_reason, HALT_WFI);
     cs->exception_index = EXCP_HLT;
     cs->halted = 1;
@@ -720,12 +723,14 @@ void HELPER(wfet)(CPUARMState *env, uint32_t rd)
     next_event = gt_calc_next_event_stream(env);
     if (next_event > 0 && next_event < nexttick) {
         timer_mod(cpu->wfxt_timer, next_event);
+        cpu->wfxt_triggers_event = true;
     } else {
         if (nexttick == INT64_MAX) {
             timer_mod_ns(cpu->wfxt_timer, INT64_MAX);
         } else {
             timer_mod(cpu->wfxt_timer, nexttick);
         }
+        cpu->wfxt_triggers_event = false;
     }
 
     qatomic_set(&env->halt_reason, HALT_WFE);
--8<---------------cut here---------------end--------------->8---


>
> -- PMM

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro