[Qemu-devel] [PATCH] riscv: Fix timer overflow in sifive_clint.c

[Anthony Coulter]

If a hart writes -1 to mtimecmp it should not receive a timer interrupt
for (2^64 - 1)/(10 MHz) = 58455 years assuming a 10 MHz realtime clock.
But in practice I get a timer interrupt immediately because of an
integer overflow bug when QEMU converts realtime clock ticks into
nanoseconds.

My proposal is to check for this overflow and, on detecting it, setting
the timer to fire as far in the future as QEMU can support:

        INT64_MAX = 2^63 - 1 nanoseconds = 292 years

This patch is technically incomplete, in that someone running QEMU for
292 years will eventually receive a timer interrupt for the hart even
though mtime < mtimecmp. But I'm fixing this for my own purposes, and
"my purposes" don't involve running QEMU for more than an hour at a
time. So it'll be fine.

My patch also eliminates some computations that canceled each other
out (e.g. subtracting the realtime clock and then adding it back in,
but using different units for each). So far as I can tell, the required
calculation is just:

        next = value*1000

but I dressed it up with symbolic constants and the fancy muldiv64 to
get more precise results in case someone changes the SIFIVE frequency
to something that isn't an exact factor of 1 billion.

Regards,
Anthony Coulter

diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
index e5a8f75cee..d6d66082e0 100644
--- a/hw/riscv/sifive_clint.c
+++ b/hw/riscv/sifive_clint.c
@@ -41,8 +41,6 @@ static uint64_t cpu_riscv_read_rtc(void)
 static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value)
 {
     uint64_t next;
-    uint64_t diff;
-
     uint64_t rtc_r = cpu_riscv_read_rtc();
 
     cpu->env.timecmp = value;
@@ -55,10 +53,15 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, 
uint64_t value)
 
     /* otherwise, set up the future timer interrupt */
     riscv_cpu_update_mip(cpu, MIP_MTIP, BOOL_TO_MASK(0));
-    diff = cpu->env.timecmp - rtc_r;
-    /* back to ns (note args switched in muldiv64) */
-    next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-        muldiv64(diff, NANOSECONDS_PER_SECOND, SIFIVE_CLINT_TIMEBASE_FREQ);
+
+    /* check for integer overflow */
+    if (value >= muldiv64(INT64_MAX, SIFIVE_CLINT_TIMEBASE_FREQ,
+        NANOSECONDS_PER_SECOND))
+        next = INT64_MAX;
+    else
+        next = muldiv64(value, NANOSECONDS_PER_SECOND,
+            SIFIVE_CLINT_TIMEBASE_FREQ);
+
     timer_mod(cpu->env.timer, next);
 }