Il lun 12 ott 2026, 20:47 Alex Bennée <[email protected]> ha scritto:
> Paolo Bonzini <[email protected]> writes: > > > > I think wfxt_timer_cb only needs to call cpu_interrupt(). > > Something like: > No, the cpu_interrupt must be unconditional due to the WFIT instruction. Your patch puts it under qatomic_read(&env->halt_reason) == HALT_WFE. > > + > + qatomic_set(&cpu->env.halt_reason, NOT_HALTED); > + > if (cpu->wfxt_timer) { > timer_del(cpu->wfxt_timer); > } > - /* clear the halt reason */ > - qatomic_set(&cpu->env.halt_reason, NOT_HALTED); > I was thinking about the ordering before timer_mod, in the wfe/wfi/wfet/wfit helpers. However it is not relevant if the timer callback does not check halt_reason. Paolo } > return leave_halt; > } > #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. > + * Unlike almost everything else that messes with the halt_reason the > + * timer callbacks are not in the vCPU context so we must use atomics > + * to check it. > */ > static void arm_wfxt_timer_cb(void *opaque) > { > @@ -903,17 +900,15 @@ static void arm_wfxt_timer_cb(void *opaque) > CPUState *cs = CPU(cpu); > CPUARMState *env = &cpu->env; > > - if (qatomic_cmpxchg(&env->halt_reason, HALT_WFE, NOT_HALTED)) { > - qatomic_set(&env->event_register, true); > + if (qatomic_read(&env->halt_reason) == HALT_WFE) { > + /* > + * We expect the CPU to be halted; this will cause > arm_cpu_is_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. > + */ > + cpu_interrupt(cs, CPU_INTERRUPT_EXITTB); > } > - > - /* > - * We expect the CPU to be halted; this will cause arm_cpu_is_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. > - */ > - cpu_interrupt(cs, CPU_INTERRUPT_EXITTB); > } > #endif > --8<---------------cut here---------------end--------------->8--- > > ? > > > > > Thanks, > > > > Paolo > > > > (*) always, even if the halt reason is not HALT_WFE, if only for > consistency > > > > (**) you don't need a lock or acquire/release here, because the timer > > callback always sees everything that happened before timer_mod(). In > > a sense, timer_mod() *is* the release operation and starting the > > callback is the acquire operation. > > -- > Alex Bennée > Virtualisation Tech Lead @ Linaro > >
