mem-cache: mark block as dirty when handling SW prefetch

This addresses the issue described in
64687ee mem-cache: Mark block as dirty after a SWPrefetchEXResp.

Previous patch misses cases when the prefetch response is ReadExResp or
UpgradeResp. Also, marking the block as dirty in serviceMSHRTargets
instead of in handleFill covers cases when the prefetch is coalesced with
other requests.

Change-Id: I2b377fdd240eb0f09e720b6bb284dee6545925ce
Signed-off-by: Tiago Mück <tiago.muck@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/19688
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Nikos Nikoleris <nikos.nikoleris@arm.com>
Tested-by: kokoro <noreply+kokoro@google.com>

diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc
index 082650c0973675cecaafd6acc6592bc03b54ed90..0de7f21507377b48fd9d9bf37979842ecbaa91f1 100644 (file)
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -1409,30 +1409,6 @@ BaseCache::handleFill(PacketPtr pkt, CacheBlk *blk, PacketList &writebacks,
             chatty_assert(!isReadOnly, "Should never see dirty snoop response "
                           "in read-only cache %s\n", name());
 
-        } else if (pkt->cmd.isSWPrefetch() && pkt->needsWritable()) {
-            // All other copies of the block were invalidated and we
-            // have an exclusive copy.
-
-            // The coherence protocol assumes that if we fetched an
-            // exclusive copy of the block, we have the intention to
-            // modify it. Therefore the MSHR for the PrefetchExReq has
-            // been the point of ordering and this cache has commited
-            // to respond to snoops for the block.
-            //
-            // In most cases this is true anyway - a PrefetchExReq
-            // will be followed by a WriteReq. However, if that
-            // doesn't happen, the block is not marked as dirty and
-            // the cache doesn't respond to snoops that has committed
-            // to do so.
-            //
-            // To avoid deadlocks in cases where there is a snoop
-            // between the PrefetchExReq and the expected WriteReq, we
-            // proactively mark the block as Dirty.
-
-            blk->status |= BlkDirty;
-
-            panic_if(!isReadOnly, "Prefetch exclusive requests from read-only "
-                     "cache %s\n", name());
         }
     }
 

diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index bded746434d05c0e85c5ec4df4b3e91c17a57247..b054cd43d7e0e76f4bd3da6e463ef304d6aee4b1 100644 (file)
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -710,6 +710,32 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk)
 
             // Software prefetch handling for cache closest to core
             if (tgt_pkt->cmd.isSWPrefetch()) {
+                if (tgt_pkt->needsWritable()) {
+                    // All other copies of the block were invalidated and we
+                    // have an exclusive copy.
+
+                    // The coherence protocol assumes that if we fetched an
+                    // exclusive copy of the block, we have the intention to
+                    // modify it. Therefore the MSHR for the PrefetchExReq has
+                    // been the point of ordering and this cache has commited
+                    // to respond to snoops for the block.
+                    //
+                    // In most cases this is true anyway - a PrefetchExReq
+                    // will be followed by a WriteReq. However, if that
+                    // doesn't happen, the block is not marked as dirty and
+                    // the cache doesn't respond to snoops that has committed
+                    // to do so.
+                    //
+                    // To avoid deadlocks in cases where there is a snoop
+                    // between the PrefetchExReq and the expected WriteReq, we
+                    // proactively mark the block as Dirty.
+                    assert(blk);
+                    blk->status |= BlkDirty;
+
+                    panic_if(isReadOnly, "Prefetch exclusive requests from "
+                            "read-only cache %s\n", name());
+                }
+
                 // a software prefetch would have already been ack'd
                 // immediately with dummy data so the core would be able to
                 // retire it. This request completes right here, so we