mem: Add snoops for CleanEvicts and Writebacks in atomic mode
authorAli Jafri <ali.jafri@arm.com>
Fri, 25 Sep 2015 11:26:57 +0000 (07:26 -0400)
committerAli Jafri <ali.jafri@arm.com>
Fri, 25 Sep 2015 11:26:57 +0000 (07:26 -0400)
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.

src/mem/cache/cache.cc
src/mem/cache/cache.hh
src/mem/coherent_xbar.cc

index 19d8dc800eb1a4ff0de9570d0eac69ff416e8129..5ca17b88d9ddbf0a6ff347799c9c9115585d9cb9 100644 (file)
@@ -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
index 577eab664299810fcbcaf0a07d8e020439615f9e..26c9637f0d14442256ac9a945377598f7e5090ce 100644 (file)
@@ -251,6 +251,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
index b07356ede9869212fc62847f2ac201427bdd432a..7181cb965802219aa3f23c1ba34ba9749652c6f6 100644 (file)
@@ -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]);
     }