mem: fix functional accesses to deal with coherence change

We can't just obliviously return the first valid cache block
we find any more... see comments for details.

3 files changed, 29 insertions, 5 deletions

diff --git a/src/mem/bridge.cc b/src/mem/bridge.cc
index d0135fc9d..668b492e8 100644
--- a/src/mem/bridge.cc
+++ b/src/mem/bridge.cc
@@ -325,8 +325,10 @@ Bridge::BridgePort::recvFunctional(PacketPtr pkt)
     pkt->pushLabel(name());
 
     for (i = sendQueue.begin();  i != sendQueue.end(); ++i) {
-        if (pkt->checkFunctional((*i)->pkt))
+        if (pkt->checkFunctional((*i)->pkt)) {
+            pkt->makeResponse();
             return;
+        }
     }
 
     pkt->popLabel();
diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index 8d2806b8d..3f3dc6877 100644
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -759,21 +759,44 @@ Cache<TagStore>::functionalAccess(PacketPtr pkt,
 {
     Addr blk_addr = blockAlign(pkt->getAddr());
     BlkType *blk = tags->findBlock(pkt->getAddr());
+    MSHR *mshr = mshrQueue.findMatch(blk_addr);
 
     pkt->pushLabel(name());
 
     CacheBlkPrintWrapper cbpw(blk);
-    bool done =
-        (blk && pkt->checkFunctional(&cbpw, blk_addr, blkSize, blk->data))
+
+    // Note that just because an L2/L3 has valid data doesn't mean an
+    // L1 doesn't have a more up-to-date modified copy that still
+    // needs to be found.  As a result we always update the request if
+    // we have it, but only declare it satisfied if we are the owner.
+
+    // see if we have data at all (owned or otherwise)
+    bool have_data = blk && blk->isValid()
+        && pkt->checkFunctional(&cbpw, blk_addr, blkSize, blk->data);
+
+    // data we have is dirty if marked as such or if valid & ownership
+    // pending due to outstanding UpgradeReq
+    bool have_dirty =
+        have_data && (blk->isDirty() ||
+                      (mshr && mshr->inService && mshr->isPendingDirty()));
+
+    bool done = have_dirty
         || incomingPort->checkFunctional(pkt)
         || mshrQueue.checkFunctional(pkt, blk_addr)
         || writeBuffer.checkFunctional(pkt, blk_addr)
         || otherSidePort->checkFunctional(pkt);
 
+    DPRINTF(Cache, "functional %s %x %s%s%s\n",
+            pkt->cmdString(), pkt->getAddr(),
+            (blk && blk->isValid()) ? "valid " : "",
+            have_data ? "data " : "", done ? "done " : "");
+
     // We're leaving the cache, so pop cache->name() label
     pkt->popLabel();
 
-    if (!done) {
+    if (done) {
+        pkt->makeResponse();
+    } else {
         otherSidePort->sendFunctional(pkt);
     }
 }
diff --git a/src/mem/packet.cc b/src/mem/packet.cc
index 001725e66..5eb2ecc4b 100644
--- a/src/mem/packet.cc
+++ b/src/mem/packet.cc
@@ -179,7 +179,6 @@ Packet::checkFunctional(Printable *obj, Addr addr, int size, uint8_t *data)
         if (func_start >= val_start && func_end <= val_end) {
             allocate();
             memcpy(getPtr<uint8_t>(), data + offset, getSize());
-            makeResponse();
             return true;
         } else {
             // In this case the timing packet only partially satisfies