mem: Add snoops for CleanEvicts and Writebacks in atomic mode

This patch mirrors the logic in timing mode which sends up snoops to
check for cached copies before sending CleanEvicts and Writebacks down
the memory hierarchy. In case there is a copy in a cache above,
discard CleanEvicts and set the BLOCK_CACHED flag in Writebacks so
that writebacks do not reset the cache residency bit in the snoop
filter below.

3 files changed, 79 insertions, 34 deletions

diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index 19d8dc800..5ca17b88d 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -455,6 +455,39 @@ Cache::doWritebacks(PacketList& writebacks, Tick forward_time)
     }
 }
 
+void
+Cache::doWritebacksAtomic(PacketList& writebacks)
+{
+    while (!writebacks.empty()) {
+        PacketPtr wbPkt = writebacks.front();
+        // Call isCachedAbove for both Writebacks and CleanEvicts. If
+        // isCachedAbove returns true we set BLOCK_CACHED flag in Writebacks
+        // and discard CleanEvicts.
+        if (isCachedAbove(wbPkt, false)) {
+            if (wbPkt->cmd == MemCmd::Writeback) {
+                // Set BLOCK_CACHED flag in Writeback and send below,
+                // so that the Writeback does not reset the bit
+                // corresponding to this address in the snoop filter
+                // below. We can discard CleanEvicts because cached
+                // copies exist above. Atomic mode isCachedAbove
+                // modifies packet to set BLOCK_CACHED flag
+                memSidePort->sendAtomic(wbPkt);
+            }
+        } else {
+            // If the block is not cached above, send packet below. Both
+            // CleanEvict and Writeback with BLOCK_CACHED flag cleared will
+            // reset the bit corresponding to this address in the snoop filter
+            // below.
+            memSidePort->sendAtomic(wbPkt);
+        }
+        writebacks.pop_front();
+        // In case of CleanEvicts, the packet destructor will delete the
+        // request object because this is a non-snoop request packet which
+        // does not require a response.
+        delete wbPkt;
+    }
+}
+
 
 void
 Cache::recvTimingSnoopResp(PacketPtr pkt)
@@ -953,12 +986,7 @@ Cache::recvAtomic(PacketPtr pkt)
 
     // handle writebacks resulting from the access here to ensure they
     // logically proceed anything happening below
-    while (!writebacks.empty()){
-        PacketPtr wbPkt = writebacks.front();
-        memSidePort->sendAtomic(wbPkt);
-        writebacks.pop_front();
-        delete wbPkt;
-    }
+    doWritebacksAtomic(writebacks);
 
     if (!satisfied) {
         // MISS
@@ -1040,12 +1068,7 @@ Cache::recvAtomic(PacketPtr pkt)
     // there).
 
     // Handle writebacks (from the response handling) if needed
-    while (!writebacks.empty()){
-        PacketPtr wbPkt = writebacks.front();
-        memSidePort->sendAtomic(wbPkt);
-        writebacks.pop_front();
-        delete wbPkt;
-    }
+    doWritebacksAtomic(writebacks);
 
     if (pkt->needsResponse()) {
         pkt->makeAtomicResponse();
@@ -1906,7 +1929,7 @@ Cache::recvTimingSnoopReq(PacketPtr pkt)
     // Snoops shouldn't happen when bypassing caches
     assert(!system->bypassCaches());
 
-    // no need to snoop writebacks or requests that are not in range
+    // no need to snoop requests that are not in range
     if (!inRange(pkt->getAddr())) {
         return;
     }
@@ -2044,11 +2067,8 @@ Cache::recvAtomicSnoop(PacketPtr pkt)
     // Snoops shouldn't happen when bypassing caches
     assert(!system->bypassCaches());
 
-    // no need to snoop writebacks or requests that are not in range. In
-    // atomic we have no Writebacks/CleanEvicts queued and no prefetches,
-    // hence there is no need to snoop upwards and determine if they are
-    // present above.
-    if (pkt->evictingBlock() || !inRange(pkt->getAddr())) {
+    // no need to snoop requests that are not in range.
+    if (!inRange(pkt->getAddr())) {
         return 0;
     }
 
@@ -2144,7 +2164,7 @@ Cache::getNextMSHR()
 }
 
 bool
-Cache::isCachedAbove(const PacketPtr pkt) const
+Cache::isCachedAbove(PacketPtr pkt, bool is_timing) const
 {
     if (!forwardSnoops)
         return false;
@@ -2153,18 +2173,22 @@ Cache::isCachedAbove(const PacketPtr pkt) const
     // same block. Using the BLOCK_CACHED flag with the Writeback/CleanEvict
     // packet, the cache can inform the crossbar below of presence or absence
     // of the block.
-
-    Packet snoop_pkt(pkt, true, false);
-    snoop_pkt.setExpressSnoop();
-    // Assert that packet is either Writeback or CleanEvict and not a prefetch
-    // request because prefetch requests need an MSHR and may generate a snoop
-    // response.
-    assert(pkt->evictingBlock());
-    snoop_pkt.senderState = NULL;
-    cpuSidePort->sendTimingSnoopReq(&snoop_pkt);
-    // Writeback/CleanEvict snoops do not generate a separate snoop response.
-    assert(!(snoop_pkt.memInhibitAsserted()));
-    return snoop_pkt.isBlockCached();
+    if (is_timing) {
+        Packet snoop_pkt(pkt, true, false);
+        snoop_pkt.setExpressSnoop();
+        // Assert that packet is either Writeback or CleanEvict and not a
+        // prefetch request because prefetch requests need an MSHR and may
+        // generate a snoop response.
+        assert(pkt->evictingBlock());
+        snoop_pkt.senderState = NULL;
+        cpuSidePort->sendTimingSnoopReq(&snoop_pkt);
+        // Writeback/CleanEvict snoops do not generate a snoop response.
+        assert(!(snoop_pkt.memInhibitAsserted()));
+        return snoop_pkt.isBlockCached();
+    } else {
+        cpuSidePort->sendAtomicSnoop(pkt);
+        return pkt->isBlockCached();
+    }
 }
 
 PacketPtr
diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh
index 577eab664..26c9637f0 100644
--- a/src/mem/cache/cache.hh
+++ b/src/mem/cache/cache.hh
@@ -252,6 +252,11 @@ class Cache : public BaseCache
     void doWritebacks(PacketList& writebacks, Tick forward_time);
 
     /**
+     * Send writebacks down the memory hierarchy in atomic mode
+     */
+    void doWritebacksAtomic(PacketList& writebacks);
+
+    /**
      * Handles a response (cache line fill/write ack) from the bus.
      * @param pkt The response packet
      */
@@ -375,7 +380,7 @@ class Cache : public BaseCache
      * Send up a snoop request and find cached copies. If cached copies are
      * found, set the BLOCK_CACHED flag in pkt.
      */
-    bool isCachedAbove(const PacketPtr pkt) const;
+    bool isCachedAbove(PacketPtr pkt, bool is_timing = true) const;
 
     /**
      * Selects an outstanding request to service.  Called when the
diff --git a/src/mem/coherent_xbar.cc b/src/mem/coherent_xbar.cc
index b07356ede..7181cb965 100644
--- a/src/mem/coherent_xbar.cc
+++ b/src/mem/coherent_xbar.cc
@@ -605,6 +605,13 @@ CoherentXBar::recvAtomic(PacketPtr pkt, PortID slave_port_id)
                     " SF size: %i lat: %i\n", __func__,
                     slavePorts[slave_port_id]->name(), pkt->cmdString(),
                     pkt->getAddr(), sf_res.first.size(), sf_res.second);
+
+            // let the snoop filter know about the success of the send
+            // operation, and do it even before sending it onwards to
+            // avoid situations where atomic upward snoops sneak in
+            // between and change the filter state
+            snoopFilter->updateRequest(pkt, *slavePorts[slave_port_id], false);
+
             snoop_result = forwardAtomic(pkt, slave_port_id, InvalidPortID,
                                          sf_res.first);
         } else {
@@ -614,6 +621,15 @@ CoherentXBar::recvAtomic(PacketPtr pkt, PortID slave_port_id)
         snoop_response_latency += snoop_result.second;
     }
 
+    // forwardAtomic snooped into peer caches of the sender, and if
+    // this is a clean evict, but the packet is found in a cache, do
+    // not forward it
+    if (pkt->cmd == MemCmd::CleanEvict && pkt->isBlockCached()) {
+        DPRINTF(CoherentXBar, "recvAtomic: Clean evict 0x%x still cached, "
+                "not forwarding\n", pkt->getAddr());
+        return 0;
+    }
+
     // even if we had a snoop response, we must continue and also
     // perform the actual request at the destination
     PortID master_port_id = findPort(pkt->getAddr());
@@ -626,8 +642,8 @@ CoherentXBar::recvAtomic(PacketPtr pkt, PortID slave_port_id)
     // forward the request to the appropriate destination
     Tick response_latency = masterPorts[master_port_id]->sendAtomic(pkt);
 
-    // Lower levels have replied, tell the snoop filter
-    if (snoopFilter && !system->bypassCaches() && pkt->isResponse()) {
+    // if lower levels have replied, tell the snoop filter
+    if (!system->bypassCaches() && snoopFilter && pkt->isResponse()) {
         snoopFilter->updateResponse(pkt, *slavePorts[slave_port_id]);
     }