changeset 19a76cedd4ea in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=19a76cedd4ea
description:
        mem: Separate the two snoop response cases in the bus

        This patch makes the flow control and state updates of the coherent
        bus more clear by separating the two cases, i.e. forward as a snoop
        response, or turn it into a normal response.

        With this change it is also more clear what resources are being
        occupied, and that we effectively bypass the busy check for the second
        case. As a result of the change in resource usage some stats change.

diffstat:

 src/mem/bus.cc          |   3 +--
 src/mem/bus.hh          |   3 +--
 src/mem/coherent_bus.cc |  49 +++++++++++++++++++++++++++++--------------------
 3 files changed, 31 insertions(+), 24 deletions(-)

diffs (124 lines):

diff -r d5a97bfa8569 -r 19a76cedd4ea src/mem/bus.cc
--- a/src/mem/bus.cc    Thu May 30 12:53:59 2013 -0400
+++ b/src/mem/bus.cc    Thu May 30 12:54:00 2013 -0400
@@ -195,8 +195,7 @@
     // if the destination port is already engaged in a transaction
     // waiting for a retry from the peer
     if (state == BUSY || (state == RETRY && port != retryingPort) ||
-        (dest_port_id != InvalidPortID &&
-         waitingForPeer[dest_port_id] != NULL)) {
+        waitingForPeer[dest_port_id] != NULL) {
         // put the port at the end of the retry list waiting for the
         // layer to be freed up (and in the case of a busy peer, for
         // that transaction to go through, and then the bus to free
diff -r d5a97bfa8569 -r 19a76cedd4ea src/mem/bus.hh
--- a/src/mem/bus.hh    Thu May 30 12:53:59 2013 -0400
+++ b/src/mem/bus.hh    Thu May 30 12:54:00 2013 -0400
@@ -130,8 +130,7 @@
          * Determine if the bus layer accepts a packet from a specific
          * port. If not, the port in question is also added to the
          * retry list. In either case the state of the layer is
-         * updated accordingly. To ignore checking the destination
-         * port (used by snoops), pass InvalidPortID.
+         * updated accordingly.
          *
          * @param port Source port presenting the packet
          * @param dest_port_id Destination port id
diff -r d5a97bfa8569 -r 19a76cedd4ea src/mem/coherent_bus.cc
--- a/src/mem/coherent_bus.cc   Thu May 30 12:53:59 2013 -0400
+++ b/src/mem/coherent_bus.cc   Thu May 30 12:54:00 2013 -0400
@@ -304,17 +304,27 @@
     // determine the source port based on the id
     SlavePort* src_port = slavePorts[slave_port_id];
 
+    // get the destination from the packet
+    PortID dest_port_id = pkt->getDest();
+
+    // 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
+    bool forwardAsSnoop = outstandingReq.find(pkt->req) ==
+        outstandingReq.end();
+
     // test if the bus should be considered occupied for the current
-    // port, do not use the destination port in the check as we do not
-    // know yet if it is to be passed on as a snoop response or normal
-    // response and we never block on either
-    if (!snoopRespLayer.tryTiming(src_port, InvalidPortID)) {
+    // port, note that the check is bypassed if the response is being
+    // passed on as a normal response since this is occupying the
+    // response layer rather than the snoop response layer
+    if (forwardAsSnoop && !snoopRespLayer.tryTiming(src_port, dest_port_id)) {
         DPRINTF(CoherentBus, "recvTimingSnoopResp: src %s %s 0x%x BUSY\n",
                 src_port->name(), pkt->cmdString(), pkt->getAddr());
         return false;
     }
 
-    DPRINTF(CoherentBus, "recvTimingSnoop: src %s %s 0x%x\n",
+    DPRINTF(CoherentBus, "recvTimingSnoopResp: src %s %s 0x%x\n",
             src_port->name(), pkt->cmdString(), pkt->getAddr());
 
     // store size and command as they might be modified when
@@ -322,28 +332,24 @@
     unsigned int pkt_size = pkt->hasData() ? pkt->getSize() : 0;
     unsigned int pkt_cmd = pkt->cmdToIndex();
 
-    // get the destination from the packet
-    PortID dest_port_id = pkt->getDest();
-
     // responses are never express snoops
     assert(!pkt->isExpressSnoop());
 
     calcPacketTiming(pkt);
     Tick packetFinishTime = pkt->busLastWordDelay + curTick();
 
-    // 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
+    // forward it either as a snoop response or a normal response
+    if (forwardAsSnoop) {
+        // this is a snoop response to a snoop request we forwarded,
+        // e.g. coming from the L1 and going to the L2, and it should
+        // be forwarded as a snoop response
         bool success M5_VAR_USED =
             masterPorts[dest_port_id]->sendTimingSnoopResp(pkt);
         pktCount[slave_port_id][dest_port_id]++;
         totPktSize[slave_port_id][dest_port_id] += pkt_size;
         assert(success);
+
+        snoopRespLayer.succeededTiming(packetFinishTime);
     } else {
         // we got a snoop response on one of our slave ports,
         // i.e. from a coherent master connected to the bus, and
@@ -358,18 +364,21 @@
         // original request came from
         assert(slave_port_id != dest_port_id);
 
-        // as a normal response, it should go back to a master
-        // through one of our slave ports
+        // 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);
     }
 
-    snoopRespLayer.succeededTiming(packetFinishTime);
-
     // stats updates
     transDist[pkt_cmd]++;
     snoopDataThroughBus += pkt_size;
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to