Cui Jin has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/39036 )
Change subject: arch-riscv: fix unintentionally CSR bit overwritten in
different mode
......................................................................
arch-riscv: fix unintentionally CSR bit overwritten in different mode
Some CSR register is physically shared between different privilige
level. Current implementation of CSR setting only considers to verify
the bits visable in current privilige level, and directly writes the
masked bits back to register. This leads to other bits invisable
to current mode is overwritten and wrong behavior across the modes.
Thus, CSR updating should always keep the bits value for other modes.
e.g. disabling interrupt in S mode with setting
SSTATUS SIE bit will lead to clear MIE bit as well (the interrupt
is disabled unintentionally).
All CSR register sharing same physical register in different mode
may have similar issue. I only fixed some important ones.
The fix is verified in FS.
Jira Issue: https://gem5.atlassian.net/browse/GEM5-860
Change-Id: I34d4766a4b483b5add2c3bbefd28b21b9abf37f6
---
M src/arch/riscv/isa/formats/standard.isa
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/src/arch/riscv/isa/formats/standard.isa
b/src/arch/riscv/isa/formats/standard.isa
index b95af76..76c1996 100644
--- a/src/arch/riscv/isa/formats/standard.isa
+++ b/src/arch/riscv/isa/formats/standard.isa
@@ -356,6 +356,7 @@
break;
}
auto mask = CSRMasks.find(csr);
+ auto olddata_all = olddata;
if (mask != CSRMasks.end())
olddata &= mask->second;
DPRINTF(RiscvMisc, "Reading CSR %s: %#x\n", CSRData.at(csr).name,
@@ -373,23 +374,32 @@
CSRData.at(csr).name);
fault = make_shared<IllegalInstFault>(error,
machInst);
} else {
- DPRINTF(RiscvMisc, "Writing %#x to CSR %s.\n",
data,
+ auto newdata_all = data;
+ if (mask != CSRMasks.end()) {
+ // we must keep those original bits not in mask
+ // olddata and data only contain thebits
visable
+ // in current privilige level
+ newdata_all = (olddata_all & (~mask->second))
+ | data;
+ }
+ DPRINTF(RiscvMisc, "Writing %#x to CSR %s.\n",
+ newdata_all,
CSRData.at(csr).name);
- INTERRUPT oldinterrupt = olddata;
- INTERRUPT newinterrupt = data;
switch (csr) {
case CSR_FCSR:
xc->setMiscReg(MISCREG_FFLAGS, bits(data, 4,
0));
xc->setMiscReg(MISCREG_FRM, bits(data, 7, 5));
break;
case CSR_MIP: case CSR_MIE:
- if (oldinterrupt.mei != newinterrupt.mei ||
- oldinterrupt.mti != newinterrupt.mti ||
- oldinterrupt.msi != newinterrupt.msi) {
-
xc->setMiscReg(CSRData.at(csr).physIndex,data);
+ case CSR_SIP: case CSR_SIE:
+ case CSR_UIP: case CSR_UIE:
+ case CSR_MSTATUS: case CSR_SSTATUS: case
CSR_USTATUS:
+ if (newdata_all != olddata_all) {
+ xc->setMiscReg(CSRData.at(csr).physIndex,
+ newdata_all);
} else {
- std::string error = "Interrupt m bits are "
- "read-only\n";
+ std::string error = "Only bits in mask
are "
+ "allowed to be set\n";
fault =
make_shared<IllegalInstFault>(error,
machInst);
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39036
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: I34d4766a4b483b5add2c3bbefd28b21b9abf37f6
Gerrit-Change-Number: 39036
Gerrit-PatchSet: 1
Gerrit-Owner: Cui Jin <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
