mem: Delay responses in the crossbar before forwarding

This patch changes how the crossbar classes deal with
responses. Instead of forwarding responses directly and burdening the
neighbouring modules in paying for the latency (through the
pkt->headerDelay), we now queue them before sending them.

The coherency protocol is not affected as requests and any snoop
requests/responses are still passed on in zero time. Thus, the
responses end up paying for any header delay accumulated when passing
through the crossbar. Any latency incurred on the request path will be
paid for on the response side, if no other module has dealt with it.

As a result of this patch, responses are returned at a later
point. This affects the number of outstanding transactions, and quite
a few regressions see an impact in blocking due to no MSHRs, increased
cache-miss latencies, etc.

Going forward we should be able to use the same concept also for snoop
responses, and any request that is not an express snoop.

7 files changed, 73 insertions, 53 deletions

diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index 9dec54f45..94a3ad4ee 100644
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -1176,6 +1176,11 @@ Cache::recvTimingResp(PacketPtr pkt)
 {
     assert(pkt->isResponse());
 
+    // all header delay should be paid for by the crossbar, unless
+    // this is a prefetch response from above
+    panic_if(pkt->headerDelay != 0 && pkt->cmd != MemCmd::HardPFResp,
+             "%s saw a non-zero packet delay\n", name());
+
     MSHR *mshr = dynamic_cast<MSHR*>(pkt->senderState);
     bool is_error = pkt->isError();
 
diff --git a/src/mem/coherent_xbar.cc b/src/mem/coherent_xbar.cc
index b58511db6..7f1639e44 100644
--- a/src/mem/coherent_xbar.cc
+++ b/src/mem/coherent_xbar.cc
@@ -89,7 +89,7 @@ CoherentXBar::CoherentXBar(const CoherentXBarParams *p)
     // create the slave ports, once again starting at zero
     for (int i = 0; i < p->port_slave_connection_count; ++i) {
         std::string portName = csprintf("%s.slave[%d]", name(), i);
-        SlavePort* bp = new CoherentXBarSlavePort(portName, *this, i);
+        QueuedSlavePort* bp = new CoherentXBarSlavePort(portName, *this, i);
         slavePorts.push_back(bp);
         respLayers.push_back(new RespLayer(*bp, *this,
                                            csprintf(".respLayer%d", i)));
@@ -345,12 +345,11 @@ CoherentXBar::recvTimingResp(PacketPtr pkt, PortID master_port_id)
         snoopFilter->updateResponse(pkt, *slavePorts[slave_port_id]);
     }
 
-    // send the packet through the destination slave port
-    bool success M5_VAR_USED = slavePorts[slave_port_id]->sendTimingResp(pkt);
-
-    // currently it is illegal to block responses... can lead to
-    // deadlock
-    assert(success);
+    // send the packet through the destination slave port and pay for
+    // any outstanding header delay
+    Tick latency = pkt->headerDelay;
+    pkt->headerDelay = 0;
+    slavePorts[slave_port_id]->schedTimingResp(pkt, curTick() + latency);
 
     // remove the request from the routing table
     routeTo.erase(route_lookup);
@@ -517,18 +516,11 @@ CoherentXBar::recvTimingSnoopResp(PacketPtr pkt, PortID slave_port_id)
                 pkt->getAddr());
 
         // as a normal response, it should go back to a master through
-        // one of our slave ports, at this point we are ignoring the
-        // fact that the response layer could be busy and do not touch
-        // its state
-        bool success M5_VAR_USED =
-            slavePorts[dest_port_id]->sendTimingResp(pkt);
-
-        // @todo Put the response in an internal FIFO and pass it on
-        // to the response layer from there
-
-        // currently it is illegal to block responses... can lead
-        // to deadlock
-        assert(success);
+        // one of our slave ports, we also pay for any outstanding
+        // header latency
+        Tick latency = pkt->headerDelay;
+        pkt->headerDelay = 0;
+        slavePorts[dest_port_id]->schedTimingResp(pkt, curTick() + latency);
 
         respLayers[dest_port_id]->succeededTiming(packetFinishTime);
     }
@@ -546,7 +538,7 @@ CoherentXBar::recvTimingSnoopResp(PacketPtr pkt, PortID slave_port_id)
 
 void
 CoherentXBar::forwardTiming(PacketPtr pkt, PortID exclude_slave_port_id,
-                           const std::vector<SlavePort*>& dests)
+                           const std::vector<QueuedSlavePort*>& dests)
 {
     DPRINTF(CoherentXBar, "%s for %s address %x size %d\n", __func__,
             pkt->cmdString(), pkt->getAddr(), pkt->getSize());
@@ -700,7 +692,7 @@ CoherentXBar::recvAtomicSnoop(PacketPtr pkt, PortID master_port_id)
 std::pair<MemCmd, Tick>
 CoherentXBar::forwardAtomic(PacketPtr pkt, PortID exclude_slave_port_id,
                            PortID source_master_port_id,
-                           const std::vector<SlavePort*>& dests)
+                           const std::vector<QueuedSlavePort*>& dests)
 {
     // the packet may be changed on snoops, record the original
     // command to enable us to restore it between snoops so that
@@ -787,6 +779,18 @@ CoherentXBar::recvFunctional(PacketPtr pkt, PortID slave_port_id)
     // there is no need to continue if the snooping has found what we
     // were looking for and the packet is already a response
     if (!pkt->isResponse()) {
+        // since our slave ports are queued ports we need to check them as well
+        for (const auto& p : slavePorts) {
+            // if we find a response that has the data, then the
+            // downstream caches/memories may be out of date, so simply stop
+            // here
+            if (p->checkFunctional(pkt)) {
+                if (pkt->needsResponse())
+                    pkt->makeResponse();
+                return;
+            }
+        }
+
         PortID dest_id = findPort(pkt->getAddr());
 
         masterPorts[dest_id]->sendFunctional(pkt);
diff --git a/src/mem/coherent_xbar.hh b/src/mem/coherent_xbar.hh
index 24506d22b..4b495ac2c 100644
--- a/src/mem/coherent_xbar.hh
+++ b/src/mem/coherent_xbar.hh
@@ -84,7 +84,7 @@ class CoherentXBar : public BaseXBar
      * be instantiated for each of the master ports connecting to the
      * crossbar.
      */
-    class CoherentXBarSlavePort : public SlavePort
+    class CoherentXBarSlavePort : public QueuedSlavePort
     {
 
       private:
@@ -92,11 +92,15 @@ class CoherentXBar : public BaseXBar
         /** A reference to the crossbar to which this port belongs. */
         CoherentXBar &xbar;
 
+        /** A normal packet queue used to store responses. */
+        RespPacketQueue queue;
+
       public:
 
         CoherentXBarSlavePort(const std::string &_name,
                              CoherentXBar &_xbar, PortID _id)
-            : SlavePort(_name, &_xbar, _id), xbar(_xbar)
+            : QueuedSlavePort(_name, &_xbar, queue, _id), xbar(_xbar),
+              queue(_xbar, *this)
         { }
 
       protected:
@@ -126,12 +130,6 @@ class CoherentXBar : public BaseXBar
         { xbar.recvFunctional(pkt, id); }
 
         /**
-         * When receiving a retry, pass it to the crossbar.
-         */
-        virtual void recvRespRetry()
-        { panic("Crossbar slave ports should never retry.\n"); }
-
-        /**
          * Return the union of all adress ranges seen by this crossbar.
          */
         virtual AddrRangeList getAddrRanges() const
@@ -215,14 +213,14 @@ class CoherentXBar : public BaseXBar
       private:
 
         /** The port which we mirror internally. */
-        SlavePort& slavePort;
+        QueuedSlavePort& slavePort;
 
       public:
 
         /**
          * Create a snoop response port that mirrors a given slave port.
          */
-        SnoopRespPort(SlavePort& slave_port, CoherentXBar& _xbar) :
+        SnoopRespPort(QueuedSlavePort& slave_port, CoherentXBar& _xbar) :
             MasterPort(slave_port.name() + ".snoopRespPort", &_xbar),
             slavePort(slave_port) { }
 
@@ -253,7 +251,7 @@ class CoherentXBar : public BaseXBar
 
     std::vector<SnoopRespPort*> snoopRespPorts;
 
-    std::vector<SlavePort*> snoopPorts;
+    std::vector<QueuedSlavePort*> snoopPorts;
 
     /**
      * Store the outstanding requests that we are expecting snoop
@@ -324,7 +322,7 @@ class CoherentXBar : public BaseXBar
      * @param dests Vector of destination ports for the forwarded pkt
      */
     void forwardTiming(PacketPtr pkt, PortID exclude_slave_port_id,
-                       const std::vector<SlavePort*>& dests);
+                       const std::vector<QueuedSlavePort*>& dests);
 
     /** Function called by the port when the crossbar is recieving a Atomic
       transaction.*/
@@ -347,7 +345,8 @@ class CoherentXBar : public BaseXBar
     std::pair<MemCmd, Tick> forwardAtomic(PacketPtr pkt,
                                           PortID exclude_slave_port_id)
     {
-        return forwardAtomic(pkt, exclude_slave_port_id, InvalidPortID, snoopPorts);
+        return forwardAtomic(pkt, exclude_slave_port_id, InvalidPortID,
+                             snoopPorts);
     }
 
     /**
@@ -365,7 +364,8 @@ class CoherentXBar : public BaseXBar
     std::pair<MemCmd, Tick> forwardAtomic(PacketPtr pkt,
                                           PortID exclude_slave_port_id,
                                           PortID source_master_port_id,
-                                          const std::vector<SlavePort*>& dests);
+                                          const std::vector<QueuedSlavePort*>&
+                                          dests);
 
     /** Function called by the port when the crossbar is recieving a Functional
         transaction.*/
diff --git a/src/mem/noncoherent_xbar.cc b/src/mem/noncoherent_xbar.cc
index e2bc85cad..330a91f1f 100644
--- a/src/mem/noncoherent_xbar.cc
+++ b/src/mem/noncoherent_xbar.cc
@@ -82,7 +82,7 @@ NoncoherentXBar::NoncoherentXBar(const NoncoherentXBarParams *p)
     // create the slave ports, once again starting at zero
     for (int i = 0; i < p->port_slave_connection_count; ++i) {
         std::string portName = csprintf("%s.slave[%d]", name(), i);
-        SlavePort* bp = new NoncoherentXBarSlavePort(portName, *this, i);
+        QueuedSlavePort* bp = new NoncoherentXBarSlavePort(portName, *this, i);
         slavePorts.push_back(bp);
         respLayers.push_back(new RespLayer(*bp, *this,
                                            csprintf(".respLayer%d", i)));
@@ -218,12 +218,11 @@ NoncoherentXBar::recvTimingResp(PacketPtr pkt, PortID master_port_id)
     // determine how long to be crossbar layer is busy
     Tick packetFinishTime = clockEdge(Cycles(1)) + pkt->payloadDelay;
 
-    // send the packet through the destination slave port
-    bool success M5_VAR_USED = slavePorts[slave_port_id]->sendTimingResp(pkt);
-
-    // currently it is illegal to block responses... can lead to
-    // deadlock
-    assert(success);
+    // send the packet through the destination slave port, and pay for
+    // any outstanding latency
+    Tick latency = pkt->headerDelay;
+    pkt->headerDelay = 0;
+    slavePorts[slave_port_id]->schedTimingResp(pkt, curTick() + latency);
 
     // remove the request from the routing table
     routeTo.erase(route_lookup);
@@ -295,6 +294,18 @@ NoncoherentXBar::recvFunctional(PacketPtr pkt, PortID slave_port_id)
                 pkt->cmdString());
     }
 
+    // since our slave ports are queued ports we need to check them as well
+    for (const auto& p : slavePorts) {
+        // if we find a response that has the data, then the
+        // downstream caches/memories may be out of date, so simply stop
+        // here
+        if (p->checkFunctional(pkt)) {
+            if (pkt->needsResponse())
+                pkt->makeResponse();
+            return;
+        }
+    }
+
     // determine the destination port
     PortID dest_id = findPort(pkt->getAddr());
 
diff --git a/src/mem/noncoherent_xbar.hh b/src/mem/noncoherent_xbar.hh
index 64a1064ab..ffcf9797f 100644
--- a/src/mem/noncoherent_xbar.hh
+++ b/src/mem/noncoherent_xbar.hh
@@ -84,18 +84,22 @@ class NoncoherentXBar : public BaseXBar
      * will be instantiated for each of the master ports connecting to
      * the crossbar.
      */
-    class NoncoherentXBarSlavePort : public SlavePort
+    class NoncoherentXBarSlavePort : public QueuedSlavePort
     {
       private:
 
         /** A reference to the crossbar to which this port belongs. */
         NoncoherentXBar &xbar;
 
+        /** A normal packet queue used to store responses. */
+        RespPacketQueue queue;
+
       public:
 
         NoncoherentXBarSlavePort(const std::string &_name,
                                 NoncoherentXBar &_xbar, PortID _id)
-            : SlavePort(_name, &_xbar, _id), xbar(_xbar)
+            : QueuedSlavePort(_name, &_xbar, queue, _id), xbar(_xbar),
+              queue(_xbar, *this)
         { }
 
       protected:
@@ -119,12 +123,6 @@ class NoncoherentXBar : public BaseXBar
         { xbar.recvFunctional(pkt, id); }
 
         /**
-         * When receiving a retry, pass it to the crossbar.
-         */
-        virtual void recvRespRetry()
-        { panic("Crossbar slave ports should never retry.\n"); }
-
-        /**
          * Return the union of all adress ranges seen by this crossbar.
          */
         virtual AddrRangeList getAddrRanges() const
diff --git a/src/mem/snoop_filter.hh b/src/mem/snoop_filter.hh
index 88b534263..1e7add660 100755
--- a/src/mem/snoop_filter.hh
+++ b/src/mem/snoop_filter.hh
@@ -50,6 +50,7 @@
 #include "base/hashmap.hh"
 #include "mem/packet.hh"
 #include "mem/port.hh"
+#include "mem/qport.hh"
 #include "params/SnoopFilter.hh"
 #include "sim/sim_object.hh"
 #include "sim/system.hh"
@@ -85,7 +86,7 @@
  */
 class SnoopFilter : public SimObject {
   public:
-    typedef std::vector<SlavePort*> SnoopList;
+    typedef std::vector<QueuedSlavePort*> SnoopList;
 
     SnoopFilter (const SnoopFilterParams *p) : SimObject(p),
         linesize(p->system->cacheLineSize()), lookupLatency(p->lookup_latency)
@@ -98,7 +99,7 @@ class SnoopFilter : public SimObject {
      *
      * @param bus_slave_ports Vector of slave ports that the bus is attached to.
      */
-    void setSlavePorts(const std::vector<SlavePort*>& bus_slave_ports) {
+    void setSlavePorts(const SnoopList& bus_slave_ports) {
         slavePorts = bus_slave_ports;
     }
 
diff --git a/src/mem/xbar.hh b/src/mem/xbar.hh
index ed678d9d0..ede60e666 100644
--- a/src/mem/xbar.hh
+++ b/src/mem/xbar.hh
@@ -57,6 +57,7 @@
 #include "base/hashmap.hh"
 #include "base/types.hh"
 #include "mem/mem_object.hh"
+#include "mem/qport.hh"
 #include "params/BaseXBar.hh"
 #include "sim/stats.hh"
 
@@ -427,7 +428,7 @@ class BaseXBar : public MemObject
     bool gotAllAddrRanges;
 
     /** The master and slave ports of the crossbar */
-    std::vector<SlavePort*> slavePorts;
+    std::vector<QueuedSlavePort*> slavePorts;
     std::vector<MasterPort*> masterPorts;
 
     /** Port that handles requests that don't match any of the interfaces.*/