Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/55223 )
Change subject: arch-riscv: Split up read/write and read only CSR
instructions.
......................................................................
arch-riscv: Split up read/write and read only CSR instructions.
If RS1 is X0 or if using an immediate and the value encoded in the RS1
field which is used as the immediate is zero, then the CSR is not
actually written. It doesn't matter whether the register value would
change or not.
Also, if an instruction wants to write to the CSR, the manual does not
say anything about whether or not the register value changes. If a
register is read only, attempting to write it with *any* value should be
illegal.
Change-Id: I5247941d4689d552aa73170c45f472b7d72ed88d
---
M src/arch/riscv/isa/formats/standard.isa
1 file changed, 113 insertions(+), 46 deletions(-)
diff --git a/src/arch/riscv/isa/formats/standard.isa
b/src/arch/riscv/isa/formats/standard.isa
index 9345c1f..5d2bee5 100644
--- a/src/arch/riscv/isa/formats/standard.isa
+++ b/src/arch/riscv/isa/formats/standard.isa
@@ -276,7 +276,7 @@
}
}};
-def template CSRExecute {{
+def template CSRExecuteRo {{
Fault
%(class_name)s::execute(ExecContext *xc,
Trace::InstRecord *traceData) const
@@ -289,8 +289,6 @@
%(op_decl)s;
%(op_rd)s;
- RegVal data, olddata;
-
switch (csr) {
case CSR_SATP: {
auto pm = (PrivilegeMode)xc->readMiscReg(MISCREG_PRV);
@@ -315,55 +313,91 @@
break;
}
+ RegVal data;
if (csr == CSR_FCSR) {
- olddata = xc->readMiscReg(MISCREG_FFLAGS) |
+ data = xc->readMiscReg(MISCREG_FFLAGS) |
+ (xc->readMiscReg(MISCREG_FRM) << FRM_OFFSET);
+ } else {
+ data = xc->readMiscReg(midx);
+ }
+
+ DPRINTF(RiscvMisc, "Reading CSR %s: %#x\n", csrName, data);
+
+ %(code)s;
+ %(op_wb)s;
+
+ return NoFault;
+ }
+}};
+
+def template CSRExecuteRw {{
+ Fault
+ %(class_name)s::execute(ExecContext *xc,
+ Trace::InstRecord *traceData) const
+ {
+ if (!valid) {
+ return std::make_shared<IllegalInstFault>(
+ csprintf("Illegal CSR index %#x\n", csr), machInst);
+ }
+ if (bits(csr, 11, 10) == 0x3) {
+ return std::make_shared<IllegalInstFault>(
+ csprintf("CSR %s is read-only\n", csrName), machInst);
+ }
+
+ %(op_decl)s;
+ %(op_rd)s;
+
+ switch (csr) {
+ case CSR_SATP: {
+ auto pm = (PrivilegeMode)xc->readMiscReg(MISCREG_PRV);
+ STATUS status = xc->readMiscReg(MISCREG_STATUS);
+ if (pm == PRV_U || (pm == PRV_S && status.tvm == 1)) {
+ return std::make_shared<IllegalInstFault>(
+ "SATP access in user mode or with TVM enabled\n",
+ machInst);
+ }
+ break;
+ }
+ case CSR_MSTATUS: {
+ auto pm = (PrivilegeMode)xc->readMiscReg(MISCREG_PRV);
+ if (pm != PrivilegeMode::PRV_M) {
+ return std::make_shared<IllegalInstFault>(
+ "MSTATUS is only accessibly in machine mode\n",
+ machInst);
+ }
+ break;
+ }
+ default:
+ break;
+ }
+
+ RegVal data;
+ if (csr == CSR_FCSR) {
+ data = xc->readMiscReg(MISCREG_FFLAGS) |
(xc->readMiscReg(MISCREG_FRM) << FRM_OFFSET);
} else {
- olddata = xc->readMiscReg(midx);
+ data = xc->readMiscReg(midx);
}
- auto olddata_all = olddata;
- olddata &= maskVal;
- DPRINTF(RiscvMisc, "Reading CSR %s: %#x\n", csrName, olddata);
- data = olddata;
+ RegVal original = data;
+
+ DPRINTF(RiscvMisc, "Reading CSR %s: %#x\n", csrName, data &
maskVal);
%(code)s;
- data &= maskVal;
- if (data != olddata) {
- if (bits(csr, 11, 10) == 0x3) {
- return std::make_shared<IllegalInstFault>(
- csprintf("CSR %s is read-only\n", csrName),
machInst);
- }
- auto newdata_all = data;
- // We must keep those original bits not in mask.
- // olddata and data only contain the bits visable
- // in current privilige level.
- newdata_all = (olddata_all & ~maskVal) | data;
- DPRINTF(RiscvMisc, "Writing %#x to CSR %s.\n",
- newdata_all, csrName);
- 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:
- 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(midx, newdata_all);
- } else {
- return std::make_shared<IllegalInstFault>(
- "Only bits in mask are allowed to be set\n",
- machInst);
- }
- break;
- default:
- xc->setMiscReg(midx, data);
- break;
- }
+ // We must keep those original bits not in the mask. Hidden bits
should
+ // keep their original value.
+ data = (original & ~maskVal) | (data & maskVal);
+
+ DPRINTF(RiscvMisc, "Writing %#x to CSR %s.\n", data, csrName);
+
+ if (csr == CSR_FCSR) {
+ xc->setMiscReg(MISCREG_FFLAGS, bits(data, 4, 0));
+ xc->setMiscReg(MISCREG_FRM, bits(data, 7, 5));
+ } else {
+ xc->setMiscReg(midx, data);
}
+
%(op_wb)s;
return NoFault;
}
@@ -467,10 +501,24 @@
exec_output = BasicExecute.subst(iop)
}};
+def template CSRDecode {{
+ if (RS1)
+ return new %(class_name)sRw(machInst);
+ else
+ return new %(class_name)sRo(machInst);
+}};
+
def format CSROp(code, *opt_flags) {{
- iop = InstObjParams(name, Name, 'CSROp', code, opt_flags)
+ iop = InstObjParams(name, Name + "Ro", 'CSROp', code, opt_flags)
header_output = BasicDeclare.subst(iop)
decoder_output = BasicConstructor.subst(iop)
- decode_block = BasicDecode.subst(iop)
- exec_output = CSRExecute.subst(iop)
+ exec_output = CSRExecuteRo.subst(iop)
+
+ iop = InstObjParams(name, Name + "Rw", 'CSROp', code, opt_flags)
+ header_output += BasicDeclare.subst(iop)
+ decoder_output += BasicConstructor.subst(iop)
+ exec_output += CSRExecuteRw.subst(iop)
+
+ iop = InstObjParams(name, Name, 'CSROp', "", opt_flags)
+ decode_block = CSRDecode.subst(iop)
}};
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/55223
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: I5247941d4689d552aa73170c45f472b7d72ed88d
Gerrit-Change-Number: 55223
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <[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
