Re: [PATCH v2] target/arm/hvf: Fix WFI halting to stop idle vCPU spinning

["Scott J. Goldman" <scottjgo@gmail.com>]

On Mon Apr 27, 2026 at 4:15 AM PDT, Peter Maydell wrote:
> On Fri, 10 Apr 2026 at 06:50, Scott J. Goldman <scottjgo@gmail.com> wrote:
>>
>> Commit b5f8f77271 ("accel/hvf: Implement WFI without using pselect()")
>> changed hvf_wfi() from blocking the vCPU thread with pselect() to
>> returning EXCP_HLT, intending QEMU's main event loop to handle the
>> idle wait. However, cpu->halted was never set, so cpu_thread_is_idle()
>> always returns false and the vCPU thread spins at 100% CPU per core
>> while the guest is idle.
>>
>> Fix this by:
>>
>> 1. Setting cpu->halted = 1 in hvf_wfi() so the vCPU thread sleeps on
>>    halt_cond in qemu_process_cpu_events().
>>
>> 2. Arming a host-side QEMU_CLOCK_HOST timer to fire when the guest's
>>    virtual timer (CNTV_CVAL_EL0) would expire. This is necessary
>>    because HVF only delivers HV_EXIT_REASON_VTIMER_ACTIVATED during
>>    hv_vcpu_run(), which is not called while the CPU is halted. The
>>    timer callback mirrors the VTIMER_ACTIVATED handler: it raises the
>>    vtimer IRQ through the GIC and marks vtimer_masked, causing the
>>    interrupt delivery chain to wake the vCPU via qemu_cpu_kick().
>
> A host side timer here seems a bit odd -- it will still
> expire even if the user halts the whole VM. We only use
> QEMU_CLOCK_HOST timers for things that are part of QEMU proper,
> like handling timeouts in the migration networking code.
>
>> diff --git a/include/system/hvf_int.h b/include/system/hvf_int.h
>> index 2621164cb2..58fb865eba 100644
>> --- a/include/system/hvf_int.h
>> +++ b/include/system/hvf_int.h
>> @@ -48,6 +48,7 @@ struct AccelCPUState {
>>      hv_vcpu_exit_t *exit;
>>      bool vtimer_masked;
>>      bool guest_debug_enabled;
>> +    struct QEMUTimer *wfi_timer;
>>  #endif
>>  };
>
> QEMUTimer objects need to be migrated. I think as it is now,
> if you do a migration while one vCPU is in WFI we won't
> migrate the timer state, and so on the destination it won't
> be woken up when it should.

Hi Peter--

Thanks for the review here. I am going to post a follow-up patch based
on your comments, but I did want to just flag that migration is broken
in hvf/arm right now (segfaults in the dirty page tracking code). Of
course, I can post fixes for that as well.


>
>> +static void hvf_wfi_timer_cb(void *opaque)
>> +{
>> +    CPUState *cpu = opaque;
>> +    ARMCPU *arm_cpu = ARM_CPU(cpu);
>> +
>> +    /*
>> +     * vtimer expired while the CPU was halted for WFI.
>> +     * Mirror HV_EXIT_REASON_VTIMER_ACTIVATED: raise the vtimer
>> +     * interrupt and mark as masked so hvf_sync_vtimer() will
>> +     * check and unmask when the guest handles it.
>> +     *
>> +     * The interrupt delivery chain (GIC -> cpu_interrupt ->
>> +     * qemu_cpu_kick) wakes the vCPU thread from halt_cond.
>> +     */
>> +    qemu_set_irq(arm_cpu->gt_timer_outputs[GTIMER_VIRT], 1);
>> +    cpu->accel->vtimer_masked = true;
>
> If the VM happens to be stopped by the user when the timer
> fires, this will change the VM state, which I suspect is
> not correct. Avoiding use of a QEMU_CLOCK_HOST timer would
> fix this.
>
>> +}
>> +
>>  static int hvf_wfi(CPUState *cpu)
>>  {
>> +    ARMCPU *arm_cpu = ARM_CPU(cpu);
>> +    uint64_t ctl, cval;
>> +    hv_return_t r;
>> +
>>      if (cpu_has_work(cpu)) {
>>          /*
>>           * Don't bother to go into our "low power state" if
>> @@ -2037,6 +2068,34 @@ static int hvf_wfi(CPUState *cpu)
>>          return 0;
>>      }
>>
>> +    /*
>> +     * Set up a host-side timer to wake us when the vtimer expires.
>> +     * HVF only delivers HV_EXIT_REASON_VTIMER_ACTIVATED during
>> +     * hv_vcpu_run(), which we won't call while halted.
>> +     */
>> +    r = hv_vcpu_get_sys_reg(cpu->accel->fd, HV_SYS_REG_CNTV_CTL_EL0, &ctl);
>> +    assert_hvf_ok(r);
>> +
>> +    if ((ctl & TMR_CTL_ENABLE) && !(ctl & TMR_CTL_IMASK)) {
>> +        r = hv_vcpu_get_sys_reg(cpu->accel->fd,
>> +                                HV_SYS_REG_CNTV_CVAL_EL0, &cval);
>> +        assert_hvf_ok(r);
>> +
>> +        uint64_t now = hvf_vtimer_val_raw();
>> +        if (cval <= now) {
>> +            /* Timer already expired, don't halt */
>> +            return 0;
>> +        }
>> +
>> +        uint64_t delta_ticks = cval - now;
>> +        int64_t delta_ns = delta_ticks * NANOSECONDS_PER_SECOND
>> +                           / arm_cpu->gt_cntfrq_hz;
>
> As a general rule, tick-to-ns and vice-versa conversions should
> use muldiv64() to avoid potential overflow in the intermediate value.
>
>> +        int64_t deadline = qemu_clock_get_ns(QEMU_CLOCK_HOST) + delta_ns;
>> +
>> +        timer_mod(cpu->accel->wfi_timer, deadline);
>> +    }
>> +
>> +    cpu->halted = 1;
>>      return EXCP_HLT;
>>  }
>
> thanks
> -- PMM