From: Fuad Tabba <[email protected]>

gt_recalc_timer() arms the timer for the ISTATUS 0->1 transition at
count == cval + offset, treating an overflow of that sum as "beyond
the counter's wrap" and arming at INT64_MAX. That reading is valid
only when offset <= count. When offset > count (a CNTVOFF_EL2 or
CNTPOFF_EL2 that puts the timer's counter ahead of the physical
count), the sum overflows for every cval still in the future and the
wrapped value is the correct next tick, which is what the code used
before commit 8d37a1425b99. The timer then never fires on its own.

A KVM guest whose counter is set ahead of the host's, as the
arch_timer_edge_cases selftest does via KVM_REG_ARM_TIMER_CNT, gets
its timer interrupt only when the vCPU is next loaded and KVM
rewrites the timer registers, so a guest polling for the interrupt
hangs.
Reproduced with VHE, nVHE and pKVM hosts on QEMU 11.0.0. The test
passes on hardware.

Test count + (cval - (count - offset)) for overflow instead, so only a
physical count past 2^64 is "never", and add a vtimer test case with
an offset that puts the count ahead.

Fixes: 8d37a1425b99 ("target/arm: Handle overflow in calculation of next timer 
tick")
Cc: [email protected]
Signed-off-by: Fuad Tabba <[email protected]>
Message-id: [email protected]
Reviewed-by: Peter Maydell <[email protected]>
Signed-off-by: Peter Maydell <[email protected]>
---
 target/arm/helper.c               |  9 +++++----
 tests/tcg/aarch64/system/vtimer.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 9a90ab68071..ff0b766b73d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1692,11 +1692,12 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
         } else {
             /*
              * Next transition is when (count - offset) == cval, i.e.
-             * when count == (cval + offset).
-             * If that would overflow, then again we set up the next interrupt
-             * for "as far in the future as possible" for the code below.
+             * cval - (count - offset) ticks from now. If count plus that
+             * overflows, set up "as far in the future as possible" below.
              */
-            if (uadd64_overflow(gt->cval, offset, &nexttick)) {
+            uint64_t remaining = gt->cval - (count - offset);
+
+            if (uadd64_overflow(count, remaining, &nexttick)) {
                 nexttick = UINT64_MAX;
             }
         }
diff --git a/tests/tcg/aarch64/system/vtimer.c 
b/tests/tcg/aarch64/system/vtimer.c
index 7d725eced34..e1cfad36be1 100644
--- a/tests/tcg/aarch64/system/vtimer.c
+++ b/tests/tcg/aarch64/system/vtimer.c
@@ -27,6 +27,7 @@
 
 int main(void)
 {
+    uint64_t freq, now;
     int i;
 
     ml_printf("VTimer Test\n");
@@ -44,5 +45,32 @@ int main(void)
         ml_printf("%d: cntv_cval_el0=%lx\n", i, read_sysreg(cntv_cval_el0));
     }
 
+    /*
+     * An offset that puts the virtual count ahead of the physical one,
+     * so cval + cntvoff wraps for every future cval. The timer must
+     * still fire. ISTATUS is set by the expiry, so poll it with a bound.
+     */
+    write_sysreg(cntv_ctl_el0, 0);
+    write_sysreg(cntvoff_el2, -(1ULL << 60));
+    asm volatile("isb");
+
+    freq = read_sysreg(cntfrq_el0);
+    now = read_sysreg(cntvct_el0);
+    write_sysreg(cntv_cval_el0, now + freq / 100);
+    write_sysreg(cntv_ctl_el0, 1);
+
+    ml_printf("cntvoff_el2=%lx\n", read_sysreg(cntvoff_el2));
+    ml_printf("cntvct_el0=%lx\n", now);
+    ml_printf("cntv_cval_el0=%lx\n", read_sysreg(cntv_cval_el0));
+
+    while (!(read_sysreg(cntv_ctl_el0) & 4)) {
+        if (read_sysreg(cntvct_el0) - now > freq) {
+            ml_printf("FAIL: ISTATUS not set within 1s: cntv_ctl_el0=%lx\n",
+                      read_sysreg(cntv_ctl_el0));
+            return 1;
+        }
+    }
+    ml_printf("ISTATUS set at cntvct_el0=%lx\n", read_sysreg(cntvct_el0));
+
     return 0;
 }
-- 
2.43.0


Reply via email to