mem: Service only the 1st FromCPU MSHR target on ReadRespWithInv

A response to a ReadReq can either be a ReadResp or a
ReadRespWithInvalidate. As we add targets to an MSHR for a ReadReq we
assume that the response will be a ReadResp. When the response is
invalidating (ReadRespWithInvalidate) servicing more than one targets
can potentially violate the memory ordering. This change fixes the way
we handle a ReadRespWithInvalidate. When a cache receives a
ReadRespWithInvalidate we service only the first FromCPU target and
all the FromSnoop targets from the MSHR target list. The rest of the
FromCPU targets are deferred and serviced by a new request.

Change-Id: I75c30c268851987ee5f8644acb46f440b4eeeec2
Reviewed-by: Andreas Hansson <andreas.hansson@arm.com>
Reviewed-by: Stephan Diestelhorst <stephan.diestelhorst@arm.com>

1 files changed, 7 insertions, 11 deletions

diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index db982c1f0..b7f336da9 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -1330,12 +1330,10 @@ Cache::recvTimingResp(PacketPtr pkt)
     int initial_offset = initial_tgt->pkt->getOffset(blkSize);
 
     bool from_cache = false;
-
-    while (mshr->hasTargets()) {
-        MSHR::Target *target = mshr->getTarget();
-        Packet *tgt_pkt = target->pkt;
-
-        switch (target->source) {
+    MSHR::TargetList targets = mshr->extractServiceableTargets(pkt);
+    for (auto &target: targets) {
+        Packet *tgt_pkt = target.pkt;
+        switch (target.source) {
           case MSHR::Target::FromCPU:
             Tick completion_time;
             // Here we charge on completion_time the delay of the xbar if the
@@ -1370,7 +1368,7 @@ Cache::recvTimingResp(PacketPtr pkt)
                 mshr->promoteWritable();
                 // NB: we use the original packet here and not the response!
                 blk = handleFill(tgt_pkt, blk, writebacks,
-                                 mshr->allocOnFill());
+                                 targets.allocOnFill);
                 assert(blk != nullptr);
 
                 // treat as a fill, and discard the invalidation
@@ -1400,7 +1398,7 @@ Cache::recvTimingResp(PacketPtr pkt)
 
                 assert(tgt_pkt->req->masterId() < system->maxMasters());
                 missLatency[tgt_pkt->cmdToIndex()][tgt_pkt->req->masterId()] +=
-                    completion_time - target->recvTime;
+                    completion_time - target.recvTime;
             } else if (pkt->cmd == MemCmd::UpgradeFailResp) {
                 // failed StoreCond upgrade
                 assert(tgt_pkt->cmd == MemCmd::StoreCondReq ||
@@ -1462,10 +1460,8 @@ Cache::recvTimingResp(PacketPtr pkt)
             break;
 
           default:
-            panic("Illegal target->source enum %d\n", target->source);
+            panic("Illegal target->source enum %d\n", target.source);
         }
-
-        mshr->popTarget();
     }
 
     maintainClusivity(from_cache, blk);