From: "Scott J. Goldman" <scottjgo@gmail.com>
To: "Peter Maydell" <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org, "Alexander Graf" <agraf@csgraf.de>,
"Phil Dennis-Jordan" <phil@philjordan.eu>,
"Roman Bolshakov" <rbolshakov@ddn.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
qemu-devel@nongnu.org
Subject: Re: [PATCH v3] target/arm/hvf: Fix WFI halting to stop idle vCPU spinning
Date: Tue, 12 May 2026 12:46:55 -0400 [thread overview]
Message-ID: <DIGUN8EOAZ7C.3VYBVHXL4F06O@gmail.com> (raw)
In-Reply-To: <20260427195516.46256-1-scottjgo@gmail.com>
On Mon Apr 27, 2026 at 3:55 PM EDT, Scott J. Goldman 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 per-vCPU QEMU_CLOCK_VIRTUAL 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().
>
> 3. Clearing cpu->halted in hvf_arch_vcpu_exec() when cpu_has_work()
> indicates a pending interrupt, and cancelling the WFI timer.
>
> 4. Re-arming the WFI timer from hvf_vm_state_change() on the resume
> transition for any halted vCPU, since the QEMUTimer is per-instance
> state and is not migrated. After cpu_synchronize_all_states() the
> migrated vtimer state is mirrored in env, so we can read CNTV_CTL
> and CNTV_CVAL from there. If the vtimer has already expired by the
> time the destination resumes, hvf_wfi_timer_cb() is invoked
> directly so the halted vCPU is woken up.
>
> Fixes: b5f8f77271 ("accel/hvf: Implement WFI without using pselect()")
> Signed-off-by: Scott J. Goldman <scottjgo@gmail.com>
> ---
> Changes since v2:
> - Use QEMU_CLOCK_VIRTUAL instead of QEMU_CLOCK_HOST so the timer
> pauses with the VM and a halted vCPU isn't woken (or its IRQ
> raised) while the user has stopped the guest. (Peter)
> - Convert vtimer ticks to nanoseconds with muldiv64() to avoid
> intermediate overflow. (Peter)
> - Re-arm the WFI timer from hvf_vm_state_change() on the resume
> transition so a halted vCPU on the migration destination is
> woken when its vtimer expires (the QEMUTimer is per-instance
> state and isn't migrated). (Peter)
> v2: https://lore.kernel.org/qemu-devel/20260410055045.63001-1-scottjgo@gmail.com/
> v1: https://lore.kernel.org/qemu-devel/20260410044726.61853-1-scottjgo@gmail.com/
>
> include/system/hvf_int.h | 1 +
> target/arm/hvf/hvf.c | 124 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 124 insertions(+), 1 deletion(-)
>
> 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
> };
>
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index 678afe5c8e..a19d7a5e1f 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -28,6 +28,7 @@
> #include "hw/core/boards.h"
> #include "hw/core/irq.h"
> #include "qemu/main-loop.h"
> +#include "qemu/timer.h"
> #include "system/cpus.h"
> #include "arm-powerctl.h"
> #include "target/arm/cpu.h"
> @@ -301,6 +302,8 @@ void hvf_arm_init_debug(void)
> #define TMR_CTL_IMASK (1 << 1)
> #define TMR_CTL_ISTATUS (1 << 2)
>
> +static void hvf_wfi_timer_cb(void *opaque);
> +
> static uint32_t chosen_ipa_bit_size;
>
> typedef struct HVFVTimer {
> @@ -1214,6 +1217,9 @@ void hvf_arch_vcpu_destroy(CPUState *cpu)
> {
> hv_return_t ret;
>
> + timer_free(cpu->accel->wfi_timer);
> + cpu->accel->wfi_timer = NULL;
> +
> ret = hv_vcpu_destroy(cpu->accel->fd);
> assert_hvf_ok(ret);
> }
> @@ -1352,6 +1358,9 @@ int hvf_arch_init_vcpu(CPUState *cpu)
> arm_cpu->isar.idregs[ID_AA64MMFR0_EL1_IDX]);
> assert_hvf_ok(ret);
>
> + cpu->accel->wfi_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> + hvf_wfi_timer_cb, cpu);
> +
> aarch64_add_sme_properties(OBJECT(cpu));
> return 0;
> }
> @@ -2027,8 +2036,67 @@ static uint64_t hvf_vtimer_val_raw(void)
> return mach_absolute_time() - hvf_state->vtimer_offset;
> }
>
> +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;
> +}
> +
> +/*
> + * Arm a host-side QEMU_CLOCK_VIRTUAL timer to fire when the guest's
> + * vtimer (CNTV_CVAL_EL0) is scheduled to expire. HVF only delivers
> + * HV_EXIT_REASON_VTIMER_ACTIVATED during hv_vcpu_run(), which we won't
> + * call while the vCPU is halted, so we need this to wake the vCPU.
> + *
> + * QEMU_CLOCK_VIRTUAL pauses while the VM is stopped, which keeps the
> + * timer in lockstep with the guest's view of vtime across pause/resume.
> + *
> + * Caller must supply the current CNTV_CTL_EL0 and CNTV_CVAL_EL0 values,
> + * since the appropriate source (HVF vs. env) depends on context.
> + *
> + * Returns 0 if the timer was armed (or if the vtimer is disabled/masked
> + * and the vCPU should still halt waiting on another event), or -1 if
> + * the vtimer has already expired.
> + */
> +static int hvf_arm_wfi_timer(CPUState *cpu, uint64_t ctl, uint64_t cval)
> +{
> + ARMCPU *arm_cpu = ARM_CPU(cpu);
> + uint64_t now;
> + int64_t delta_ns;
> +
> + if (!(ctl & TMR_CTL_ENABLE) || (ctl & TMR_CTL_IMASK)) {
> + return 0;
> + }
> +
> + now = hvf_vtimer_val_raw();
> + if (cval <= now) {
> + return -1;
> + }
> +
> + delta_ns = muldiv64(cval - now, NANOSECONDS_PER_SECOND,
> + arm_cpu->gt_cntfrq_hz);
> + timer_mod(cpu->accel->wfi_timer,
> + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + delta_ns);
> + return 0;
> +}
> +
> static int hvf_wfi(CPUState *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 +2105,22 @@ static int hvf_wfi(CPUState *cpu)
> return 0;
> }
>
> + /*
> + * Read the vtimer state directly from HVF. We're on the vCPU thread,
> + * just exited from hv_vcpu_run(), so HVF holds the authoritative
> + * values and env may be stale.
> + */
> + r = hv_vcpu_get_sys_reg(cpu->accel->fd, HV_SYS_REG_CNTV_CTL_EL0, &ctl);
> + assert_hvf_ok(r);
> + r = hv_vcpu_get_sys_reg(cpu->accel->fd, HV_SYS_REG_CNTV_CVAL_EL0, &cval);
> + assert_hvf_ok(r);
> +
> + if (hvf_arm_wfi_timer(cpu, ctl, cval) < 0) {
> + /* vtimer already expired, don't halt */
> + return 0;
> + }
> +
> + cpu->halted = 1;
> return EXCP_HLT;
> }
>
> @@ -2332,7 +2416,11 @@ int hvf_arch_vcpu_exec(CPUState *cpu)
> hv_return_t r;
>
> if (cpu->halted) {
> - return EXCP_HLT;
> + if (!cpu_has_work(cpu)) {
> + return EXCP_HLT;
> + }
> + cpu->halted = 0;
> + timer_del(cpu->accel->wfi_timer);
> }
>
> flush_cpu_state(cpu);
> @@ -2376,11 +2464,45 @@ static const VMStateDescription vmstate_hvf_vtimer = {
> static void hvf_vm_state_change(void *opaque, bool running, RunState state)
> {
> HVFVTimer *s = opaque;
> + CPUState *cpu;
>
> if (running) {
> /* Update vtimer offset on all CPUs */
> hvf_state->vtimer_offset = mach_absolute_time() - s->vtimer_val;
> cpu_synchronize_all_states();
> +
> + /*
> + * After migration restore (or any resume), the wfi_timer is not
> + * scheduled on this QEMU instance, so re-arm it for any halted
> + * vCPU with a pending vtimer. For a non-migration resume the
> + * QEMU_CLOCK_VIRTUAL timer was already scheduled; recomputing the
> + * deadline produces the same value and is a harmless no-op.
> + *
> + * cpu_synchronize_all_states() above ensures env mirrors the
> + * authoritative vtimer state (whether that came from HVF or from
> + * the migration stream), so we can safely read it here from the
> + * iothread.
> + */
> + CPU_FOREACH(cpu) {
> + ARMCPU *arm_cpu;
> + uint64_t ctl, cval;
> +
> + if (!cpu->accel || !cpu->halted) {
> + continue;
> + }
> +
> + arm_cpu = ARM_CPU(cpu);
> + ctl = arm_cpu->env.cp15.c14_timer[GTIMER_VIRT].ctl;
> + cval = arm_cpu->env.cp15.c14_timer[GTIMER_VIRT].cval;
> +
> + if (hvf_arm_wfi_timer(cpu, ctl, cval) < 0) {
> + /*
> + * vtimer already expired while we were paused; raise the
> + * IRQ now so the halted vCPU wakes up.
> + */
> + hvf_wfi_timer_cb(cpu);
> + }
> + }
> } else {
> /* Remember vtimer value on every pause */
> s->vtimer_val = hvf_vtimer_val_raw();
Hi Peter--
Sorry to nag, just was wondering if you could take a look at this follow-up
to your earlier review comments in:
https://lore.kernel.org/qemu-devel/CAFEAcA9RHQ+7++=kLn2goJwcgzDnaqdkQtQBtxQ2Rw1-uiKY=g@mail.gmail.com/#t
It's a bit of an unfortunate regression, so I was hoping to get some form of
the fix in.
Additionally, re: migration under hvf being broken, it looks like one of my
fixes got merged, but there is one patch remaining here as well:
https://lore.kernel.org/qemu-devel/5e123304-628a-4da9-81ca-585498c809b6@linaro.org/
Thanks, and hope the ping is ok (just following the docs that say to retry
every 2 weeks)
-sjg
next prev parent reply other threads:[~2026-05-12 16:47 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-10 4:47 [PATCH] target/arm/hvf: Fix WFI halting to stop idle vCPU spinning Scott J. Goldman
2026-04-10 5:06 ` Mohamed Mediouni
2026-04-10 5:28 ` Scott J. Goldman
2026-04-10 5:50 ` [PATCH v2] " Scott J. Goldman
2026-04-10 6:18 ` Mohamed Mediouni
2026-04-16 21:20 ` Scott J. Goldman
2026-04-17 9:57 ` Philippe Mathieu-Daudé
2026-04-17 20:30 ` Scott J. Goldman
2026-04-21 23:24 ` Scott J. Goldman
2026-04-27 11:15 ` Peter Maydell
2026-04-27 18:42 ` Scott J. Goldman
2026-04-27 19:55 ` [PATCH v3] " Scott J. Goldman
2026-05-12 16:46 ` Scott J. Goldman [this message]
2026-05-12 19:18 ` Peter Maydell
2026-05-12 17:54 ` Mohamed Mediouni
2026-05-12 20:36 ` Scott J. Goldman
2026-05-12 20:51 ` Mohamed Mediouni
2026-05-13 1:21 ` Mohamed Mediouni
2026-05-13 2:21 ` [PATCH v11.1+ v4] " Scott J. Goldman
2026-05-13 2:25 ` [PATCH v3] " Scott J. Goldman
2026-05-13 7:14 ` Mohamed Mediouni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DIGUN8EOAZ7C.3VYBVHXL4F06O@gmail.com \
--to=scottjgo@gmail.com \
--cc=agraf@csgraf.de \
--cc=peter.maydell@linaro.org \
--cc=phil@philjordan.eu \
--cc=philmd@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=rbolshakov@ddn.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox