From cabd4768c7186911fda91b9ea458df775b79486a Mon Sep 17 00:00:00 2001 From: Krishnendra Nathella Date: Sun, 19 Jul 2015 15:03:30 -0500 Subject: cpu: Fix LLSC atomic CPU wakeup Writes to locked memory addresses (LLSC) did not wake up the locking CPU. This can lead to deadlocks on multi-core runs. In AtomicSimpleCPU, recvAtomicSnoop was checking if the incoming packet was an invalidation (isInvalidate) and only then handled a locked snoop. But, writes are seen instead of invalidates when running without caches (fast-forward configurations). As as simple fix, now handleLockedSnoop is also called even if the incoming snoop packet are from writes. --- src/cpu/minor/lsq.cc | 4 +++- src/cpu/o3/lsq_unit_impl.hh | 6 ++---- src/cpu/simple/atomic.cc | 5 ++++- src/cpu/simple/timing.cc | 10 ++++++++-- 4 files changed, 17 insertions(+), 8 deletions(-) (limited to 'src/cpu') diff --git a/src/cpu/minor/lsq.cc b/src/cpu/minor/lsq.cc index e644951f8..e0c5796c8 100644 --- a/src/cpu/minor/lsq.cc +++ b/src/cpu/minor/lsq.cc @@ -1617,7 +1617,9 @@ LSQ::recvTimingSnoopReq(PacketPtr pkt) * this action on snoops. */ /* THREAD */ - TheISA::handleLockedSnoop(cpu.getContext(0), pkt, cacheBlockMask); + if (pkt->isInvalidate() || pkt->isWrite()) { + TheISA::handleLockedSnoop(cpu.getContext(0), pkt, cacheBlockMask); + } } } diff --git a/src/cpu/o3/lsq_unit_impl.hh b/src/cpu/o3/lsq_unit_impl.hh index 288f6271e..b87ab0240 100644 --- a/src/cpu/o3/lsq_unit_impl.hh +++ b/src/cpu/o3/lsq_unit_impl.hh @@ -438,10 +438,8 @@ LSQUnit::checkSnoop(PacketPtr pkt) int load_idx = loadHead; DPRINTF(LSQUnit, "Got snoop for address %#x\n", pkt->getAddr()); - // Unlock the cpu-local monitor when the CPU sees a snoop to a locked - // address. The CPU can speculatively execute a LL operation after a pending - // SC operation in the pipeline and that can make the cache monitor the CPU - // is connected to valid while it really shouldn't be. + // Only Invalidate packet calls checkSnoop + assert(pkt->isInvalidate()); for (int x = 0; x < cpu->numContexts(); x++) { ThreadContext *tc = cpu->getContext(x); bool no_squash = cpu->thread[x]->noSquashFromTC; diff --git a/src/cpu/simple/atomic.cc b/src/cpu/simple/atomic.cc index 1eb219483..f3e14d401 100644 --- a/src/cpu/simple/atomic.cc +++ b/src/cpu/simple/atomic.cc @@ -292,7 +292,10 @@ AtomicSimpleCPU::AtomicCPUDPort::recvAtomicSnoop(PacketPtr pkt) } // if snoop invalidates, release any associated locks - if (pkt->isInvalidate()) { + // When run without caches, Invalidation packets will not be received + // hence we must check if the incoming packets are writes and wakeup + // the processor accordingly + if (pkt->isInvalidate() || pkt->isWrite()) { DPRINTF(SimpleCPU, "received invalidation for addr:%#x\n", pkt->getAddr()); for (auto &t_info : cpu->threadInfo) { diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple/timing.cc index da6427306..43f4eb9f4 100644 --- a/src/cpu/simple/timing.cc +++ b/src/cpu/simple/timing.cc @@ -876,8 +876,14 @@ TimingSimpleCPU::DcachePort::recvTimingSnoopReq(PacketPtr pkt) } } - for (auto &t_info : cpu->threadInfo) { - TheISA::handleLockedSnoop(t_info->thread, pkt, cacheBlockMask); + // Making it uniform across all CPUs: + // The CPUs need to be woken up only on an invalidation packet (when using caches) + // or on an incoming write packet (when not using caches) + // It is not necessary to wake up the processor on all incoming packets + if (pkt->isInvalidate() || pkt->isWrite()) { + for (auto &t_info : cpu->threadInfo) { + TheISA::handleLockedSnoop(t_info->thread, pkt, cacheBlockMask); + } } } -- cgit v1.2.3