riscv: fix Linux problems with LR and SC ops

Some of the functions in the Linux toolchain that allocate memory make
use of paired LR and SC instructions, which didn't work properly for
that toolchain.  This patch fixes that so attempting to use those
functions doesn't cause an endless loop of failed SC instructions.

Change-Id: If27696323dd6229a0277818e3744fbdf7180fca7
Reviewed-on: https://gem5-review.googlesource.com/2340
Maintainer: Alec Roelke <ar4jc@virginia.edu>
Reviewed-by: Jason Lowe-Power <jason@lowepower.com>

diff --git a/src/arch/riscv/SConscript b/src/arch/riscv/SConscript
index dcb670a6789d458723ff59edffbf930775cbeadc..5aaac6be4a78e2b9dec829fe0700280ece0a17b0 100644 (file)
--- a/src/arch/riscv/SConscript
+++ b/src/arch/riscv/SConscript
@@ -50,6 +50,7 @@ if env['TARGET_ISA'] == 'riscv':
     Source('faults.cc')
     Source('isa.cc')
     Source('interrupts.cc')
+    Source('locked_mem.cc')
     Source('process.cc')
     Source('pagetable.cc')
     Source('remote_gdb.cc')

diff --git a/src/arch/riscv/isa/decoder.isa b/src/arch/riscv/isa/decoder.isa
index eac0652c01ae1c7f6335fddee22035f46e2dae73..2b23c1fe4e809220f07f19a90540947b5a55dbd9 100644 (file)
--- a/src/arch/riscv/isa/decoder.isa
+++ b/src/arch/riscv/isa/decoder.isa
@@ -170,12 +170,12 @@ decode OPCODE default Unknown::unknown() {
         0x2: decode AMOFUNCT {
             0x2: LoadReserved::lr_w({{
                 Rd_sd = Mem_sw;
-            }}, mem_flags=LLSC, aq=AQ, rl=RL);
+            }}, mem_flags=LLSC);
             0x3: StoreCond::sc_w({{
                 Mem_uw = Rs2_uw;
             }}, {{
                 Rd = result;
-            }}, inst_flags=IsStoreConditional, mem_flags=LLSC, aq=AQ, rl=RL);
+            }}, inst_flags=IsStoreConditional, mem_flags=LLSC);
             format AtomicMemOp {
                 0x0: amoadd_w({{Rt_sd = Mem_sw;}}, {{
                     Mem_sw = Rs2_sw + Rt_sd;
@@ -218,12 +218,12 @@ decode OPCODE default Unknown::unknown() {
         0x3: decode AMOFUNCT {
             0x2: LoadReserved::lr_d({{
                 Rd_sd = Mem_sd;
-            }}, mem_flags=LLSC, aq=AQ, rl=RL);
+            }}, mem_flags=LLSC);
             0x3: StoreCond::sc_d({{
                 Mem = Rs2;
             }}, {{
                 Rd = result;
-            }}, mem_flags=LLSC, inst_flags=IsStoreConditional, aq=AQ, rl=RL);
+            }}, mem_flags=LLSC, inst_flags=IsStoreConditional);
             format AtomicMemOp {
                 0x0: amoadd_d({{Rt_sd = Mem_sd;}}, {{
                     Mem_sd = Rs2_sd + Rt_sd;

diff --git a/src/arch/riscv/isa/formats/amo.isa b/src/arch/riscv/isa/formats/amo.isa
index b837cc9c3266bada32ef7376e257875b87080ea8..d60c4e0cdbc10840c8cb6383bdc828f98255829e 100644 (file)
--- a/src/arch/riscv/isa/formats/amo.isa
+++ b/src/arch/riscv/isa/formats/amo.isa
@@ -34,6 +34,33 @@
 // Atomic memory operation instructions
 //
 output header {{
+    class LoadReserved : public RiscvStaticInst
+    {
+      protected:
+        Request::Flags memAccessFlags;
+
+        LoadReserved(const char *mnem, ExtMachInst _machInst,
+            OpClass __opClass)
+                : RiscvStaticInst(mnem, _machInst, __opClass)
+        {}
+
+        std::string
+        generateDisassembly(Addr pc, const SymbolTable *symtab) const;
+    };
+
+    class StoreCond : public RiscvStaticInst
+    {
+      protected:
+        Request::Flags memAccessFlags;
+
+        StoreCond(const char* mnem, ExtMachInst _machInst, OpClass __opClass)
+                : RiscvStaticInst(mnem, _machInst, __opClass)
+        {}
+
+        std::string
+        generateDisassembly(Addr pc, const SymbolTable *symtab) const;
+    };
+
     class AtomicMemOp : public RiscvMacroInst
     {
     protected:
@@ -65,11 +92,29 @@ output header {{
 }};
 
 output decoder {{
+    std::string LoadReserved::generateDisassembly(Addr pc,
+        const SymbolTable *symtab) const
+    {
+        std::stringstream ss;
+        ss << mnemonic << ' ' << regName(_destRegIdx[0]) << ", ("
+                << regName(_srcRegIdx[0]) << ')';
+        return ss.str();
+    }
+
+    std::string StoreCond::generateDisassembly(Addr pc,
+        const SymbolTable *symtab) const
+    {
+        std::stringstream ss;
+        ss << mnemonic << ' ' << regName(_destRegIdx[0]) << ", "
+                << regName(_srcRegIdx[1]) << ", ("
+                << regName(_srcRegIdx[0]) << ')';
+        return ss.str();
+    }
+
     std::string AtomicMemOp::generateDisassembly(Addr pc,
         const SymbolTable *symtab) const
     {
         std::stringstream ss;
-        ss << csprintf("0x%08x", machInst) << ' ';
         ss << mnemonic << ' ' << regName(_destRegIdx[0]) << ", "
                 << regName(_srcRegIdx[1]) << ", ("
                 << regName(_srcRegIdx[0]) << ')';
@@ -85,6 +130,22 @@ output decoder {{
     }
 }};
 
+def template LRSCDeclare {{
+    class %(class_name)s : public %(base_class)s
+    {
+      public:
+        %(class_name)s(ExtMachInst machInst);
+
+        %(BasicExecDeclare)s
+
+        %(EACompDeclare)s
+
+        %(InitiateAccDeclare)s
+
+        %(CompleteAccDeclare)s
+    };
+}};
+
 def template AtomicMemOpDeclare {{
     /**
      * Static instruction class for an AtomicMemOp operation
@@ -129,6 +190,18 @@ def template AtomicMemOpDeclare {{
     };
 }};
 
+def template LRSCConstructor {{
+    %(class_name)s::%(class_name)s(ExtMachInst machInst):
+        %(base_class)s("%(mnemonic)s", machInst, %(op_class)s)
+    {
+        %(constructor)s;
+        if (AQ)
+            memAccessFlags = memAccessFlags | Request::ACQUIRE;
+        if (RL)
+            memAccessFlags = memAccessFlags | Request::RELEASE;
+    }
+}};
+
 def template AtomicMemOpMacroConstructor {{
     %(class_name)s::%(class_name)s(ExtMachInst machInst)
             : %(base_class)s("%(mnemonic)s", machInst, %(op_class)s)
@@ -169,6 +242,67 @@ def template AtomicMemOpMacroDecode {{
     return new %(class_name)s(machInst);
 }};
 
+def template LoadReservedExecute {{
+    Fault
+    %(class_name)s::execute(CPU_EXEC_CONTEXT *xc,
+        Trace::InstRecord *traceData) const
+    {
+        Addr EA;
+        Fault fault = NoFault;
+
+        %(op_decl)s;
+        %(op_rd)s;
+        %(ea_code)s;
+
+        if (fault == NoFault) {
+            fault = readMemAtomic(xc, traceData, EA, Mem, memAccessFlags);
+            %(memacc_code)s;
+        }
+
+        if (fault == NoFault) {
+            %(op_wb)s;
+        }
+
+        return fault;
+    }
+}};
+
+def template StoreCondExecute {{
+    Fault %(class_name)s::execute(CPU_EXEC_CONTEXT *xc,
+        Trace::InstRecord *traceData) const
+    {
+        Addr EA;
+        Fault fault = NoFault;
+        uint64_t result;
+
+        %(op_decl)s;
+        %(op_rd)s;
+        %(ea_code)s;
+
+        if (fault == NoFault) {
+            %(memacc_code)s;
+        }
+
+        if (fault == NoFault) {
+            fault = writeMemAtomic(xc, traceData, Mem, EA, memAccessFlags,
+                &result);
+            // RISC-V has the opposite convention gem5 has for success flags,
+            // so we invert the result here.
+            result = !result;
+        }
+
+        if (fault == NoFault) {
+            %(postacc_code)s;
+        }
+
+        if (fault == NoFault) {
+            %(op_wb)s;
+        }
+
+        return fault;
+    }
+}};
+
 def template AtomicMemOpLoadExecute {{
     Fault %(class_name)s::%(class_name)sLoad::execute(CPU_EXEC_CONTEXT *xc,
         Trace::InstRecord *traceData) const
@@ -224,6 +358,27 @@ def template AtomicMemOpStoreExecute {{
     }
 }};
 
+def template LRSCEACompExecute {{
+    Fault
+    %(class_name)s::eaComp(CPU_EXEC_CONTEXT *xc,
+        Trace::InstRecord *traceData) const
+    {
+        Addr EA;
+        Fault fault = NoFault;
+
+        %(op_decl)s;
+        %(op_rd)s;
+        %(ea_code)s;
+
+        if (fault == NoFault) {
+            %(op_wb)s;
+            xc->setEA(EA);
+        }
+
+        return fault;
+    }
+}};
+
 def template AtomicMemOpLoadEACompExecute {{
     Fault %(class_name)s::%(class_name)sLoad::eaComp(CPU_EXEC_CONTEXT *xc,
         Trace::InstRecord *traceData) const
@@ -264,6 +419,55 @@ def template AtomicMemOpStoreEACompExecute {{
     }
 }};
 
+def template LoadReservedInitiateAcc {{
+    Fault
+    %(class_name)s::initiateAcc(CPU_EXEC_CONTEXT *xc,
+        Trace::InstRecord *traceData) const
+    {
+        Addr EA;
+        Fault fault = NoFault;
+
+        %(op_src_decl)s;
+        %(op_rd)s;
+        %(ea_code)s;
+
+        if (fault == NoFault) {
+            fault = initiateMemRead(xc, traceData, EA, Mem, memAccessFlags);
+        }
+
+        return fault;
+    }
+}};
+
+def template StoreCondInitiateAcc {{
+    Fault
+    %(class_name)s::initiateAcc(CPU_EXEC_CONTEXT *xc,
+        Trace::InstRecord *traceData) const
+    {
+        Addr EA;
+        Fault fault = NoFault;
+
+        %(op_decl)s;
+        %(op_rd)s;
+        %(ea_code)s;
+
+        if (fault == NoFault) {
+            %(memacc_code)s;
+        }
+
+        if (fault == NoFault) {
+            fault = writeMemTiming(xc, traceData, Mem, EA,
+                memAccessFlags, nullptr);
+        }
+
+        if (fault == NoFault) {
+            %(op_wb)s;
+        }
+
+        return fault;
+    }
+}};
+
 def template AtomicMemOpLoadInitiateAcc {{
     Fault %(class_name)s::%(class_name)sLoad::initiateAcc(CPU_EXEC_CONTEXT *xc,
         Trace::InstRecord *traceData) const
@@ -311,6 +515,54 @@ def template AtomicMemOpStoreInitiateAcc {{
     }
 }};
 
+def template LoadReservedCompleteAcc {{
+    Fault
+    %(class_name)s::completeAcc(PacketPtr pkt, CPU_EXEC_CONTEXT *xc,
+        Trace::InstRecord *traceData) const
+    {
+        Fault fault = NoFault;
+
+        %(op_decl)s;
+        %(op_rd)s;
+
+        getMem(pkt, Mem, traceData);
+
+        if (fault == NoFault) {
+            %(memacc_code)s;
+        }
+
+        if (fault == NoFault) {
+            %(op_wb)s;
+        }
+
+        return fault;
+    }
+}};
+
+def template StoreCondCompleteAcc {{
+    Fault %(class_name)s::completeAcc(Packet *pkt, CPU_EXEC_CONTEXT *xc,
+        Trace::InstRecord *traceData) const
+    {
+        Fault fault = NoFault;
+
+        %(op_dest_decl)s;
+
+        // RISC-V has the opposite convention gem5 has for success flags,
+        // so we invert the result here.
+        uint64_t result = !pkt->req->getExtraData();
+
+        if (fault == NoFault) {
+            %(postacc_code)s;
+        }
+
+        if (fault == NoFault) {
+            %(op_wb)s;
+        }
+
+        return fault;
+    }
+}};
+
 def template AtomicMemOpLoadCompleteAcc {{
     Fault %(class_name)s::%(class_name)sLoad::completeAcc(PacketPtr pkt,
         CPU_EXEC_CONTEXT *xc, Trace::InstRecord *traceData) const
@@ -342,6 +594,44 @@ def template AtomicMemOpStoreCompleteAcc {{
     }
 }};
 
+def format LoadReserved(memacc_code, postacc_code={{ }}, ea_code={{EA = Rs1;}},
+        mem_flags=[], inst_flags=[]) {{
+    mem_flags = makeList(mem_flags)
+    inst_flags = makeList(inst_flags)
+    iop = InstObjParams(name, Name, 'LoadReserved',
+        {'ea_code': ea_code, 'memacc_code': memacc_code,
+        'postacc_code': postacc_code}, inst_flags)
+    iop.constructor += '\n\tmemAccessFlags = memAccessFlags | ' + \
+        '|'.join(['Request::%s' % flag for flag in mem_flags]) + ';'
+
+    header_output = LRSCDeclare.subst(iop)
+    decoder_output = LRSCConstructor.subst(iop)
+    decode_block = BasicDecode.subst(iop)
+    exec_output = LoadReservedExecute.subst(iop) \
+        + LRSCEACompExecute.subst(iop) \
+        + LoadReservedInitiateAcc.subst(iop) \
+        + LoadReservedCompleteAcc.subst(iop)
+}};
+
+def format StoreCond(memacc_code, postacc_code={{ }}, ea_code={{EA = Rs1;}},
+        mem_flags=[], inst_flags=[]) {{
+    mem_flags = makeList(mem_flags)
+    inst_flags = makeList(inst_flags)
+    iop = InstObjParams(name, Name, 'StoreCond',
+        {'ea_code': ea_code, 'memacc_code': memacc_code,
+        'postacc_code': postacc_code}, inst_flags)
+    iop.constructor += '\n\tmemAccessFlags = memAccessFlags | ' + \
+        '|'.join(['Request::%s' % flag for flag in mem_flags]) + ';'
+
+    header_output = LRSCDeclare.subst(iop)
+    decoder_output = LRSCConstructor.subst(iop)
+    decode_block = BasicDecode.subst(iop)
+    exec_output = StoreCondExecute.subst(iop) \
+        + LRSCEACompExecute.subst(iop) \
+        + StoreCondInitiateAcc.subst(iop) \
+        + StoreCondCompleteAcc.subst(iop)
+}};
+
 def format AtomicMemOp(load_code, store_code, ea_code, load_flags=[],
         store_flags=[], inst_flags=[]) {{
     macro_iop = InstObjParams(name, Name, 'AtomicMemOp', ea_code, inst_flags)

diff --git a/src/arch/riscv/isa/formats/mem.isa b/src/arch/riscv/isa/formats/mem.isa
index 69a72dfa814fd5df8ea5234f1a5918ebd1ba625a..2a00850a27cd1aca0db939813e2e3460d41dfe2e 100644 (file)
--- a/src/arch/riscv/isa/formats/mem.isa
+++ b/src/arch/riscv/isa/formats/mem.isa
@@ -186,10 +186,6 @@ def LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags, inst_flags,
 
     # select templates
 
-    # The InitiateAcc template is the same for StoreCond templates as the
-    # corresponding Store template..
-    StoreCondInitiateAcc = StoreInitiateAcc
-
     fullExecTemplate = eval(exec_template_base + 'Execute')
     initiateAccTemplate = eval(exec_template_base + 'InitiateAcc')
     completeAccTemplate = eval(exec_template_base + 'CompleteAcc')
@@ -344,66 +340,6 @@ def template StoreCompleteAcc {{
     }
 }};
 
-def template StoreCondExecute {{
-    Fault %(class_name)s::execute(CPU_EXEC_CONTEXT *xc,
-        Trace::InstRecord *traceData) const
-    {
-        Addr EA;
-        Fault fault = NoFault;
-        uint64_t result;
-
-        %(op_decl)s;
-        %(op_rd)s;
-        %(ea_code)s;
-
-        if (fault == NoFault) {
-            %(memacc_code)s;
-        }
-
-        if (fault == NoFault) {
-            fault = writeMemAtomic(xc, traceData, Mem, EA, memAccessFlags,
-                &result);
-            // RISC-V has the opposite convention gem5 has for success flags,
-            // so we invert the result here.
-            result = !result;
-        }
-
-        if (fault == NoFault) {
-            %(postacc_code)s;
-        }
-
-        if (fault == NoFault) {
-            %(op_wb)s;
-        }
-
-        return fault;
-    }
-}};
-
-def template StoreCondCompleteAcc {{
-    Fault %(class_name)s::completeAcc(Packet *pkt, CPU_EXEC_CONTEXT *xc,
-        Trace::InstRecord *traceData) const
-    {
-        Fault fault = NoFault;
-
-        %(op_dest_decl)s;
-
-        // RISC-V has the opposite convention gem5 has for success flags,
-        // so we invert the result here.
-        uint64_t result = !pkt->req->getExtraData();
-
-        if (fault == NoFault) {
-            %(postacc_code)s;
-        }
-
-        if (fault == NoFault) {
-            %(op_wb)s;
-        }
-
-        return fault;
-    }
-}};
-
 def format Load(memacc_code, ea_code = {{EA = Rs1 + ldisp;}}, mem_flags=[],
         inst_flags=[]) {{
     (header_output, decoder_output, decode_block, exec_output) = \
@@ -417,25 +353,3 @@ def format Store(memacc_code, ea_code={{EA = Rs1 + sdisp;}}, mem_flags=[],
         LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags, inst_flags,
         'Store', exec_template_base='Store')
 }};
-
-def format StoreCond(memacc_code, postacc_code, ea_code={{EA = Rs1;}},
-        mem_flags=[], inst_flags=[], aq=0, rl=0) {{
-    if aq:
-        mem_flags = makeList(mem_flags) + ["ACQUIRE"]
-    if rl:
-        mem_flags = makeList(mem_flags) + ["RELEASE"]
-    (header_output, decoder_output, decode_block, exec_output) = LoadStoreBase(
-            name, Name, ea_code, memacc_code, mem_flags, inst_flags, 'Store',
-            postacc_code, exec_template_base='StoreCond')
-}};
-
-def format LoadReserved(memacc_code, ea_code={{EA = Rs1;}}, mem_flags=[],
-        inst_flags=[], aq=0, rl=0) {{
-    if aq:
-        mem_flags = makeList(mem_flags) + ["ACQUIRE"]
-    if rl:
-        mem_flags = makeList(mem_flags) + ["RELEASE"]
-    (header_output, decoder_output, decode_block, exec_output) = LoadStoreBase(
-            name, Name, ea_code, memacc_code, mem_flags, inst_flags, 'Load',
-            exec_template_base='Load')
-}};

diff --git a/src/arch/riscv/locked_mem.cc b/src/arch/riscv/locked_mem.cc
new file mode 100644 (file)
index 0000000..3c8dbe9
--- /dev/null
+++ b/src/arch/riscv/locked_mem.cc
@@ -0,0 +1,12 @@
+#include "arch/riscv/locked_mem.hh"
+
+#include <stack>
+
+#include "base/types.hh"
+
+namespace RiscvISA
+{
+
+std::stack<Addr> locked_addrs;
+
+}

diff --git a/src/arch/riscv/locked_mem.hh b/src/arch/riscv/locked_mem.hh
index 4a809206cd29d320e9c9e8c99cd1ad7c1f2f8687..d7fc0ca5a86ec620ac2e25b0813c30211ac25f5f 100644 (file)
--- a/src/arch/riscv/locked_mem.hh
+++ b/src/arch/riscv/locked_mem.hh
@@ -67,7 +67,7 @@ const int WARN_FAILURE = 10000;
 
 // RISC-V allows multiple locks per hart, but each SC has to unlock the most
 // recent one, so we use a stack here.
-static std::stack<Addr> locked_addrs;
+extern std::stack<Addr> locked_addrs;
 
 template <class XC> inline void
 handleLockedSnoop(XC *xc, PacketPtr pkt, Addr cacheBlockMask)

diff --git a/tests/test-progs/insttest/src/riscv/rv64a.cpp b/tests/test-progs/insttest/src/riscv/rv64a.cpp
index 55150fbc3742f2c063d0896c14602b95c2af23c8..a6e226c04b9a85738fb2e7d5491e767c7e6c9823 100644 (file)
--- a/tests/test-progs/insttest/src/riscv/rv64a.cpp
+++ b/tests/test-progs/insttest/src/riscv/rv64a.cpp
@@ -40,12 +40,16 @@ int main()
     using namespace insttest;
 
     // Memory (LR.W, SC.W)
-    expect<pair<int64_t, uint64_t>>({-1, 0}, []{
+    expect<pair<int64_t, int64_t>>({-1, 256}, []{
             int32_t mem = -1;
             int64_t rs2 = 256;
-            int64_t rd = A::lr_w(mem);
-            pair<int64_t, uint64_t> result = A::sc_w(rs2, mem);
-            return pair<int64_t, uint64_t>(rd, result.second);
+            int64_t rd;
+            pair<int64_t, uint64_t> result;
+            do {
+                rd = A::lr_w(mem);
+                result = A::sc_w(rs2, mem);
+            } while (result.second == 1);
+            return pair<int64_t, uint64_t>(rd, result.first);
         }, "lr.w/sc.w");
     expect<pair<bool, int64_t>>({true, 200}, []{
             int32_t mem = 200;
@@ -130,12 +134,16 @@ int main()
             "amomaxu.w, sign extend");
 
     // Memory (LR.D, SC.D)
-    expect<pair<int64_t, uint64_t>>({-1, 0}, []{
+    expect<pair<int64_t, int64_t>>({-1, 256}, []{
             int64_t mem = -1;
             int64_t rs2 = 256;
-            int64_t rd = A::lr_d(mem);
-            pair<int64_t, uint64_t> result = A::sc_d(rs2, mem);
-            return pair<int64_t, uint64_t>(rd, result.second);
+            int64_t rd;
+            pair<int64_t, uint64_t> result;
+            do {
+                rd = A::lr_d(mem);
+                result = A::sc_d(rs2, mem);
+            } while (result.second == 1);
+            return pair<int64_t, uint64_t>(rd, result.first);
         }, "lr.d/sc.d");
     expect<pair<bool, int64_t>>({true, 200}, []{
             int64_t mem = 200;