dev-arm: Fix handling of writing timer control registers

We should also deal with change of the imask bit, or we will lose timer
interrupt if the timer expires before the guest kernel unmasks the bit.
More precisely, consider the following common pattern in timer interrupt
handling:

    1. Set the interrupt mask bit (CNTV_CTL.IMASK)
    2. Reprogram the downcounter (CNTV_TVAL) for the next interrupt
    3. Clear the interrupt mask bit (CNTV_CTL.IMASK)

The timer can expires between step 2 & 3 if the value programmed in step
2 is small enough, and this seems very likely to happen in KVM mode. If
we don't check for timer expiration right after unmasking, we will miss
the only chance to inject the interrupt.

JIRA: https://gem5.atlassian.net/browse/GEM5-663

Change-Id: I75e8253bb78d15ae72cb985ed132f896d8e92ca6
Signed-off-by: Hsuan Hsu <hsuan.hsu@mediatek.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/30918
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Maintainer: Andreas Sandberg <andreas.sandberg@arm.com>
Tested-by: kokoro <noreply+kokoro@google.com>

diff --git a/src/dev/arm/generic_timer.cc b/src/dev/arm/generic_timer.cc
index 42b03adfae6aca8d8b033a230bab0012ad8e3bd4..bf6cd4eb03390e106c4d5ab890977475a7bec917 100644 (file)
--- a/src/dev/arm/generic_timer.cc
+++ b/src/dev/arm/generic_timer.cc
@@ -313,11 +313,13 @@ ArchTimer::setControl(uint32_t val)
     _control.enable = new_ctl.enable;
     _control.imask = new_ctl.imask;
     _control.istatus = old_ctl.istatus;
-    // Timer enabled
-    if (!old_ctl.enable && new_ctl.enable)
+    // Timer unmasked or enabled
+    if ((old_ctl.imask && !new_ctl.imask) ||
+        (!old_ctl.enable && new_ctl.enable))
         updateCounter();
-    // Timer disabled
-    else if (old_ctl.enable && !new_ctl.enable) {
+    // Timer masked or disabled
+    else if ((!old_ctl.imask && new_ctl.imask) ||
+             (old_ctl.enable && !new_ctl.enable)) {
         if (_control.istatus) {
             DPRINTF(Timer, "Clearing interrupt\n");
             _interrupt->clear();