From c513835c4f15530828c577992f31bead90588539 Mon Sep 17 00:00:00 2001 From: Hsuan Hsu Date: Wed, 10 Jun 2020 11:10:33 +0800 Subject: [PATCH] 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 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/30918 Reviewed-by: Andreas Sandberg Maintainer: Andreas Sandberg Tested-by: kokoro --- src/dev/arm/generic_timer.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/dev/arm/generic_timer.cc b/src/dev/arm/generic_timer.cc index 42b03adfa..bf6cd4eb0 100644 --- 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(); -- 2.30.2