mem-cache: Ensure that responses get data from the right source

This CL makes sure that we use the right source for data for
responses after a response from the cache below.

Change-Id: I7329f3e6bcb7ce2054e912eb9dea48c9d169d45a
Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23667
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Tested-by: kokoro <noreply+kokoro@google.com>

diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index e7dd5efc952ac772520b0ba9341a6f71b9980ca6..a4f2baeb6593230c8e1939b6472b402239803f51 100644 (file)
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -756,7 +756,14 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk)
                 assert(blk->isWritable());
             }
 
-            if (blk && blk->isValid() && !mshr->isForward) {
+            // Here we decide whether we will satisfy the target using
+            // data from the block or from the response. We use the
+            // block data to satisfy the request when the block is
+            // present and valid and in addition the response in not
+            // forwarding data to the cache above (we didn't fill
+            // either); otherwise we use the packet data.
+            if (blk && blk->isValid() &&
+                (!mshr->isForward || !pkt->hasData())) {
                 satisfyRequest(tgt_pkt, blk, true, mshr->hasPostDowngrade());
 
                 // How many bytes past the first request is this one
@@ -791,15 +798,15 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk)
                     pkt->payloadDelay;
                 tgt_pkt->req->setExtraData(0);
             } else {
-                // We are about to send a response to a cache above
-                // that asked for an invalidation; we need to
-                // invalidate our copy immediately as the most
-                // up-to-date copy of the block will now be in the
-                // cache above. It will also prevent this cache from
-                // responding (if the block was previously dirty) to
-                // snoops as they should snoop the caches above where
-                // they will get the response from.
                 if (is_invalidate && blk && blk->isValid()) {
+                    // We are about to send a response to a cache above
+                    // that asked for an invalidation; we need to
+                    // invalidate our copy immediately as the most
+                    // up-to-date copy of the block will now be in the
+                    // cache above. It will also prevent this cache from
+                    // responding (if the block was previously dirty) to
+                    // snoops as they should snoop the caches above where
+                    // they will get the response from.
                     invalidateBlock(blk);
                 }
                 // not a cache fill, just forwarding response
@@ -807,12 +814,22 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk)
                 // from lower level cahces/memory to the core.
                 completion_time += clockEdge(responseLatency) +
                     pkt->payloadDelay;
-                if (pkt->isRead() && !is_error) {
-                    // sanity check
-                    assert(pkt->matchAddr(tgt_pkt));
-                    assert(pkt->getSize() >= tgt_pkt->getSize());
+                if (!is_error) {
+                    if (pkt->isRead()) {
+                        // sanity check
+                        assert(pkt->matchAddr(tgt_pkt));
+                        assert(pkt->getSize() >= tgt_pkt->getSize());
 
-                    tgt_pkt->setData(pkt->getConstPtr<uint8_t>());
+                        tgt_pkt->setData(pkt->getConstPtr<uint8_t>());
+                    } else {
+                        // MSHR targets can read data either from the
+                        // block or the response pkt. If we can't get data
+                        // from the block (i.e., invalid or has old data)
+                        // or the response (did not bring in any data)
+                        // then make sure that the target didn't expect
+                        // any.
+                        assert(!tgt_pkt->hasRespData());
+                    }
                 }
 
                 // this response did not allocate here and therefore