All of lore.kernel.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: 22+ 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
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
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-15 10:39         ` Peter Maydell
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=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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.