MEM: Separate requests and responses for timing accesses

This patch moves send/recvTiming and send/recvTimingSnoop from the
Port base class to the MasterPort and SlavePort, and also splits them
into separate member functions for requests and responses:
send/recvTimingReq, send/recvTimingResp, and send/recvTimingSnoopReq,
send/recvTimingSnoopResp. A master port sends requests and receives
responses, and also receives snoop requests and sends snoop
responses. A slave port has the reciprocal behaviour as it receives
requests and sends responses, and sends snoop requests and receives
snoop responses.

For all MemObjects that have only master ports or slave ports (but not
both), e.g. a CPU, or a PIO device, this patch merely adds more
clarity to what kind of access is taking place. For example, a CPU
port used to call sendTiming, and will now call
sendTimingReq. Similarly, a response previously came back through
recvTiming, which is now recvTimingResp. For the modules that have
both master and slave ports, e.g. the bus, the behaviour was
previously relying on branches based on pkt->isRequest(), and this is
now replaced with a direct call to the apprioriate member function
depending on the type of access. Please note that send/recvRetry is
still shared by all the timing accessors and remains in the Port base
class for now (to maintain the current bus functionality and avoid
changing the statistics of all regressions).

The packet queue is split into a MasterPort and SlavePort version to
facilitate the use of the new timing accessors. All uses of the
PacketQueue are updated accordingly.

With this patch, the type of packet (request or response) is now well
defined for each type of access, and asserts on pkt->isRequest() and
pkt->isResponse() are now moved to the appropriate send member
functions. It is also worth noting that sendTimingSnoopReq no longer
returns a boolean, as the semantics do not alow snoop requests to be
rejected or stalled. All these assumptions are now excplicitly part of
the port interface itself.

1 files changed, 147 insertions, 147 deletions

diff --git a/src/mem/bus.cc b/src/mem/bus.cc
index 911276f75..61b84d82e 100644
--- a/src/mem/bus.cc
+++ b/src/mem/bus.cc
@@ -198,186 +198,199 @@ Bus::isOccupied(PacketPtr pkt, Port* port)
 }
 
 bool
-Bus::recvTiming(PacketPtr pkt)
+Bus::recvTimingReq(PacketPtr pkt)
 {
-    // get the source id
-    Packet::NodeID src_id = pkt->getSrc();
-
-    // determine the source port based on the id and direction
-    Port *src_port = NULL;
-    if (pkt->isRequest())
-        src_port = slavePorts[src_id];
-    else
-        src_port = masterPorts[src_id];
+    // determine the source port based on the id
+    SlavePort *src_port = slavePorts[pkt->getSrc()];
 
     // test if the bus should be considered occupied for the current
     // packet, and exclude express snoops from the check
     if (!pkt->isExpressSnoop() && isOccupied(pkt, src_port)) {
-        DPRINTF(Bus, "recvTiming: src %s %s 0x%x BUSY\n",
+        DPRINTF(Bus, "recvTimingReq: src %s %s 0x%x BUSY\n",
                 src_port->name(), pkt->cmdString(), pkt->getAddr());
         return false;
     }
 
-    DPRINTF(Bus, "recvTiming: src %s %s 0x%x\n",
+    DPRINTF(Bus, "recvTimingReq: src %s %s 0x%x\n",
             src_port->name(), pkt->cmdString(), pkt->getAddr());
 
     Tick headerFinishTime = pkt->isExpressSnoop() ? 0 : calcPacketTiming(pkt);
     Tick packetFinishTime = pkt->isExpressSnoop() ? 0 : pkt->finishTime;
 
-    // decide what to do based on the direction
-    if (pkt->isRequest()) {
-        // the packet is a memory-mapped request and should be
-        // broadcasted to our snoopers but the source
-        forwardTiming(pkt, src_id);
-
-        // remember if we add an outstanding req so we can undo it if
-        // necessary, if the packet needs a response, we should add it
-        // as outstanding and express snoops never fail so there is
-        // not need to worry about them
-        bool add_outstanding = !pkt->isExpressSnoop() && pkt->needsResponse();
-
-        // keep track that we have an outstanding request packet
-        // matching this request, this is used by the coherency
-        // mechanism in determining what to do with snoop responses
-        // (in recvTimingSnoop)
-        if (add_outstanding) {
-            // we should never have an exsiting request outstanding
-            assert(outstandingReq.find(pkt->req) == outstandingReq.end());
-            outstandingReq.insert(pkt->req);
-        }
+    // the packet is a memory-mapped request and should be
+    // broadcasted to our snoopers but the source
+    forwardTiming(pkt, pkt->getSrc());
+
+    // remember if we add an outstanding req so we can undo it if
+    // necessary, if the packet needs a response, we should add it
+    // as outstanding and express snoops never fail so there is
+    // not need to worry about them
+    bool add_outstanding = !pkt->isExpressSnoop() && pkt->needsResponse();
+
+    // keep track that we have an outstanding request packet
+    // matching this request, this is used by the coherency
+    // mechanism in determining what to do with snoop responses
+    // (in recvTimingSnoop)
+    if (add_outstanding) {
+        // we should never have an exsiting request outstanding
+        assert(outstandingReq.find(pkt->req) == outstandingReq.end());
+        outstandingReq.insert(pkt->req);
+    }
+
+    // since it is a normal request, determine the destination
+    // based on the address and attempt to send the packet
+    bool success = masterPorts[findPort(pkt->getAddr())]->sendTimingReq(pkt);
+
+    if (!success)  {
+        // inhibited packets should never be forced to retry
+        assert(!pkt->memInhibitAsserted());
+
+        // if it was added as outstanding and the send failed, then
+        // erase it again
+        if (add_outstanding)
+            outstandingReq.erase(pkt->req);
 
-        // since it is a normal request, determine the destination
-        // based on the address and attempt to send the packet
-        bool success = masterPorts[findPort(pkt->getAddr())]->sendTiming(pkt);
+        DPRINTF(Bus, "recvTimingReq: src %s %s 0x%x RETRY\n",
+                src_port->name(), pkt->cmdString(), pkt->getAddr());
 
-        if (!success)  {
-            // inhibited packets should never be forced to retry
-            assert(!pkt->memInhibitAsserted());
+        addToRetryList(src_port);
+        occupyBus(headerFinishTime);
 
-            // if it was added as outstanding and the send failed, then
-            // erase it again
-            if (add_outstanding)
-                outstandingReq.erase(pkt->req);
+        return false;
+    }
 
-            DPRINTF(Bus, "recvTiming: src %s %s 0x%x RETRY\n",
-                    src_port->name(), pkt->cmdString(), pkt->getAddr());
+    succeededTiming(packetFinishTime);
 
-            addToRetryList(src_port);
-            occupyBus(headerFinishTime);
+    return true;
+}
 
-            return false;
-        }
-    } else {
-        // the packet is a normal response to a request that we should
-        // have seen passing through the bus
-        assert(outstandingReq.find(pkt->req) != outstandingReq.end());
+bool
+Bus::recvTimingResp(PacketPtr pkt)
+{
+    // determine the source port based on the id
+    MasterPort *src_port = masterPorts[pkt->getSrc()];
 
-        // remove it as outstanding
-        outstandingReq.erase(pkt->req);
+    // test if the bus should be considered occupied for the current
+    // packet
+    if (isOccupied(pkt, src_port)) {
+        DPRINTF(Bus, "recvTimingResp: src %s %s 0x%x BUSY\n",
+                src_port->name(), pkt->cmdString(), pkt->getAddr());
+        return false;
+    }
 
-        // send the packet to the destination through one of our slave
-        // ports, as determined by the destination field
-        bool success M5_VAR_USED = slavePorts[pkt->getDest()]->sendTiming(pkt);
+    DPRINTF(Bus, "recvTimingResp: src %s %s 0x%x\n",
+            src_port->name(), pkt->cmdString(), pkt->getAddr());
 
-        // currently it is illegal to block responses... can lead to
-        // deadlock
-        assert(success);
-    }
+    calcPacketTiming(pkt);
+    Tick packetFinishTime = pkt->finishTime;
+
+    // the packet is a normal response to a request that we should
+    // have seen passing through the bus
+    assert(outstandingReq.find(pkt->req) != outstandingReq.end());
+
+    // remove it as outstanding
+    outstandingReq.erase(pkt->req);
+
+    // send the packet to the destination through one of our slave
+    // ports, as determined by the destination field
+    bool success M5_VAR_USED = slavePorts[pkt->getDest()]->sendTimingResp(pkt);
+
+    // currently it is illegal to block responses... can lead to
+    // deadlock
+    assert(success);
 
     succeededTiming(packetFinishTime);
 
     return true;
 }
 
-bool
-Bus::recvTimingSnoop(PacketPtr pkt)
+void
+Bus::recvTimingSnoopReq(PacketPtr pkt)
 {
-    // get the source id
-    Packet::NodeID src_id = pkt->getSrc();
+    DPRINTF(Bus, "recvTimingSnoopReq: src %s %s 0x%x\n",
+            masterPorts[pkt->getSrc()]->name(), pkt->cmdString(),
+            pkt->getAddr());
 
-    if (pkt->isRequest()) {
-        DPRINTF(Bus, "recvTimingSnoop: src %d %s 0x%x\n",
-                src_id, pkt->cmdString(), pkt->getAddr());
+    // we should only see express snoops from caches
+    assert(pkt->isExpressSnoop());
 
-        // the packet is an express snoop request and should be
-        // broadcasted to our snoopers
-        assert(pkt->isExpressSnoop());
+    // forward to all snoopers
+    forwardTiming(pkt, Port::INVALID_PORT_ID);
 
-        // forward to all snoopers
-        forwardTiming(pkt, Port::INVALID_PORT_ID);
+    // a snoop request came from a connected slave device (one of
+    // our master ports), and if it is not coming from the slave
+    // device responsible for the address range something is
+    // wrong, hence there is nothing further to do as the packet
+    // would be going back to where it came from
+    assert(pkt->getSrc() == findPort(pkt->getAddr()));
 
-        // a snoop request came from a connected slave device (one of
-        // our master ports), and if it is not coming from the slave
-        // device responsible for the address range something is
-        // wrong, hence there is nothing further to do as the packet
-        // would be going back to where it came from
-        assert(src_id == findPort(pkt->getAddr()));
+    // this is an express snoop and is never forced to retry
+    assert(!inRetry);
+}
 
-        // this is an express snoop and is never forced to retry
-        assert(!inRetry);
+bool
+Bus::recvTimingSnoopResp(PacketPtr pkt)
+{
+    // determine the source port based on the id
+    SlavePort* src_port = slavePorts[pkt->getSrc()];
 
-        return true;
-    } else {
-        // determine the source port based on the id
-        SlavePort* src_port = slavePorts[src_id];
+    if (isOccupied(pkt, src_port)) {
+        DPRINTF(Bus, "recvTimingSnoopResp: src %s %s 0x%x BUSY\n",
+                src_port->name(), pkt->cmdString(), pkt->getAddr());
+        return false;
+    }
 
-        if (isOccupied(pkt, src_port)) {
-            DPRINTF(Bus, "recvTimingSnoop: src %s %s 0x%x BUSY\n",
-                    src_port->name(), pkt->cmdString(), pkt->getAddr());
-            return false;
-        }
+    DPRINTF(Bus, "recvTimingSnoop: src %s %s 0x%x\n",
+            src_port->name(), pkt->cmdString(), pkt->getAddr());
 
-        DPRINTF(Bus, "recvTimingSnoop: src %s %s 0x%x\n",
-                src_port->name(), pkt->cmdString(), pkt->getAddr());
+    // get the destination from the packet
+    Packet::NodeID dest = pkt->getDest();
 
-        // get the destination from the packet
-        Packet::NodeID dest = pkt->getDest();
-
-        // responses are never express snoops
-        assert(!pkt->isExpressSnoop());
-
-        calcPacketTiming(pkt);
-        Tick packetFinishTime = pkt->finishTime;
-
-        // determine if the response is from a snoop request we
-        // created as the result of a normal request (in which case it
-        // should be in the outstandingReq), or if we merely forwarded
-        // someone else's snoop request
-        if (outstandingReq.find(pkt->req) == outstandingReq.end()) {
-            // this is a snoop response to a snoop request we
-            // forwarded, e.g. coming from the L1 and going to the L2
-            // this should be forwarded as a snoop response
-            bool success M5_VAR_USED = masterPorts[dest]->sendTimingSnoop(pkt);
-            assert(success);
-        } else {
-            // we got a snoop response on one of our slave ports,
-            // i.e. from a coherent master connected to the bus, and
-            // since we created the snoop request as part of
-            // recvTiming, this should now be a normal response again
-            outstandingReq.erase(pkt->req);
+    // responses are never express snoops
+    assert(!pkt->isExpressSnoop());
 
-            // this is a snoop response from a coherent master, with a
-            // destination field set on its way through the bus as
-            // request, hence it should never go back to where the
-            // snoop response came from, but instead to where the
-            // original request came from
-            assert(src_id != dest);
+    calcPacketTiming(pkt);
+    Tick packetFinishTime = pkt->finishTime;
 
-            // as a normal response, it should go back to a master
-            // through one of our slave ports
-            bool success M5_VAR_USED = slavePorts[dest]->sendTiming(pkt);
+    // determine if the response is from a snoop request we
+    // created as the result of a normal request (in which case it
+    // should be in the outstandingReq), or if we merely forwarded
+    // someone else's snoop request
+    if (outstandingReq.find(pkt->req) == outstandingReq.end()) {
+        // this is a snoop response to a snoop request we
+        // forwarded, e.g. coming from the L1 and going to the L2
+        // this should be forwarded as a snoop response
+        bool success M5_VAR_USED = masterPorts[dest]->sendTimingSnoopResp(pkt);
+        assert(success);
+    } else {
+        // we got a snoop response on one of our slave ports,
+        // i.e. from a coherent master connected to the bus, and
+        // since we created the snoop request as part of
+        // recvTiming, this should now be a normal response again
+        outstandingReq.erase(pkt->req);
 
-            // currently it is illegal to block responses... can lead
-            // to deadlock
-            assert(success);
-        }
+        // this is a snoop response from a coherent master, with a
+        // destination field set on its way through the bus as
+        // request, hence it should never go back to where the
+        // snoop response came from, but instead to where the
+        // original request came from
+        assert(pkt->getSrc() != dest);
 
-        succeededTiming(packetFinishTime);
+        // as a normal response, it should go back to a master
+        // through one of our slave ports
+        bool success M5_VAR_USED = slavePorts[dest]->sendTimingResp(pkt);
 
-        return true;
+        // currently it is illegal to block responses... can lead
+        // to deadlock
+        assert(success);
     }
+
+    succeededTiming(packetFinishTime);
+
+    return true;
 }
 
+
 void
 Bus::succeededTiming(Tick busy_time)
 {
@@ -405,8 +418,7 @@ Bus::forwardTiming(PacketPtr pkt, int exclude_slave_port_id)
         if (exclude_slave_port_id == Port::INVALID_PORT_ID ||
             p->getId() != exclude_slave_port_id) {
             // cache is not allowed to refuse snoop
-            bool success M5_VAR_USED = p->sendTimingSnoop(pkt);
-            assert(success);
+            p->sendTimingSnoopReq(pkt);
         }
     }
 }
@@ -531,9 +543,6 @@ Bus::recvAtomic(PacketPtr pkt)
             slavePorts[pkt->getSrc()]->name(), pkt->getAddr(),
             pkt->cmdString());
 
-    // we should always see a request routed based on the address
-    assert(pkt->isRequest());
-
     // forward to all snoopers but the source
     std::pair<MemCmd, Tick> snoop_result = forwardAtomic(pkt, pkt->getSrc());
     MemCmd snoop_response_cmd = snoop_result.first;
@@ -565,9 +574,6 @@ Bus::recvAtomicSnoop(PacketPtr pkt)
             masterPorts[pkt->getSrc()]->name(), pkt->getAddr(),
             pkt->cmdString());
 
-    // we should always see a request routed based on the address
-    assert(pkt->isRequest());
-
     // forward to all snoopers
     std::pair<MemCmd, Tick> snoop_result =
         forwardAtomic(pkt, Port::INVALID_PORT_ID);
@@ -637,9 +643,6 @@ Bus::recvFunctional(PacketPtr pkt)
                 pkt->cmdString());
     }
 
-    // we should always see a request routed based on the address
-    assert(pkt->isRequest());
-
     // forward to all snoopers but the source
     forwardFunctional(pkt, pkt->getSrc());
 
@@ -663,9 +666,6 @@ Bus::recvFunctionalSnoop(PacketPtr pkt)
                 pkt->cmdString());
     }
 
-    // we should always see a request routed based on the address
-    assert(pkt->isRequest());
-
     // forward to all snoopers
     forwardFunctional(pkt, Port::INVALID_PORT_ID);
 }