mem: Clarify cache behaviour for pending dirty responses

This patch adds a bit of clarification around the assumptions made in
the cache when packets are sent out, and dirty responses are
pending. As part of the change, the marking of an MSHR as in service
is simplified slightly, and comments are added to explain what
assumptions are made.

diff --git a/src/mem/cache/base.hh b/src/mem/cache/base.hh
index 845a689a4ff71d934defa90a1f4766d295adfa73..0be6b79447d7b65b646a6bcc315248cee92bf04d 100644 (file)
--- a/src/mem/cache/base.hh
+++ b/src/mem/cache/base.hh
@@ -218,11 +218,11 @@ class BaseCache : public MemObject
         return mshr;
     }
 
-    void markInServiceInternal(MSHR *mshr, PacketPtr pkt)
+    void markInServiceInternal(MSHR *mshr, bool pending_dirty_resp)
     {
         MSHRQueue *mq = mshr->queue;
         bool wasFull = mq->isFull();
-        mq->markInService(mshr, pkt);
+        mq->markInService(mshr, pending_dirty_resp);
         if (wasFull && !mq->isFull()) {
             clearBlocked((BlockedCause)mq->index);
         }

diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh
index e0bd29752b903e224740708300c5e8d52fe96e38..21a00dbbd825fcc7e9f521836a05a9b31848ca1c 100644 (file)
--- a/src/mem/cache/cache.hh
+++ b/src/mem/cache/cache.hh
@@ -375,12 +375,13 @@ class Cache : public BaseCache
     PacketPtr getTimingPacket();
 
     /**
-     * Marks a request as in service (sent on the bus). This can have side
-     * effect since storage for no response commands is deallocated once they
-     * are successfully sent.
-     * @param pkt The request that was sent on the bus.
+     * Marks a request as in service (sent on the bus). This can have
+     * side effect since storage for no response commands is
+     * deallocated once they are successfully sent. Also remember if
+     * we are expecting a dirty response from another cache,
+     * effectively making this MSHR the ordering point.
      */
-    void markInService(MSHR *mshr, PacketPtr pkt = NULL);
+    void markInService(MSHR *mshr, bool pending_dirty_resp);
 
     /**
      * Return whether there are any outstanding misses.

diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index 0dd158afef72f659c301c58437c07a5b6d7cfc29..9ec0250e1169634a7fe849e3109cb77ac9819f72 100644 (file)
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -256,11 +256,9 @@ Cache<TagStore>::satisfyCpuSideRequest(PacketPtr pkt, BlkType *blk,
 
 template<class TagStore>
 void
-Cache<TagStore>::markInService(MSHR *mshr, PacketPtr pkt)
+Cache<TagStore>::markInService(MSHR *mshr, bool pending_dirty_resp)
 {
-    // packet can be either a request or response
-
-    markInServiceInternal(mshr, pkt);
+    markInServiceInternal(mshr, pending_dirty_resp);
 #if 0
         if (mshr->originalCmd == MemCmd::HardPFReq) {
             DPRINTF(HWPrefetch, "%s:Marking a HW_PF in service\n",
@@ -1745,7 +1743,7 @@ Cache<TagStore>::recvTimingSnoopReq(PacketPtr pkt)
 
             if (pkt->isInvalidate()) {
                 // Invalidation trumps our writeback... discard here
-                markInService(mshr);
+                markInService(mshr, false);
                 delete wb_pkt;
             }
         } // writebacks.size()
@@ -1910,9 +1908,10 @@ Cache<TagStore>::getTimingPacket()
             snoop_pkt.senderState = mshr;
             cpuSidePort->sendTimingSnoopReq(&snoop_pkt);
 
-            // Check to see if the prefetch was squashed by an upper cache
-            // Or if a writeback arrived between the time the prefetch was
-            // placed in the MSHRs and when it was selected to send.
+            // Check to see if the prefetch was squashed by an upper
+            // cache (to prevent us from grabbing the line) or if a
+            // writeback arrived between the time the prefetch was
+            // placed in the MSHRs and when it was selected to be sent.
             if (snoop_pkt.prefetchSquashed() || blk != NULL) {
                 DPRINTF(Cache, "Prefetch squashed by cache.  "
                                "Deallocating mshr target %#x.\n", mshr->addr);
@@ -1926,8 +1925,13 @@ Cache<TagStore>::getTimingPacket()
                 return NULL;
             }
 
+            // Check if the prefetch hit a writeback in an upper cache
+            // and if so we will eventually get a HardPFResp from
+            // above
             if (snoop_pkt.memInhibitAsserted()) {
-                markInService(mshr, &snoop_pkt);
+                // If we are getting a non-shared response it is dirty
+                bool pending_dirty_resp = !snoop_pkt.sharedAsserted();
+                markInService(mshr, pending_dirty_resp);
                 DPRINTF(Cache, "Upward snoop of prefetch for addr"
                         " %#x (%s) hit\n",
                         tgt_pkt->getAddr(), tgt_pkt->isSecure()? "s": "ns");
@@ -2148,7 +2152,19 @@ Cache<TagStore>::MemSidePacketQueue::sendDeferredPacket()
                 // care about this packet and might override it before
                 // it gets retried
             } else {
-                cache.markInService(mshr, pkt);
+                // As part of the call to sendTimingReq the packet is
+                // forwarded to all neighbouring caches (and any
+                // caches above them) as a snoop. The packet is also
+                // sent to any potential cache below as the
+                // interconnect is not allowed to buffer the
+                // packet. Thus at this point we know if any of the
+                // neighbouring, or the downstream cache is
+                // responding, and if so, if it is with a dirty line
+                // or not.
+                bool pending_dirty_resp = !pkt->sharedAsserted() &&
+                    pkt->memInhibitAsserted();
+
+                cache.markInService(mshr, pending_dirty_resp);
             }
         }
     }

diff --git a/src/mem/cache/mshr.cc b/src/mem/cache/mshr.cc
index 793db02c21ff9668c03a9a845bd1877dc965c5dd..50196edd18aa7b9f7c3cc6778431480e7ef38a5a 100644 (file)
--- a/src/mem/cache/mshr.cc
+++ b/src/mem/cache/mshr.cc
@@ -238,7 +238,7 @@ MSHR::clearDownstreamPending()
 }
 
 bool
-MSHR::markInService(PacketPtr pkt)
+MSHR::markInService(bool pending_dirty_resp)
 {
     assert(!inService);
     if (isForwardNoResponse()) {
@@ -249,10 +249,8 @@ MSHR::markInService(PacketPtr pkt)
         return true;
     }
 
-    assert(pkt != NULL);
     inService = true;
-    pendingDirty = targets.needsExclusive ||
-                   (!pkt->sharedAsserted() && pkt->memInhibitAsserted());
+    pendingDirty = targets.needsExclusive || pending_dirty_resp;
     postInvalidate = postDowngrade = false;
 
     if (!downstreamPending) {

diff --git a/src/mem/cache/mshr.hh b/src/mem/cache/mshr.hh
index 65357b9e6debf2e668155ec31210db11240e9092..c8967d2eaa1802c47635a5d8df237fff17bf437f 100644 (file)
--- a/src/mem/cache/mshr.hh
+++ b/src/mem/cache/mshr.hh
@@ -225,7 +225,7 @@ class MSHR : public Packet::SenderState, public Printable
     void allocate(Addr addr, int size, PacketPtr pkt,
                   Tick when, Counter _order);
 
-    bool markInService(PacketPtr pkt);
+    bool markInService(bool pending_dirty_resp);
 
     void clearDownstreamPending();
 

diff --git a/src/mem/cache/mshr_queue.cc b/src/mem/cache/mshr_queue.cc
index cdd6da52cd39b39371b447aeaccbc2c68c56869f..72e9b2ec3a9e381a4e90669077c8a8d41f6b594c 100644 (file)
--- a/src/mem/cache/mshr_queue.cc
+++ b/src/mem/cache/mshr_queue.cc
@@ -214,9 +214,9 @@ MSHRQueue::moveToFront(MSHR *mshr)
 }
 
 void
-MSHRQueue::markInService(MSHR *mshr, PacketPtr pkt)
+MSHRQueue::markInService(MSHR *mshr, bool pending_dirty_resp)
 {
-    if (mshr->markInService(pkt)) {
+    if (mshr->markInService(pending_dirty_resp)) {
         deallocate(mshr);
     } else {
         readyList.erase(mshr->readyIter);

diff --git a/src/mem/cache/mshr_queue.hh b/src/mem/cache/mshr_queue.hh
index 7050421fe29045b1388176f76cd3fc29df170116..1e1218782e72b66a9356e1752e6896c7dd6cda4b 100644 (file)
--- a/src/mem/cache/mshr_queue.hh
+++ b/src/mem/cache/mshr_queue.hh
@@ -183,10 +183,13 @@ class MSHRQueue : public Drainable
 
     /**
      * Mark the given MSHR as in service. This removes the MSHR from the
-     * readyList. Deallocates the MSHR if it does not expect a response.
+     * readyList or deallocates the MSHR if it does not expect a response.
+     *
      * @param mshr The MSHR to mark in service.
+     * @param pending_dirty_resp Whether we expect a dirty response
+     *                           from another cache
      */
-    void markInService(MSHR *mshr, PacketPtr pkt);
+    void markInService(MSHR *mshr, bool pending_dirty_resp);
 
     /**
      * Mark an in service entry as pending, used to resend a request.