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.

3 files changed, 45 insertions, 6 deletions

diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index 24e3eec15..4b51a6359 100644
--- 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 79a91da2b..fcf9fc425 100644
--- 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 65357b9e6..9ea9cfb2c 100644
--- 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.
      */