Giacomo Travaglini has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/31775 )
Change subject: dev-arm: Fix <timer>_CTL_EL<x>.ISTATUS when masking the irq
......................................................................
dev-arm: Fix <timer>_CTL_EL<x>.ISTATUS when masking the irq
According to the ArmArm:
"When the value of the ENABLE bit is 1, ISTATUS indicates whether the
timer condition is met. ISTATUS takes no account of the value of the
IMASK bit. If the value of ISTATUS is 1 and the value of IMASK is 0 then
the timer interrupt is asserted."
Since ISTATUS is simply flagging that timer conditions are met, an
interrupt mask (via the <timer>_CTL_EL<x>.IMASK) shouldn't reset the
field to 0.
Clearing the ISTATUS bit leads to the following problem
as an example:
1) virtual timer (EL1) issuing a physical interrupt to the GIC
2) hypervisor handling the physical interrupt; setting the
CNTV_CTL_EL0.IMASK to 1 before issuing the virtual interrupt
to the VM
3) The VM receives the virtual interrupt but it gets confused
since CNTV_CTL_EL0.ISTATUS is 0 (due to point 2)
What happens when we disable the timer?
"When the value of the ENABLE bit is 0, the ISTATUS field is UNKNOWN."
So we are allowed to not clear the ISTATUS bit if the timer gets
disabled
Change-Id: I8eb32459a3ef6829c1910cf63815e102e2705566
Signed-off-by: Giacomo Travaglini <giacomo.travagl...@arm.com>
Reviewed-by: Andreas Sandberg <andreas.sandb...@arm.com>
Reviewed-by: Adrian Herrera <adrian.herr...@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/31775
Reviewed-by: Hsuan Hsu <kugwa2...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/dev/arm/generic_timer.cc
1 file changed, 13 insertions(+), 4 deletions(-)
Approvals:
Hsuan Hsu: Looks good to me, approved
Giacomo Travaglini: 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 7bb2def..a620eec 100644
--- a/src/dev/arm/generic_timer.cc
+++ b/src/dev/arm/generic_timer.cc
@@ -281,11 +281,14 @@
if (value() >= _counterLimit) {
counterLimitReached();
} else {
- if (_control.istatus) {
+ // Clear the interurpt when timers conditions are not met
+ if (_interrupt->active()) {
DPRINTF(Timer, "Clearing interrupt\n");
_interrupt->clear();
- _control.istatus = 0;
}
+
+ _control.istatus = 0;
+
if (scheduleEvents()) {
_parent.schedule(_counterLimitReachedEvent,
whenValue(_counterLimit));
@@ -320,10 +323,16 @@
// Timer masked or disabled
else if ((!old_ctl.imask && new_ctl.imask) ||
(old_ctl.enable && !new_ctl.enable)) {
- if (_control.istatus) {
+
+ if (_interrupt->active()) {
DPRINTF(Timer, "Clearing interrupt\n");
+ // We are clearing the interrupt but we are not
+ // setting istatus to 0 as we are doing
+ // in the updateCounter.
+ // istatus signals that Timer conditions are met.
+ // It shouldn't depend on masking.
+ // if enable is zero. istatus is unknown.
_interrupt->clear();
- _control.istatus = 0;
}
}
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/31775
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: I8eb32459a3ef6829c1910cf63815e102e2705566
Gerrit-Change-Number: 31775
Gerrit-PatchSet: 5
Gerrit-Owner: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Adrian Herrera <adrian.herr...@arm.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Hsuan Hsu <kugwa2...@gmail.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
