From 89c49d1ab06ea5364ab1f80586f8b01c0297cb12 Mon Sep 17 00:00:00 2001
From: zhongchengyong <zhongcy93@gmail.com>
Date: Tue, 7 Feb 2023 22:21:53 +0800
Subject: [PATCH] arch-riscv: Fix the CSR instruction behavior.

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

Change-Id: I5e7a43cf639474ae76c19a1f430d314b4634ce62
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/67717
Reviewed-by: Hoa Nguyen <hoanguyen@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
---
 src/arch/riscv/insts/standard.hh        | 19 +++++++++++++++++--
 src/arch/riscv/isa/formats/standard.isa | 12 +++++++-----
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/src/arch/riscv/insts/standard.hh b/src/arch/riscv/insts/standard.hh
index 5b0e8c2c22..afcfd7a915 100644
--- a/src/arch/riscv/insts/standard.hh
+++ b/src/arch/riscv/insts/standard.hh
@@ -91,18 +91,33 @@ class CSROp : public RiscvStaticInst
   protected:
     uint64_t csr;
     uint64_t uimm;
+    bool read;
+    bool write;
 
     /// Constructor
     CSROp(const char *mnem, ExtMachInst _machInst, OpClass __opClass)
         : RiscvStaticInst(mnem, _machInst, __opClass),
-            csr(FUNCT12), uimm(CSRIMM)
+            csr(FUNCT12), uimm(CSRIMM), read(true), write(true)
     {
         if (csr == CSR_SATP) {
             flags[IsSquashAfter] = true;
         }
+        if (strcmp(mnemonic, "csrrw") == 0 ||
+            strcmp(mnemonic, "csrrwi") == 0) {
+          if (RD == 0){
+            read = false;
+          }
+        } else if (strcmp(mnemonic, "csrrs") == 0 ||
+                   strcmp(mnemonic, "csrrc") == 0 ||
+                   strcmp(mnemonic, "csrrsi") == 0 ||
+                   strcmp(mnemonic, "csrrci") == 0 ){
+          if (RS1 == 0) {
+            write = false;
+          }
+        }
     }
 
-    std::string generateDisassembly(
+  std::string generateDisassembly(
         Addr pc, const loader::SymbolTable *symtab) const override;
 };
 
diff --git a/src/arch/riscv/isa/formats/standard.isa b/src/arch/riscv/isa/formats/standard.isa
index bb500f5f49..1bd431ac4d 100644
--- a/src/arch/riscv/isa/formats/standard.isa
+++ b/src/arch/riscv/isa/formats/standard.isa
@@ -358,7 +358,7 @@ def template CSRExecute {{
         %(op_decl)s;
         %(op_rd)s;
 
-        RegVal data, olddata;
+        RegVal data = 0, olddata = 0;
         auto lowestAllowedMode = (PrivilegeMode)bits(csr, 9, 8);
         auto pm = (PrivilegeMode)xc->readMiscReg(MISCREG_PRV);
         if (pm < lowestAllowedMode) {
@@ -380,11 +380,13 @@ def template CSRExecute {{
             break;
         }
 
-        if (csr == CSR_FCSR) {
+        if (read) {
+          if (csr == CSR_FCSR) {
             olddata = xc->readMiscReg(MISCREG_FFLAGS) |
-                      (xc->readMiscReg(MISCREG_FRM) << FRM_OFFSET);
-        } else {
+              (xc->readMiscReg(MISCREG_FRM) << FRM_OFFSET);
+          } else {
             olddata = xc->readMiscReg(midx);
+          }
         }
         olddata = rvZext(olddata);
         auto olddata_all = olddata;
@@ -396,7 +398,7 @@ def template CSRExecute {{
         %(code)s;
 
         data &= maskVal;
-        if (data != olddata) {
+        if (write) {
             if (bits(csr, 11, 10) == 0x3) {
                 return std::make_shared<IllegalInstFault>(
                         csprintf("CSR %s is read-only\n", csrName), machInst);