Adrià Armejach has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/71558?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
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/71558
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Reviewed-by: Roger Chang <rogerycch...@google.com>
Maintainer: Jason Lowe-Power <power...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
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(-)
Approvals:
Roger Chang: Looks good to me, approved
Jason Lowe-Power: Looks good to me, but someone else must approve; Looks
good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/generic/isa.hh b/src/arch/generic/isa.hh
index e9e4d95..58f66fc 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) {}
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/+/71558?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I64513534710b5f269260fcb204f717801913e2f5
Gerrit-Change-Number: 71558
Gerrit-PatchSet: 3
Gerrit-Owner: Adrià Armejach <adria.armej...@bsc.es>
Gerrit-Reviewer: Adrià Armejach <mico...@gmail.com>
Gerrit-Reviewer: Bobby Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Hoa Nguyen <hoangu...@ucdavis.edu>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: Roger Chang <rogerycch...@google.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-CC: kokoro <noreply+kok...@google.com>
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
