commit a8d5dd1c136529e9605066368877d3b8e24dcdba
author Nikos Nikoleris <nikos.nikoleris@arm.com> Tue Feb 12 08:26:14 2019 +0000
committer Nikos Nikoleris <nikos.nikoleris@arm.com> Wed Apr 10 09:05:02 2019 +0000
tree ccf4979676d9a1c0d82689a1dc9757a83c229ed0
parent daa9dcbc0c3fe4e5014855c8b31f1cfeb5f0af91

mem-cache: Fix MSHR handling of cache clean requests

Previously satisfied clean requests would not snoop in-service
MSHRs. This is a problem when a clean request is also invalidating, in
which case we have to post-invalidate or post-downgrade outstanding
requests. This changes fixes this bug.

Change-Id: I31e42aa94dd3637b2818e00fbaae68c810145eaf
Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/17728
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>

src/mem/cache/cache.cc

diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index 89e40f3..938b6f4 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -1180,13 +1180,6 @@
         return;
     }
 
-    // Bypass any existing cache maintenance requests if the request
-    // has been satisfied already (i.e., the dirty block has been
-    // found).
-    if (mshr && pkt->req->isCacheMaintenance() && pkt->satisfied()) {
-        return;
-    }
-
     // Let the MSHR itself track the snoop and decide whether we want
     // to go ahead and do the regular cache snoop
     if (mshr && mshr->handleSnoop(pkt, order++)) {

src/mem/cache/mshr.cc

diff --git a/src/mem/cache/mshr.cc b/src/mem/cache/mshr.cc
index 5b93029..8db7c29 100644
--- a/src/mem/cache/mshr.cc
+++ b/src/mem/cache/mshr.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2013, 2015-2018 ARM Limited
+ * Copyright (c) 2012-2013, 2015-2019 ARM Limited
  * All rights reserved.
  *
  * The license below extends only to copyright in the software and shall
@@ -459,7 +459,14 @@
             // in the case of an uncacheable request there is no need
             // to set the responderHadWritable flag, but since the
             // recipient does not care there is no harm in doing so
+        } else if (isPendingModified() && pkt->isClean()) {
+            // this cache doesn't respond to the clean request, a
+            // destination xbar will respond to this request, but to
+            // do so it needs to know if it should wait for the
+            // WriteCleanReq
+            pkt->setSatisfied();
         }
+
         targets.add(cp_pkt, curTick(), _order, Target::FromSnoop,
                     downstreamPending && targets.needsWritable, false);
 
@@ -467,10 +474,6 @@
             // This transaction will take away our pending copy
             postInvalidate = true;
         }
-
-        if (isPendingModified() && pkt->isClean()) {
-            pkt->setSatisfied();
-        }
     }
 
     if (!pkt->needsWritable() && !pkt->req->isUncacheable()) {