All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fuad Tabba <fuad.tabba@linux.dev>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org, qemu-devel@nongnu.org,
	qemu-stable@nongnu.org, "Alex Bennée" <alex.bennee@linaro.org>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Will Deacon" <will@kernel.org>, "Fuad Tabba" <tabba@google.com>
Subject: [PATCH 2/2] target/arm: Fix WFxT timeouts when the offset puts the count ahead
Date: Mon, 21 Sep 2026 08:44:51 +0100	[thread overview]
Message-ID: <20260921074451.3158645-3-fuad.tabba@linux.dev> (raw)
In-Reply-To: <20260921074451.3158645-1-fuad.tabba@linux.dev>

The WFIT and WFET helpers arm the wakeup timer at count == timeout +
offset, treating an overflow of that sum as "beyond the counter's
wrap" and arming at INT64_MAX. As in gt_recalc_timer(), that reading
is valid only when offset <= count: with a CNTVOFF_EL2 that puts the
virtual count ahead of the physical count, the sum overflows for
every timeout still in the future and the wrapped value was the
correct wakeup. The CPU then waits until an interrupt or event
instead of waking at its timeout. A Linux guest uses WFIT and WFET in
__delay() when FEAT_WFxT is present, which -cpu max advertises.

Arm cntval + (timeout - cntvct) instead, so only a physical count past
2^64 is "never", and add a tcg system test that issues WFIT and WFET
with such an offset, with a timer interrupt 1s out so a broken WFxT
still returns: before this change both wake at the interrupt, after
it at their timeout.

Fixes: a96edb687e76 ("target/arm: Implement FEAT WFxT and enable for '-cpu max'")
Fixes: da9b86c35fa8 ("target/arm: implement WFET")
Cc: qemu-stable@nongnu.org
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 target/arm/tcg/op_helper.c           |   6 +-
 tests/tcg/aarch64/system/meson.build |   7 ++
 tests/tcg/aarch64/system/wfxt.c      | 118 +++++++++++++++++++++++++++
 3 files changed, 129 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/aarch64/system/wfxt.c

diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
index c2b09176cb..562f11f390 100644
--- a/target/arm/tcg/op_helper.c
+++ b/target/arm/tcg/op_helper.c
@@ -448,7 +448,8 @@ void HELPER(wfit)(CPUARMState *env, uint32_t rd)
         raise_exception(env, excp, syn_wfx(1, 0xe, rd, true, WFIT, false), target_el);
     }
 
-    if (uadd64_overflow(timeout, offset, &nexttick)) {
+    /* Physical count at the timeout. Only an overflow of it is "never". */
+    if (uadd64_overflow(cntval, timeout - cntvct, &nexttick)) {
         nexttick = UINT64_MAX;
     }
     if (nexttick > INT64_MAX / gt_cntfrq_period_ns(cpu)) {
@@ -705,7 +706,8 @@ void HELPER(wfet)(CPUARMState *env, uint32_t rd)
      * The WFET should time out when CNTVCT_EL0 >= the specified value.
      */
     cpu = env_archcpu(env);
-    if (uadd64_overflow(timeout, offset, &nexttick)) {
+    /* Physical count at the timeout. Only an overflow of it is "never". */
+    if (uadd64_overflow(cntval, timeout - cntvct, &nexttick)) {
         nexttick = UINT64_MAX;
     }
     if (nexttick > INT64_MAX / gt_cntfrq_period_ns(cpu)) {
diff --git a/tests/tcg/aarch64/system/meson.build b/tests/tcg/aarch64/system/meson.build
index 94e16ba330..5d96a9c25a 100644
--- a/tests/tcg/aarch64/system/meson.build
+++ b/tests/tcg/aarch64/system/meson.build
@@ -72,6 +72,13 @@ tests += {
                   '-semihosting-config', 'enable=on,arg=2',
                   qemu_base_args]
   },
+  'wfxt.c': {
+    'cflags': cflags,
+    'qemu_args': ['-M', 'virt,virtualization=on,gic-version=2',
+                  '-cpu', 'max', '-smp', '1',
+                  '-semihosting-config', 'enable=on,arg=2',
+                  qemu_base_args]
+  },
 }
 
 tests += {
diff --git a/tests/tcg/aarch64/system/wfxt.c b/tests/tcg/aarch64/system/wfxt.c
new file mode 100644
index 0000000000..9d50590a68
--- /dev/null
+++ b/tests/tcg/aarch64/system/wfxt.c
@@ -0,0 +1,118 @@
+/*
+ * WFIT/WFET timeout test
+ *
+ * Copyright (c) 2026 Google LLC
+ * Author: Fuad Tabba <fuad.tabba@linux.dev>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include <stdint.h>
+#include <minilib.h>
+
+/* from Linux's include/linux/stringify.h */
+#define __stringify_1(x...) #x
+#define __stringify(x...)   __stringify_1(x)
+
+#define read_sysreg(r) ({                                           \
+            uint64_t __val;                                         \
+            asm volatile("mrs %0, " __stringify(r) : "=r" (__val)); \
+            __val;                                                  \
+})
+
+#define write_sysreg(r, v) do {                     \
+        uint64_t __val = (uint64_t)(v);             \
+        asm volatile("msr " __stringify(r) ", %x0"  \
+                 : : "rZ" (__val));                 \
+} while (0)
+
+/* .inst forms of WFIT x0 and WFET x0, for assemblers without FEAT_WFxT */
+static inline void wfit(uint64_t timeout)
+{
+    register uint64_t x0 asm("x0") = timeout;
+
+    asm volatile(".inst 0xd5031020" : : "r" (x0) : "memory");
+}
+
+static inline void wfet(uint64_t timeout)
+{
+    register uint64_t x0 asm("x0") = timeout;
+
+    asm volatile(".inst 0xd5031000" : : "r" (x0) : "memory");
+}
+
+/* virt machine, GICv2 */
+#define GICD_BASE 0x08000000UL
+#define GICC_BASE 0x08010000UL
+#define GICD_CTLR 0x000
+#define GICD_ISENABLER0 0x100
+#define GICC_CTLR 0x000
+#define GICC_PMR 0x004
+#define VTIMER_PPI 27
+
+static inline void mmio_write32(uintptr_t addr, uint32_t val)
+{
+    /* GIC registers: MMIO, MMU off at EL2 */
+    *(volatile uint32_t *)addr = val;
+}
+
+static int test_one(const char *name, void (*wait)(uint64_t), uint64_t freq)
+{
+    uint64_t now, timeout, elapsed;
+    int early = 0;
+
+    /*
+     * The virtual timer interrupt, 1s out, wakes the CPU even with
+     * interrupts masked. The timeout, 10ms out, must be what wakes it.
+     */
+    now = read_sysreg(cntvct_el0);
+    write_sysreg(cntv_cval_el0, now + freq);
+    write_sysreg(cntv_ctl_el0, 1);
+    timeout = now + freq / 100;
+
+    do {
+        wait(timeout);
+        early++;
+    } while (read_sysreg(cntvct_el0) < timeout && early < 1000);
+
+    elapsed = read_sysreg(cntvct_el0) - now;
+    write_sysreg(cntv_ctl_el0, 0);
+
+    ml_printf("%s: woke after %ld ticks (%d wakes)\n", name, elapsed, early);
+    if (early >= 1000) {
+        ml_printf("FAIL: %s kept waking before its timeout\n", name);
+        return 1;
+    }
+    if (elapsed > freq / 2) {
+        ml_printf("FAIL: %s woke on the timer interrupt, not its timeout\n",
+                  name);
+        return 1;
+    }
+    return 0;
+}
+
+int main(void)
+{
+    uint64_t freq;
+    int ret;
+
+    ml_printf("WFxT Test\n");
+
+    mmio_write32(GICD_BASE + GICD_ISENABLER0, 1u << VTIMER_PPI);
+    mmio_write32(GICD_BASE + GICD_CTLR, 1);
+    mmio_write32(GICC_BASE + GICC_PMR, 0xff);
+    mmio_write32(GICC_BASE + GICC_CTLR, 1);
+
+    /* Put the virtual count ahead of the physical count */
+    write_sysreg(cntvoff_el2, -(1ULL << 60));
+    asm volatile("isb");
+    freq = read_sysreg(cntfrq_el0);
+
+    ml_printf("cntvoff_el2=%lx cntfrq_el0=%ld\n",
+              read_sysreg(cntvoff_el2), freq);
+
+    ret = test_one("wfit", wfit, freq);
+    ret |= test_one("wfet", wfet, freq);
+
+    return ret;
+}
-- 
2.39.5



  parent reply	other threads:[~2026-09-21 12:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-21  7:44 [PATCH 0/2] target/arm: Fix timer deadlines when the offset puts the count ahead Fuad Tabba
2026-09-21  7:44 ` [PATCH 1/2] target/arm: Fix next timer tick " Fuad Tabba
2026-09-21  7:44 ` Fuad Tabba [this message]
2026-09-30  8:56   ` [PATCH 2/2] target/arm: Fix WFxT timeouts " Michael Tokarev
2026-09-30 11:41     ` Peter Maydell
2026-09-28 16:20 ` [PATCH 0/2] target/arm: Fix timer deadlines " Peter Maydell

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=20260921074451.3158645-3-fuad.tabba@linux.dev \
    --to=fuad.tabba@linux.dev \
    --cc=alex.bennee@linaro.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=tabba@google.com \
    --cc=will@kernel.org \
    /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.