Attention is currently required from: Daniel Carvalho, Bobby R. Bruce, Gabe
Black, Ayaz Akram.
Hello kokoro, Daniel Carvalho, Bobby R. Bruce, Gabe Black, Ayaz Akram,
I'd like you to do a code review.
Please visit
https://gem5-review.googlesource.com/c/public/gem5/+/48099
to review the following change.
Change subject: arch-riscv: Revert "Split up read/write and read only CSR
instructions."
......................................................................
arch-riscv: Revert "Split up read/write and read only CSR instructions."
This reverts commit 1cf41d4c54c988ef4808d8efc1f6212e54a4c120.
Reason for revert:
The above commit caused booting Linux using RISCV either to
hang or to take significantly time more than to finish.
For the v21-1 release, the above commit will be reverted.
Change-Id: I58fbe96d7ea50031eba40ff49dabdef971faf6ff
---
M src/arch/riscv/isa/formats/standard.isa
1 file changed, 46 insertions(+), 94 deletions(-)
diff --git a/src/arch/riscv/isa/formats/standard.isa
b/src/arch/riscv/isa/formats/standard.isa
index dba460f..e0b270b 100644
--- a/src/arch/riscv/isa/formats/standard.isa
+++ b/src/arch/riscv/isa/formats/standard.isa
@@ -274,7 +274,7 @@
}
}};
-def template CSRExecuteRo {{
+def template CSRExecute {{
Fault
%(class_name)s::execute(ExecContext *xc,
Trace::InstRecord *traceData) const
@@ -287,6 +287,8 @@
%(op_decl)s;
%(op_rd)s;
+ RegVal data, olddata;
+
switch (csr) {
case CSR_SATP: {
auto pm = (PrivilegeMode)xc->readMiscReg(MISCREG_PRV);
@@ -311,91 +313,55 @@
break;
}
- RegVal data;
if (csr == CSR_FCSR) {
- 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) |
+ olddata = xc->readMiscReg(MISCREG_FFLAGS) |
(xc->readMiscReg(MISCREG_FRM) << FRM_OFFSET);
} else {
- data = xc->readMiscReg(midx);
+ olddata = xc->readMiscReg(midx);
}
+ auto olddata_all = olddata;
- RegVal original = data;
-
- DPRINTF(RiscvMisc, "Reading CSR %s: %#x\n", csrName, data &
maskVal);
+ olddata &= maskVal;
+ DPRINTF(RiscvMisc, "Reading CSR %s: %#x\n", csrName, olddata);
+ data = olddata;
%(code)s;
- // 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);
+ 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;
+ }
}
-
%(op_wb)s;
return NoFault;
}
@@ -499,24 +465,10 @@
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 + "Ro", 'CSROp', code, opt_flags)
+ iop = InstObjParams(name, Name, 'CSROp', code, opt_flags)
header_output = BasicDeclare.subst(iop)
decoder_output = BasicConstructor.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)
+ decode_block = BasicDecode.subst(iop)
+ exec_output = CSRExecute.subst(iop)
}};
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48099
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: I58fbe96d7ea50031eba40ff49dabdef971faf6ff
Gerrit-Change-Number: 48099
Gerrit-PatchSet: 1
Gerrit-Owner: Hoa Nguyen <hoangu...@ucdavis.edu>
Gerrit-Reviewer: Ayaz Akram <yazak...@ucdavis.edu>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-Attention: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Attention: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Attention: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Attention: Ayaz Akram <yazak...@ucdavis.edu>
Gerrit-MessageType: newchange
_______________________________________________
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
