derek/gem5
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

arch-riscv: fix unintentionally CSR bit overwritten in different mode

Browse Source 
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
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39036
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Ayaz Akram <yazakram@ucdavis.edu>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
This commit is contained in:
Cui Jin
2021-01-13 16:00:50 +08:00
parent 5a23207ee9
commit b1d7c8e77b
1 changed files with 19 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
src/arch/riscv/isa/formats/standard.isa
View File

@@ -356,6 +356,7 @@ def template CSRExecute {{
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,
@@ -374,23 +375,32 @@ def template CSRExecute {{
fault = std::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 = std::make_shared<IllegalInstFault>(
error, machInst);
}