public inbox for qemu-arm@nongnu.org
 help / color / mirror / Atom feed
From: "Scott J. Goldman" <scottjgo@gmail.com>
To: "Mohamed Mediouni" <mohamed@unpredictable.fr>,
	"Scott J. Goldman" <scottjgo@gmail.com>
Cc: <qemu-devel@nongnu.org>, <qemu-arm@nongnu.org>,
	<rbolshakov@ddn.com>, <phil@philjordan.eu>, <agraf@csgraf.de>,
	<peter.maydell@linaro.org>
Subject: Re: [PATCH] target/arm/hvf: Fix WFI halting to stop idle vCPU spinning
Date: Thu, 09 Apr 2026 22:28:42 -0700	[thread overview]
Message-ID: <DHP86IRPEW4G.16N17DFVR56EW@gmail.com> (raw)
In-Reply-To: <E41FA391-ABC2-4CA9-850C-BDB3263C1E07@unpredictable.fr>

On Thu Apr 9, 2026 at 10:06 PM PDT, Mohamed Mediouni wrote:
>
>
>> On 10. Apr 2026, at 06:47, 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().
>> 
>> 3. Clearing cpu->halted in hvf_arch_vcpu_exec() when cpu_has_work()
>>   indicates a pending interrupt, and cancelling the WFI timer.
>> 
>> Fixes: b5f8f77271 ("accel/hvf: Implement WFI without using pselect()")
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/959
>> Signed-off-by: Scott J. Goldman <scottjgo@gmail.com>
>
> Hi,
>
> Reviewed-by: Mohamed Mediouni <mohamed@unpredictable.fr>
>> ---
>> include/system/hvf_int.h |  1 +
>> target/arm/hvf/hvf.c     | 65 +++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 65 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 5fc8f6bbbd..0e737d3583 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_HOST,
>> +                                          hvf_wfi_timer_cb, cpu);
>> +
>>     aarch64_add_sme_properties(OBJECT(cpu));
>>     return 0;
>> }
>> @@ -2027,8 +2036,29 @@ 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.
>> +     * 	
>> +     * 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;
>> +}
>> +
>> 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 +2067,35 @@ 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;
>
> We have CNTFRQ_EL0 in cpu->gt_cntfrq_hz but this works too
> as XNU today tries to take care that host and VM run with
> the same CNTFRQ_EL0
>

Ah, I can post a follow-up.

>> +        mach_timebase_info_data_t timebase;
>> +        mach_timebase_info(&timebase);
>> +        int64_t delta_ns = delta_ticks * timebase.numer / timebase.denom;
>> +        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;
>> }
>> 
>> @@ -2332,7 +2391,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);
>> -- 
>> 2.50.1 (Apple Git-155)
>> 
>> 

Thanks,
-sjg


  reply	other threads:[~2026-04-10  6:44 UTC|newest]

Thread overview: 5+ 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 [this message]
2026-04-10  5:50 ` [PATCH v2] " Scott J. Goldman
2026-04-10  6:18   ` 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=DHP86IRPEW4G.16N17DFVR56EW@gmail.com \
    --to=scottjgo@gmail.com \
    --cc=agraf@csgraf.de \
    --cc=mohamed@unpredictable.fr \
    --cc=peter.maydell@linaro.org \
    --cc=phil@philjordan.eu \
    --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