mem: Align rules for sinking inhibited packets at the slave

This patch aligns how the memory-system slaves, i.e. the various
memory controllers and the bridge, identify and deal with sinking of
inhibited packets that are only useful within the coherent part of the
memory system.

In the future we could shift the onus to the crossbar, and add a
parameter "is_point_of_coherence" that would allow it to sink the
aforementioned packets.

diff --git a/src/mem/bridge.cc b/src/mem/bridge.cc
index ecaf6de0483dbce5b9ad4904d25bbdd2d3b4a119..855f39de32d786cc25e8285f9427403ae0bbb2eb 100644 (file)
--- a/src/mem/bridge.cc
+++ b/src/mem/bridge.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011-2013 ARM Limited
+ * Copyright (c) 2011-2013, 2015 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -150,8 +150,20 @@ Bridge::BridgeSlavePort::recvTimingReq(PacketPtr pkt)
     DPRINTF(Bridge, "recvTimingReq: %s addr 0x%x\n",
             pkt->cmdString(), pkt->getAddr());
 
-    // we should not see a timing request if we are already in a retry
-    assert(!retryReq);
+    // sink inhibited packets without further action, also discard any
+    // packet that is not a read or a write
+    if (pkt->memInhibitAsserted() ||
+        !(pkt->isWrite() || pkt->isRead())) {
+        assert(!pkt->needsResponse());
+        pendingDelete.reset(pkt);
+        return true;
+    }
+
+    // we should not get a new request after committing to retry the
+    // current one, but unfortunately the CPU violates this rule, so
+    // simply ignore it for now
+    if (retryReq)
+        return false;
 
     DPRINTF(Bridge, "Response queue size: %d outresp: %d\n",
             transmitList.size(), outstandingResponses);
@@ -162,8 +174,7 @@ Bridge::BridgeSlavePort::recvTimingReq(PacketPtr pkt)
         retryReq = true;
     } else {
         // look at the response queue if we expect to see a response
-        bool expects_response = pkt->needsResponse() &&
-            !pkt->memInhibitAsserted();
+        bool expects_response = pkt->needsResponse();
         if (expects_response) {
             if (respQueueFull()) {
                 DPRINTF(Bridge, "Response queue full\n");

diff --git a/src/mem/bridge.hh b/src/mem/bridge.hh
index 6aebe5204d87db465203ab0c3229ad7b77f7130a..ad3585997e240a424d3f626df23abd93134a3db9 100644 (file)
--- a/src/mem/bridge.hh
+++ b/src/mem/bridge.hh
@@ -135,6 +135,12 @@ class Bridge : public MemObject
         /** Max queue size for reserved responses. */
         unsigned int respQueueLimit;
 
+        /**
+         * Upstream caches need this packet until true is returned, so
+         * hold it for deletion until a subsequent call
+         */
+        std::unique_ptr<Packet> pendingDelete;
+
         /**
          * Is this side blocked from accepting new response packets.
          *

diff --git a/src/mem/dram_ctrl.cc b/src/mem/dram_ctrl.cc
index 4bd83f761c2082b7d365fc3ae318716cae8badf1..abf570910dac100570f95eda4d6db66164716c8e 100644 (file)
--- a/src/mem/dram_ctrl.cc
+++ b/src/mem/dram_ctrl.cc
@@ -590,10 +590,8 @@ DRAMCtrl::recvTimingReq(PacketPtr pkt)
     DPRINTF(DRAM, "recvTimingReq: request %s addr %lld size %d\n",
             pkt->cmdString(), pkt->getAddr(), pkt->getSize());
 
-    // simply drop inhibited packets and clean evictions
-    if (pkt->memInhibitAsserted() ||
-        pkt->cmd == MemCmd::CleanEvict) {
-        DPRINTF(DRAM, "Inhibited packet or clean evict -- Dropping it now\n");
+    // sink inhibited packets without further action
+    if (pkt->memInhibitAsserted()) {
         pendingDelete.reset(pkt);
         return true;
     }

diff --git a/src/mem/dramsim2.cc b/src/mem/dramsim2.cc
index cd0f45b23bdd1ced96463d7a2a219dac1e24d304..58227e06a48915dfdb779b4f15f56c1317a58988 100644 (file)
--- a/src/mem/dramsim2.cc
+++ b/src/mem/dramsim2.cc
@@ -175,16 +175,18 @@ DRAMSim2::recvFunctional(PacketPtr pkt)
 bool
 DRAMSim2::recvTimingReq(PacketPtr pkt)
 {
-    // we should never see a new request while in retry
-    assert(!retryReq);
-
+    // sink inhibited packets without further action
     if (pkt->memInhibitAsserted()) {
-        // snooper will supply based on copy of packet
-        // still target's responsibility to delete packet
         pendingDelete.reset(pkt);
         return true;
     }
 
+    // we should not get a new request after committing to retry the
+    // current one, but unfortunately the CPU violates this rule, so
+    // simply ignore it for now
+    if (retryReq)
+        return false;
+
     // if we cannot accept we need to send a retry once progress can
     // be made
     bool can_accept = nbrOutstanding() < wrapper.queueSize();

diff --git a/src/mem/simple_mem.cc b/src/mem/simple_mem.cc
index 7e350feb6a6d6ee606243b416097293f1adf5c5d..639ccbe319945b4c97f8e689cf5bd9d7a70751be 100644 (file)
--- a/src/mem/simple_mem.cc
+++ b/src/mem/simple_mem.cc
@@ -97,17 +97,15 @@ SimpleMemory::recvFunctional(PacketPtr pkt)
 bool
 SimpleMemory::recvTimingReq(PacketPtr pkt)
 {
+    // sink inhibited packets without further action
     if (pkt->memInhibitAsserted()) {
-        // snooper will supply based on copy of packet
-        // still target's responsibility to delete packet
         pendingDelete.reset(pkt);
         return true;
     }
 
-    // we should never get a new request after committing to retry the
-    // current one, the bus violates the rule as it simply sends a
-    // retry to the next one waiting on the retry list, so simply
-    // ignore it
+    // we should not get a new request after committing to retry the
+    // current one, but unfortunately the CPU violates this rule, so
+    // simply ignore it for now
     if (retryReq)
         return false;