Alec Roelke has submitted this change and it was merged. (
https://gem5-review.googlesource.com/2340 )
Change subject: riscv: fix Linux problems with LR and SC ops
......................................................................
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 <ar...@virginia.edu>
Reviewed-by: Jason Lowe-Power <ja...@lowepower.com>
---
M src/arch/riscv/SConscript
M src/arch/riscv/isa/decoder.isa
M src/arch/riscv/isa/formats/amo.isa
M src/arch/riscv/isa/formats/mem.isa
A src/arch/riscv/locked_mem.cc
M src/arch/riscv/locked_mem.hh
M tests/test-progs/insttest/src/riscv/rv64a.cpp
7 files changed, 325 insertions(+), 100 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Alec Roelke: Looks good to me, approved
diff --git a/src/arch/riscv/SConscript b/src/arch/riscv/SConscript
index dcb670a..5aaac6b 100644
--- a/src/arch/riscv/SConscript
+++ b/src/arch/riscv/SConscript
@@ -50,6 +50,7 @@
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 eac0652..2b23c1f 100644
--- a/src/arch/riscv/isa/decoder.isa
+++ b/src/arch/riscv/isa/decoder.isa
@@ -170,12 +170,12 @@
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 @@
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 b837cc9..d60c4e0 100644
--- 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 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]) << ')';
@@ -83,6 +128,22 @@
ss << csprintf("0x%08x", machInst) << ' ' << mnemonic;
return ss.str();
}
+}};
+
+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 {{
@@ -129,6 +190,18 @@
};
}};
+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)
@@ -167,6 +240,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 {{
@@ -224,6 +358,27 @@
}
}};
+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
@@ -258,6 +413,55 @@
if (fault == NoFault) {
%(op_wb)s;
xc->setEA(EA);
+ }
+
+ return fault;
+ }
+}};
+
+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;
@@ -311,6 +515,54 @@
}
}};
+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 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 69a72df..2a00850 100644
--- a/src/arch/riscv/isa/formats/mem.isa
+++ b/src/arch/riscv/isa/formats/mem.isa
@@ -186,10 +186,6 @@
# 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 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) = \
@@ -416,26 +352,4 @@
(header_output, decoder_output, decode_block, exec_output) = \
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
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 4a80920..d7fc0ca 100644
--- a/src/arch/riscv/locked_mem.hh
+++ b/src/arch/riscv/locked_mem.hh
@@ -67,7 +67,7 @@
// 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 55150fb..a6e226c 100644
--- a/tests/test-progs/insttest/src/riscv/rv64a.cpp
+++ b/tests/test-progs/insttest/src/riscv/rv64a.cpp
@@ -40,12 +40,16 @@
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 @@
"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;
--
To view, visit https://gem5-review.googlesource.com/2340
To unsubscribe, visit https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If27696323dd6229a0277818e3744fbdf7180fca7
Gerrit-Change-Number: 2340
Gerrit-PatchSet: 8
Gerrit-Owner: Alec Roelke <ar...@virginia.edu>
Gerrit-Reviewer: Alec Roelke <ar...@virginia.edu>
Gerrit-Reviewer: Jason Lowe-Power <ja...@lowepower.com>
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev
