bool hpmCounterEnabled(int counter) const;

Bobby Bruce has submitted this change. ( 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 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/71520 Maintainer: Bobby Bruce <bbruce@ucdavis.edu> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Bobby Bruce <bbruce@ucdavis.edu> --- 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: kokoro: Regressions pass Bobby Bruce: Looks good to me, approved; Looks good to me, approved 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/+/71520?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: release-staging-v23-0 Gerrit-Change-Id: I64513534710b5f269260fcb204f717801913e2f5 Gerrit-Change-Number: 71520 Gerrit-PatchSet: 3 Gerrit-Owner: Bobby Bruce <bbruce@ucdavis.edu> Gerrit-Reviewer: Bobby Bruce <bbruce@ucdavis.edu> Gerrit-Reviewer: kokoro <noreply+kokoro@google.com> Gerrit-CC: Adrià Armejach <adria.armejach@bsc.es> Gerrit-CC: kokoro <noreply+kokoro@google.com>