mem: have WriteInvalidate obsolete MSHRs

Since WriteInvalidate directly writes into the cache, it can
create tricky timing interleavings with reads and writes to the
same cache line that haven't yet completed.  This patch ensures
that these requests, when completed, don't overwrite the newer
data from the WriteInvalidate.

diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index 24e3eec15e3864583668c5fca62b71bcbf2bd9c0..4b51a63591d0625b2036cc348e932ceec8545975 100644 (file)
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -636,7 +636,20 @@ Cache<TagStore>::recvTimingReq(PacketPtr pkt)
             pkt = pf;
         }
 
-        if (mshr) {
+        if (pkt && (pkt->cmd == MemCmd::WriteInvalidateReq)) {
+            // WriteInvalidates cannot coalesce with other requests, so
+            // we cannot use an existing MSHR.  If one exists, we mark it
+            // as 'obsolete' so they don't modify the cache.
+            if (mshr) {
+                // Everything up to this point is obsolete, meaning
+                // they should not modify the cache.
+                DPRINTF(Cache, "%s: marking MSHR obsolete in %s of %x\n",
+                        __func__, pkt->cmdString(), pkt->getAddr());
+
+                mshr->markObsolete();
+            }
+            allocateMissBuffer(pkt, time, true);
+        } else if (mshr) {
             /// MSHR hit
             /// @note writebacks will be checked in getNextMSHR()
             /// for any conflicting requests to the same block
@@ -1077,7 +1090,10 @@ Cache<TagStore>::recvTimingResp(PacketPtr pkt)
     bool is_fill = !mshr->isForward &&
         (pkt->isRead() || pkt->cmd == MemCmd::UpgradeResp);
 
-    if (is_fill && !is_error) {
+    if (mshr->isObsolete()) {
+        DPRINTF(Cache, "%s: skipping cache fills; data for %s of %x "
+                "is obsolete\n", __func__, pkt->cmdString(), pkt->getAddr());
+    } else if (is_fill && !is_error) {
         DPRINTF(Cache, "Block for addr %x being updated in Cache\n",
                 pkt->getAddr());
 
@@ -1113,8 +1129,19 @@ Cache<TagStore>::recvTimingResp(PacketPtr pkt)
             }
 
             if (is_fill) {
-                satisfyCpuSideRequest(target->pkt, blk,
-                                      true, mshr->hasPostDowngrade());
+                // Presently the only situation leading to 'obsolete'
+                // data is when a WriteInvalidate blows away an already
+                // pending/in-progress read. We don't want to overwrite
+                // cache data in that case.
+                if (mshr->isObsolete()) {
+                    DPRINTF(Cache, "%s: skipping satisfyCpuSideRequest; "
+                            "data for %s of %x is obsolete\n",
+                            __func__, target->pkt->cmdString(),
+                            target->pkt->getAddr());
+                } else {
+                    satisfyCpuSideRequest(target->pkt, blk,
+                                          true, mshr->hasPostDowngrade());
+                }
                 // How many bytes past the first request is this one
                 int transfer_offset =
                     target->pkt->getOffset(blkSize) - initial_offset;

diff --git a/src/mem/cache/mshr.cc b/src/mem/cache/mshr.cc
index 79a91da2be0552090e3c276f97b677e4bb4e501c..fcf9fc425ec19a3a9971f747f6c195e083c6cc9d 100644 (file)
--- a/src/mem/cache/mshr.cc
+++ b/src/mem/cache/mshr.cc
@@ -62,8 +62,8 @@
 using namespace std;
 
 MSHR::MSHR() : readyTime(0), _isUncacheable(false), downstreamPending(false),
-               pendingDirty(false), postInvalidate(false),
-               postDowngrade(false), queue(NULL), order(0), addr(0), size(0),
+               pendingDirty(false), postInvalidate(false), postDowngrade(false),
+               _isObsolete(false), queue(NULL), order(0), addr(0), size(0),
                isSecure(false), inService(false), isForward(false),
                threadNum(InvalidThreadID), data(NULL)
 {
@@ -214,6 +214,7 @@ MSHR::allocate(Addr _addr, int _size, PacketPtr target, Tick whenReady,
     _isUncacheable = target->req->isUncacheable();
     inService = false;
     downstreamPending = false;
+    _isObsolete = false;
     threadNum = 0;
     assert(targets.isReset());
     // Don't know of a case where we would allocate a new MSHR for a

diff --git a/src/mem/cache/mshr.hh b/src/mem/cache/mshr.hh
index 65357b9e6debf2e668155ec31210db11240e9092..9ea9cfb2c031a0b8b0aa8e3352b5aa765eeb19a5 100644 (file)
--- a/src/mem/cache/mshr.hh
+++ b/src/mem/cache/mshr.hh
@@ -89,6 +89,9 @@ class MSHR : public Packet::SenderState, public Printable
     /** Did we snoop a read while waiting for data? */
     bool postDowngrade;
 
+    /** Did we get WriteInvalidate'd (and therefore obsoleted)? */
+    bool _isObsolete;
+
   public:
 
     class Target {
@@ -214,6 +217,8 @@ class MSHR : public Packet::SenderState, public Printable
 
     bool isUncacheable() const { return _isUncacheable; }
 
+    bool isObsolete() const { return _isObsolete; }
+
     /**
      * Allocate a miss to this MSHR.
      * @param cmd The requesting command.
@@ -289,6 +294,12 @@ class MSHR : public Packet::SenderState, public Printable
 
     bool checkFunctional(PacketPtr pkt);
 
+    /** Mark this MSHR as tracking a transaction with obsoleted data. It still
+      * needs to complete its lifecycle, but should not modify the cache. */
+    void markObsolete() {
+        _isObsolete = true;
+    }
+
     /**
      * Prints the contents of this MSHR for debugging.
      */