Bobby Bruce has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/71520?usp=email )
Change subject: arch-riscv: fix load reserved store conditional
......................................................................
arch-riscv: fix load reserved store conditional
* According to the manual, load reservations must be cleared on a
failed or a successful SC attempt.
* A load reservation can be arbitrarily large. The current
implementation was reserving something different than cacheBlockSize
which could lead to problems if snoop addresses are cache block
aligned. This patch implementation assumes a cacheBlock granularity.
* Load reservations should also be cleared on faults
Change-Id: I64513534710b5f269260fcb204f717801913e2f5
---
M src/arch/generic/isa.hh
M src/arch/riscv/faults.cc
M src/arch/riscv/isa.cc
M src/arch/riscv/isa.hh
4 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/src/arch/generic/isa.hh b/src/arch/generic/isa.hh
index e9e4d95..2e7e38d 100644
--- a/src/arch/generic/isa.hh
+++ b/src/arch/generic/isa.hh
@@ -70,6 +70,7 @@
public:
virtual PCStateBase *newPCState(Addr new_inst_addr=0) const = 0;
virtual void clear() {}
+ virtual void clearLoadReservation(ContextID cid) = 0;
virtual RegVal readMiscRegNoEffect(RegIndex idx) const = 0;
virtual RegVal readMiscReg(RegIndex idx) = 0;
diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc
index 940f710..8fb8f81 100644
--- a/src/arch/riscv/faults.cc
+++ b/src/arch/riscv/faults.cc
@@ -153,6 +153,9 @@
tc->setMiscReg(MISCREG_NMIE, 0);
}
+ // Clear load reservation address
+ tc->getIsaPtr()->clearLoadReservation(tc->contextId());
+
// Set PC to fault handler address
Addr addr = mbits(tc->readMiscReg(tvec), 63, 2);
if (isInterrupt() && bits(tc->readMiscReg(tvec), 1, 0) == 1)
diff --git a/src/arch/riscv/isa.cc b/src/arch/riscv/isa.cc
index d744fe36..94a8239 100644
--- a/src/arch/riscv/isa.cc
+++ b/src/arch/riscv/isa.cc
@@ -672,11 +672,6 @@
UNSERIALIZE_CONTAINER(miscRegFile);
}
-const int WARN_FAILURE = 10000;
-
-const Addr INVALID_RESERVATION_ADDR = (Addr) -1;
-std::unordered_map<int, Addr> load_reservation_addrs;
-
void
ISA::handleLockedSnoop(PacketPtr pkt, Addr cacheBlockMask)
{
@@ -696,9 +691,9 @@
{
Addr& load_reservation_addr = load_reservation_addrs[tc->contextId()];
- load_reservation_addr = req->getPaddr() & ~0xF;
+ load_reservation_addr = req->getPaddr();
DPRINTF(LLSC, "[cid:%d]: Reserved address %x.\n",
- req->contextId(), req->getPaddr() & ~0xF);
+ req->contextId(), req->getPaddr());
}
bool
@@ -717,12 +712,13 @@
lr_addr_empty ? "yes" : "no");
if (!lr_addr_empty) {
DPRINTF(LLSC, "[cid:%d]: addr = %x.\n", req->contextId(),
- req->getPaddr() & ~0xF);
+ req->getPaddr() & cacheBlockMask);
DPRINTF(LLSC, "[cid:%d]: last locked addr = %x.\n",
req->contextId(),
- load_reservation_addr);
+ load_reservation_addr & cacheBlockMask);
}
- if (lr_addr_empty
- || load_reservation_addr != ((req->getPaddr() & ~0xF))) {
+ if (lr_addr_empty ||
+ (load_reservation_addr & cacheBlockMask)
+ != ((req->getPaddr() & cacheBlockMask))) {
req->setExtraData(0);
int stCondFailures = tc->readStCondFailures();
tc->setStCondFailures(++stCondFailures);
@@ -730,12 +726,21 @@
warn("%i: context %d: %d consecutive SC failures.\n",
curTick(), tc->contextId(), stCondFailures);
}
+
+ // Must clear any reservations
+ load_reservation_addr = INVALID_RESERVATION_ADDR;
+
return false;
}
if (req->isUncacheable()) {
req->setExtraData(2);
}
+ // Must clear any reservations
+ load_reservation_addr = INVALID_RESERVATION_ADDR;
+
+ DPRINTF(LLSC, "[cid:%d]: SC success! Current locked addr = %x.\n",
+ req->contextId(), load_reservation_addr & cacheBlockMask);
return true;
}
@@ -743,6 +748,8 @@
ISA::globalClearExclusive()
{
tc->getCpuPtr()->wakeup(tc->threadId());
+ Addr& load_reservation_addr = load_reservation_addrs[tc->contextId()];
+ load_reservation_addr = INVALID_RESERVATION_ADDR;
}
void
diff --git a/src/arch/riscv/isa.hh b/src/arch/riscv/isa.hh
index 5a2a610..7ef5c52 100644
--- a/src/arch/riscv/isa.hh
+++ b/src/arch/riscv/isa.hh
@@ -76,6 +76,11 @@
bool hpmCounterEnabled(int counter) const;
+ // Load reserve - store conditional monitor
+ const int WARN_FAILURE = 10000;
+ const Addr INVALID_RESERVATION_ADDR = (Addr)-1;
+ std::unordered_map<int, Addr> load_reservation_addrs;
+
public:
using Params = RiscvISAParams;
@@ -87,6 +92,13 @@
return new PCState(new_inst_addr, rv_type);
}
+ void
+ clearLoadReservation(ContextID cid) override
+ {
+ Addr& load_reservation_addr = load_reservation_addrs[cid];
+ load_reservation_addr = INVALID_RESERVATION_ADDR;
+ }
+
public:
RegVal readMiscRegNoEffect(RegIndex idx) const override;
RegVal readMiscReg(RegIndex idx) override;
--
To view, visit
https://gem5-review.googlesource.com/c/public/gem5/+/71520?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v23-0
Gerrit-Change-Id: I64513534710b5f269260fcb204f717801913e2f5
Gerrit-Change-Number: 71520
Gerrit-PatchSet: 1
Gerrit-Owner: Bobby Bruce <bbr...@ucdavis.edu>
Gerrit-CC: Adrià Armejach <adria.armej...@bsc.es>
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
