arch-riscv: Fix CSR instruction behavior 2nd attempts (#1099)

Quote from change[1] > The RISC-V spec clarifies the CSR instruction operation, some of them shall not read or write CSR by the hints of RD/RS1/uimm, but the original version use the 'data != oldData' condition to determine whether write or not, and always read CSR first. See CSR instruction in spec: Section 9.1 Page 56 of https://github.com/riscv/riscv-isa-manual/releases/download/Ratified-IMAFDQC/riscv-spec-20191213.pdf |||Register operand||| |--- |--- |--- |--- |--- | |Instruction|rd is x0|rs1 is x0|Reads CSR|Writes CSR| |CSRRW|Yes|-|No|Yes| |CSRRW|No|-|Yes|Yes| |CSRRS/CSRRC|-|Yes|Yes|No| |CSRRS/CSRRC|-|No|Yes|Yes| |||Immediate operand||| |Instruction|rd is x0|uimm = 0|Reads CSR|Writes CSR| |CSRRWI|Yes|-|No|Yes| |CSRRWI|No|-|Yes|Yes| |CSRRSI/CSRRCI|-|Yes|Yes|No| |CSRRSI/CSRRCI|-|No|Yes|Yes| The issue cause the ubuntu hanging because we shared the same status CSR with `mstatus`, `sstatus` and `ustatus` and interrupt enabling CSR with mip, sip and uip. We may need to read origin CSR without effect of unmask bits to avoid override the bits of other CSR. Now the ubuntu can work after the patch merged. [1] https://gem5-review.googlesource.com/c/public/gem5/+/67717
2024-05-10 10:21:48 -07:00
parent e3c2a322a1 c1713a0b18
commit 53245fa0e8
4 changed files with 136 additions and 53 deletions
--- a/src/arch/riscv/isa.cc
+++ b/src/arch/riscv/isa.cc
@@ -482,12 +482,28 @@ ISA::readMiscReg(RegIndex idx)
                    tc->getCpuPtr()->getInterruptController(tc->threadId()));
            return ic->readIP();
        }
+      case MISCREG_UIP:
+        {
+            return readMiscReg(MISCREG_IP) & UI_MASK[getPrivilegeModeSet()];
+        }
+      case MISCREG_SIP:
+        {
+            return readMiscReg(MISCREG_IP) & SI_MASK[getPrivilegeModeSet()];
+        }
      case MISCREG_IE:
        {
            auto ic = dynamic_cast<RiscvISA::Interrupts *>(
                    tc->getCpuPtr()->getInterruptController(tc->threadId()));
            return ic->readIE();
        }
+      case MISCREG_UIE:
+        {
+            return readMiscReg(MISCREG_IE) & UI_MASK[getPrivilegeModeSet()];
+        }
+      case MISCREG_SIE:
+        {
+            return readMiscReg(MISCREG_IE) & SI_MASK[getPrivilegeModeSet()];
+        }
      case MISCREG_SEPC:
      case MISCREG_MEPC:
        {
@@ -551,6 +567,16 @@ ISA::readMiscReg(RegIndex idx)

            return readMiscRegNoEffect(idx);
        }
+      case MISCREG_USTATUS:
+        {
+           return readMiscReg(MISCREG_STATUS) &
+                  USTATUS_MASKS[rvType()][getPrivilegeModeSet()];
+        }
+      case MISCREG_SSTATUS:
+        {
+           return readMiscReg(MISCREG_STATUS) &
+                  SSTATUS_MASKS[rvType()][getPrivilegeModeSet()];
+        }
      case MISCREG_VLENB:
        {
            auto rpc = tc->pcState().as<PCState>();
@@ -576,6 +602,11 @@ ISA::readMiscReg(RegIndex idx)
        {
            return readMiscRegNoEffect(MISCREG_FFLAGS) & FFLAGS_MASK;
        }
+      case MISCREG_FCSR:
+        {
+            return readMiscRegNoEffect(MISCREG_FFLAGS) |
+                  (readMiscRegNoEffect(MISCREG_FRM) << FRM_OFFSET);
+        }
      default:
        // Try reading HPM counters
        // As a placeholder, all HPM counters are just cycle counters
@@ -693,18 +724,48 @@ ISA::setMiscReg(RegIndex idx, RegVal val)

          case MISCREG_IP:
            {
+                val = val & MI_MASK[getPrivilegeModeSet()];
                auto ic = dynamic_cast<RiscvISA::Interrupts *>(
                    tc->getCpuPtr()->getInterruptController(tc->threadId()));
                ic->setIP(val);
            }
            break;
+          case MISCREG_UIP:
+            {
+                RegVal mask = UI_MASK[getPrivilegeModeSet()];
+                val = (val & mask) | (readMiscReg(MISCREG_IP) & ~mask);
+                setMiscReg(MISCREG_IP, val);
+            }
+            break;
+          case MISCREG_SIP:
+            {
+                RegVal mask = SI_MASK[getPrivilegeModeSet()];
+                val = (val & mask) | (readMiscReg(MISCREG_IP) & ~mask);
+                setMiscReg(MISCREG_IP, val);
+            }
+            break;
          case MISCREG_IE:
            {
+                val = val & MI_MASK[getPrivilegeModeSet()];
                auto ic = dynamic_cast<RiscvISA::Interrupts *>(
                    tc->getCpuPtr()->getInterruptController(tc->threadId()));
                ic->setIE(val);
            }
            break;
+          case MISCREG_UIE:
+            {
+                RegVal mask = UI_MASK[getPrivilegeModeSet()];
+                val = (val & mask) | (readMiscReg(MISCREG_IE) & ~mask);
+                setMiscReg(MISCREG_IE, val);
+            }
+            break;
+          case MISCREG_SIE:
+            {
+                RegVal mask = SI_MASK[getPrivilegeModeSet()];
+                val = (val & mask) | (readMiscReg(MISCREG_IE) & ~mask);
+                setMiscReg(MISCREG_IE, val);
+            }
+            break;
          case MISCREG_SATP:
            {
                // we only support bare and Sv39 mode; setting a different mode
@@ -746,6 +807,7 @@ ISA::setMiscReg(RegIndex idx, RegVal val)
            break;
          case MISCREG_STATUS:
            {
+                val = val & MSTATUS_MASKS[rvType()][getPrivilegeModeSet()];
                if (_rvType != RV32) {
                    // SXL and UXL are hard-wired to 64 bit
                    auto cur = readMiscRegNoEffect(idx);
@@ -759,6 +821,22 @@ ISA::setMiscReg(RegIndex idx, RegVal val)
                setMiscRegNoEffect(idx, val);
            }
            break;
+          case MISCREG_USTATUS:
+            {
+                RegVal mask = USTATUS_MASKS[rvType()][getPrivilegeModeSet()];
+                val = (val & mask) |
+                      (readMiscRegNoEffect(MISCREG_STATUS) & ~mask);
+                setMiscReg(MISCREG_STATUS, val);
+            }
+            break;
+          case MISCREG_SSTATUS:
+            {
+                RegVal mask = SSTATUS_MASKS[rvType()][getPrivilegeModeSet()];
+                val = (val & mask) |
+                      (readMiscRegNoEffect(MISCREG_STATUS) & ~mask);
+                setMiscReg(MISCREG_STATUS, val);
+            }
+            break;
          case MISCREG_VXSAT:
            {
                setMiscRegNoEffect(idx, val & 0x1);
@@ -782,6 +860,22 @@ ISA::setMiscReg(RegIndex idx, RegVal val)
                setMiscRegNoEffect(MISCREG_FFLAGS, new_val);
            }
            break;
+          case MISCREG_FFLAGS:
+            {
+                setMiscRegNoEffect(MISCREG_FFLAGS, val & FFLAGS_MASK);
+            }
+            break;
+          case MISCREG_FRM:
+            {
+                setMiscRegNoEffect(MISCREG_FRM, val & FRM_MASK);
+            }
+            break;
+          case MISCREG_FCSR:
+            {
+                setMiscRegNoEffect(MISCREG_FFLAGS, bits(val, 4, 0));
+                setMiscRegNoEffect(MISCREG_FRM, bits(val, 7, 5));
+            }
+            break;
          default:
            setMiscRegNoEffect(idx, val);
        }
--- a/src/arch/riscv/isa/decoder.isa
+++ b/src/arch/riscv/isa/decoder.isa
@@ -5012,27 +5012,33 @@ decode QUADRANT default Unknown::unknown() {
                0x1: csrrw({{
                    Rd = rvSext(data);
                    data = rvZext(Rs1);
-                }}, IsSerializeAfter, IsNonSpeculative, No_OpClass);
+                }}, 'RD != 0', 'true'
+                  , IsSerializeAfter, IsNonSpeculative, No_OpClass);
                0x2: csrrs({{
                    Rd = rvSext(data);
                    data = rvZext(data | Rs1);
-                }}, IsSerializeAfter, IsNonSpeculative, No_OpClass);
+                }}, 'true', 'RS1 != 0'
+                  , IsSerializeAfter, IsNonSpeculative, No_OpClass);
                0x3: csrrc({{
                    Rd = rvSext(data);
                    data = rvZext(data & ~Rs1);
-                }}, IsSerializeAfter, IsNonSpeculative, No_OpClass);
+                }}, 'true', 'RS1 != 0'
+                  , IsSerializeAfter, IsNonSpeculative, No_OpClass);
                0x5: csrrwi({{
                    Rd = rvSext(data);
                    data = rvZext(uimm);
-                }}, IsSerializeAfter, IsNonSpeculative, No_OpClass);
+                }}, 'RD != 0', 'true'
+                  , IsSerializeAfter, IsNonSpeculative, No_OpClass);
                0x6: csrrsi({{
                    Rd = rvSext(data);
                    data = rvZext(data | uimm);
-                }}, IsSerializeAfter, IsNonSpeculative, No_OpClass);
+                }}, 'true', 'uimm != 0'
+                  , IsSerializeAfter, IsNonSpeculative, No_OpClass);
                0x7: csrrci({{
                    Rd = rvSext(data);
                    data = rvZext(data & ~uimm);
-                }}, IsSerializeAfter, IsNonSpeculative, No_OpClass);
+                }}, 'true', 'uimm != 0'
+                  , IsSerializeAfter, IsNonSpeculative, No_OpClass);
            }
        }

--- a/src/arch/riscv/isa/formats/standard.isa
+++ b/src/arch/riscv/isa/formats/standard.isa
@@ -333,7 +333,6 @@ def template CSRExecute {{
        // We assume a riscv instruction is always run with a riscv ISA.
        auto isa = static_cast<RiscvISA::ISA*>(xc->tcBase()->getIsaPtr());
        auto& csr_data = isa->getCSRDataMap();
-        auto& csr_masks = isa->getCSRMaskMap();
        MISA misa = isa->readMiscRegNoEffect(MISCREG_ISA);

        auto csr_data_it = csr_data.find(csr);
@@ -360,14 +359,10 @@ def template CSRExecute {{
                    machInst);
        }

-        auto mask_it = csr_masks.find(csr);
-        RegVal maskVal = (mask_it == csr_masks.end()) ? mask(64)
-                                                      : mask_it->second;
-
        %(op_decl)s;
        %(op_rd)s;

-        RegVal data, olddata;
+        RegVal data = 0;
        auto lowestAllowedMode = (PrivilegeMode)bits(csr, 9, 8);
        auto pm = (PrivilegeMode)xc->readMiscReg(MISCREG_PRV);
        if (pm < lowestAllowedMode) {
@@ -389,43 +384,20 @@ def template CSRExecute {{
            break;
        }

-        if (csr == CSR_FCSR) {
-            olddata = xc->readMiscReg(MISCREG_FFLAGS) |
-                      (xc->readMiscReg(MISCREG_FRM) << FRM_OFFSET);
-        } else {
-            olddata = xc->readMiscReg(midx);
+        if (%(read_cond)s) {
+            data = rvZext(xc->readMiscReg(midx));
        }
-        olddata = rvZext(olddata);
-        auto olddata_all = olddata;
-
-        olddata &= maskVal;
-        DPRINTF(RiscvMisc, "Reading CSR %s: %#x\n", csrName, olddata);
-        data = olddata;

        %(code)s;

-        data &= maskVal;
-        if (data != olddata) {
+        if (%(write_cond)s) {
            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;
-              default:
-                xc->setMiscReg(midx, newdata_all);
-                break;
-            }
+                    data, csrName);
+            xc->setMiscReg(midx, data);
        }
        %(op_wb)s;
        return NoFault;
@@ -530,8 +502,10 @@ def format SystemOp(code, *opt_flags) {{
    exec_output = BasicExecute.subst(iop)
 }};

-def format CSROp(code, *opt_flags) {{
-    iop = InstObjParams(name, Name, 'CSROp', code, opt_flags)
+def format CSROp(code, read_cond, write_cond, *opt_flags) {{
+    iop = InstObjParams(name, Name, 'CSROp',
+        {'code': code, 'read_cond': read_cond,
+         'write_cond': write_cond}, opt_flags)
    header_output = BasicDeclare.subst(iop)
    decoder_output = BasicConstructor.subst(iop)
    decode_block = BasicDecode.subst(iop)
--- a/src/arch/riscv/regs/misc.hh
+++ b/src/arch/riscv/regs/misc.hh
@@ -248,8 +248,18 @@ enum MiscRegIndex

    NUM_PHYS_MISCREGS,

+    MISCREG_MSTATUS = MISCREG_STATUS,
+    MISCREG_MIP = MISCREG_IP,
+    MISCREG_MIE = MISCREG_IE,
    // This CSR shared the same space with MISCREG_FFLAGS
    MISCREG_FFLAGS_EXE = NUM_PHYS_MISCREGS,
+    MISCREG_FCSR,
+    MISCREG_USTATUS,
+    MISCREG_UIP,
+    MISCREG_UIE,
+    MISCREG_SSTATUS,
+    MISCREG_SIP,
+    MISCREG_SIE,

    NUM_MISCREGS
 };
@@ -525,10 +535,10 @@ constexpr uint64_t isaExtsFlags() {

 const std::unordered_map<int, CSRMetadata> CSRData = {
    {CSR_USTATUS,
-        {"ustatus", MISCREG_STATUS, rvTypeFlags(RV64, RV32),
+        {"ustatus", MISCREG_USTATUS, rvTypeFlags(RV64, RV32),
         isaExtsFlags('n')}},
    {CSR_UIE,
-        {"uie", MISCREG_IE, rvTypeFlags(RV64, RV32), isaExtsFlags('n')}},
+        {"uie", MISCREG_UIE, rvTypeFlags(RV64, RV32), isaExtsFlags('n')}},
    {CSR_UTVEC,
        {"utvec", MISCREG_UTVEC, rvTypeFlags(RV64, RV32), isaExtsFlags('n')}},
    {CSR_USCRATCH,
@@ -542,15 +552,14 @@ const std::unordered_map<int, CSRMetadata> CSRData = {
    {CSR_UTVAL,
        {"utval", MISCREG_UTVAL, rvTypeFlags(RV64, RV32), isaExtsFlags('n')}},
    {CSR_UIP,
-        {"uip", MISCREG_IP, rvTypeFlags(RV64, RV32), isaExtsFlags('n')}},
+        {"uip", MISCREG_UIP, rvTypeFlags(RV64, RV32), isaExtsFlags('n')}},
    {CSR_FFLAGS,
        {"fflags", MISCREG_FFLAGS, rvTypeFlags(RV64, RV32),
         isaExtsFlags('f')}},
    {CSR_FRM,
        {"frm", MISCREG_FRM, rvTypeFlags(RV64, RV32), isaExtsFlags('f')}},
-     // Actually FRM << 5 | FFLAGS
    {CSR_FCSR,
-        {"fcsr", MISCREG_FFLAGS, rvTypeFlags(RV64, RV32), isaExtsFlags('f')}},
+        {"fcsr", MISCREG_FCSR, rvTypeFlags(RV64, RV32), isaExtsFlags('f')}},
    {CSR_CYCLE,
        {"cycle", MISCREG_CYCLE, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
    {CSR_TIME,
@@ -739,7 +748,7 @@ const std::unordered_map<int, CSRMetadata> CSRData = {
         isaExtsFlags()}},

    {CSR_SSTATUS,
-        {"sstatus", MISCREG_STATUS, rvTypeFlags(RV64, RV32),
+        {"sstatus", MISCREG_SSTATUS, rvTypeFlags(RV64, RV32),
         isaExtsFlags('s')}},
    {CSR_SEDELEG,
        {"sedeleg", MISCREG_SEDELEG, rvTypeFlags(RV64, RV32),
@@ -748,7 +757,7 @@ const std::unordered_map<int, CSRMetadata> CSRData = {
        {"sideleg", MISCREG_SIDELEG, rvTypeFlags(RV64, RV32),
         isaExtsFlags('s')}},
    {CSR_SIE,
-        {"sie", MISCREG_IE, rvTypeFlags(RV64, RV32), isaExtsFlags('s')}},
+        {"sie", MISCREG_SIE, rvTypeFlags(RV64, RV32), isaExtsFlags('s')}},
    {CSR_STVEC,
        {"stvec", MISCREG_STVEC, rvTypeFlags(RV64, RV32), isaExtsFlags('s')}},
    {CSR_SCOUNTEREN,
@@ -765,7 +774,7 @@ const std::unordered_map<int, CSRMetadata> CSRData = {
    {CSR_STVAL,
        {"stval", MISCREG_STVAL, rvTypeFlags(RV64, RV32), isaExtsFlags('s')}},
    {CSR_SIP,
-        {"sip", MISCREG_IP, rvTypeFlags(RV64, RV32), isaExtsFlags('s')}},
+        {"sip", MISCREG_SIP, rvTypeFlags(RV64, RV32), isaExtsFlags('s')}},
    {CSR_SATP,
        {"satp", MISCREG_SATP, rvTypeFlags(RV64, RV32), isaExtsFlags('s')}},

@@ -779,7 +788,7 @@ const std::unordered_map<int, CSRMetadata> CSRData = {
    {CSR_MHARTID,
        {"mhartid", MISCREG_HARTID, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
    {CSR_MSTATUS,
-        {"mstatus", MISCREG_STATUS, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
+        {"mstatus", MISCREG_MSTATUS, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
    {CSR_MISA,
        {"misa", MISCREG_ISA, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
    {CSR_MEDELEG,
@@ -787,7 +796,7 @@ const std::unordered_map<int, CSRMetadata> CSRData = {
    {CSR_MIDELEG,
        {"mideleg", MISCREG_MIDELEG, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
    {CSR_MIE,
-        {"mie", MISCREG_IE, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
+        {"mie", MISCREG_MIE, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
    {CSR_MTVEC,
        {"mtvec", MISCREG_MTVEC, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
    {CSR_MCOUNTEREN,
@@ -805,7 +814,7 @@ const std::unordered_map<int, CSRMetadata> CSRData = {
    {CSR_MTVAL,
        {"mtval", MISCREG_MTVAL, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
    {CSR_MIP,
-        {"mip", MISCREG_IP, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
+        {"mip", MISCREG_MIP, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
    {CSR_PMPCFG0,
        {"pmpcfg0", MISCREG_PMPCFG0, rvTypeFlags(RV64, RV32), isaExtsFlags()}},
    // pmpcfg1 rv32 only