Hsuan Hsu has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/30918 )
Change subject: dev-arm: Fix handling of writing timer control registers
......................................................................
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 <[email protected]>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/30918
Reviewed-by: Andreas Sandberg <[email protected]>
Maintainer: Andreas Sandberg <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/dev/arm/generic_timer.cc
1 file changed, 6 insertions(+), 4 deletions(-)
Approvals:
Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/dev/arm/generic_timer.cc b/src/dev/arm/generic_timer.cc
index 42b03ad..bf6cd4e 100644
--- a/src/dev/arm/generic_timer.cc
+++ b/src/dev/arm/generic_timer.cc
@@ -313,11 +313,13 @@
_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();
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30918
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I75e8253bb78d15ae72cb985ed132f896d8e92ca6
Gerrit-Change-Number: 30918
Gerrit-PatchSet: 7
Gerrit-Owner: Hsuan Hsu <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Giacomo Travaglini <[email protected]>
Gerrit-Reviewer: Hsuan Hsu <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
