From 1f9ea6e122f6a39d936aec2f8f5ce72d267799a8 Mon Sep 17 00:00:00 2001
From: Steve Reinhardt <stever@eecs.umich.edu>
Date: Mon, 23 Jul 2007 22:28:40 -0700
Subject: [PATCH] A couple more minor bug fixes for multilevel coherence.

--HG--
extra : convert_revision : 370f9e34911157765be6fd49e826fa1af589b466
---
 src/mem/cache/cache_impl.hh | 21 ++++++++++++++-------
 src/mem/cache/miss/mshr.cc  | 10 ++++++++--
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index efd7f4588..412d10599 100644
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -148,7 +148,13 @@ void
 Cache<TagStore>::satisfyCpuSideRequest(PacketPtr pkt, BlkType *blk)
 {
     assert(blk);
-    assert(pkt->needsExclusive() ? blk->isWritable() : blk->isValid());
+    // Occasionally this is not true... if we are a lower-level cache
+    // satisfying a string of Read and ReadEx requests from
+    // upper-level caches, a Read will mark the block as shared but we
+    // can satisfy a following ReadEx anyway since we can rely on the
+    // Read requester(s) to have buffered the ReadEx snoop and to
+    // invalidate their blocks after receiving them.
+    // assert(pkt->needsExclusive() ? blk->isWritable() : blk->isValid());
     assert(pkt->getOffset(blkSize) + pkt->getSize() <= blkSize);
 
     // Check RMW operations first since both isRead() and
@@ -727,7 +733,7 @@ Cache<TagStore>::handleResponse(PacketPtr pkt)
             Tick completion_time;
             if (blk != NULL) {
                 satisfyCpuSideRequest(target->pkt, blk);
-                // How many bytes pass the first request is this one
+                // How many bytes past the first request is this one
                 int transfer_offset =
                     target->pkt->getOffset(blkSize) - initial_offset;
                 if (transfer_offset < 0) {
@@ -738,10 +744,9 @@ Cache<TagStore>::handleResponse(PacketPtr pkt)
                 completion_time = tags->getHitLatency() +
                     transfer_offset ? pkt->finishTime : pkt->firstWordTime;
 
-                if (!target->pkt->req->isUncacheable()) {
-                    missLatency[target->pkt->cmdToIndex()][0/*pkt->req->getThreadNum()*/] +=
-                        completion_time - target->recvTime;
-                }
+                assert(!target->pkt->req->isUncacheable());
+                missLatency[target->pkt->cmdToIndex()][0/*pkt->req->getThreadNum()*/] +=
+                    completion_time - target->recvTime;
             } else {
                 // not a cache fill, just forwarding response
                 completion_time = tags->getHitLatency() + pkt->finishTime;
@@ -1004,7 +1009,9 @@ template<class TagStore>
 void
 Cache<TagStore>::snoopTiming(PacketPtr pkt)
 {
-    if (pkt->req->isUncacheable()) {
+    // Note that some deferred snoops don't have requests, since the
+    // original access may have already completed
+    if (pkt->req && pkt->req->isUncacheable()) {
         //Can't get a hit on an uncacheable address
         //Revisit this for multi level coherence
         return;
diff --git a/src/mem/cache/miss/mshr.cc b/src/mem/cache/miss/mshr.cc
index 9b05aea3f..7f216ad39 100644
--- a/src/mem/cache/miss/mshr.cc
+++ b/src/mem/cache/miss/mshr.cc
@@ -208,7 +208,7 @@ MSHR::allocateTarget(PacketPtr pkt, Tick whenReady, Counter _order)
 bool
 MSHR::handleSnoop(PacketPtr pkt, Counter _order)
 {
-    if (!inService || downstreamPending) {
+    if (!inService || (pkt->isExpressSnoop() && downstreamPending)) {
         // Request has not been issued yet, or it's been issued
         // locally but is buffered unissued at some downstream cache
         // which is forwarding us this snoop.  Either way, the packet
@@ -249,13 +249,19 @@ MSHR::handleSnoop(PacketPtr pkt, Counter _order)
     if (targets->needsExclusive || pkt->needsExclusive()) {
         // actual target device (typ. PhysicalMemory) will delete the
         // packet on reception, so we need to save a copy here
-        targets->add(new Packet(pkt), curTick, _order, false);
+        PacketPtr cp_pkt = new Packet(pkt);
+        targets->add(cp_pkt, curTick, _order, false);
         ++ntargets;
 
         if (targets->needsExclusive) {
             // We're awaiting an exclusive copy, so ownership is pending.
             // It's up to us to respond once the data arrives.
             pkt->assertMemInhibit();
+        } else {
+            // Someone else may respond before we get around to
+            // processing this snoop, which means the copied request
+            // pointer will no longer be valid
+            cp_pkt->req = NULL;
         }
 
         if (pkt->needsExclusive()) {
-- 
2.30.2