mem: Enforce strict use of busFirst- and busLastWordTime

This patch adds a check to ensure that the delay incurred by
the bus is not simply disregarded, but accounted for by someone. At
this point, all the modules do is to zero it out, and no additional
time is spent. This highlights where the bus timing is simply dropped
instead of being paid for.

As a follow up, the locations identified in this patch should add this
additional time to the packets in one way or another. For now it
simply acts as a sanity check and highlights where the delay is simply
ignored.

Since no time is added, all regressions remain the same.

diff --git a/src/dev/io_device.cc b/src/dev/io_device.cc
index 988f8344aafddfa458f7d2e7d20500b2fe09f204..6f76f4f27858f035604faed6fab253e3d0bbbd3d 100644 (file)
--- a/src/dev/io_device.cc
+++ b/src/dev/io_device.cc
@@ -54,6 +54,9 @@ PioPort::PioPort(PioDevice *dev)
 Tick
 PioPort::recvAtomic(PacketPtr pkt)
 {
+    // @todo: We need to pay for this and not just zero it out
+    pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
+
     return pkt->isRead() ? device->read(pkt) : device->write(pkt);
 }
 

diff --git a/src/dev/pcidev.cc b/src/dev/pcidev.cc
index 592852e293ebac273e84c1b34fdd5ee01e4d8bf9..af78f518016e78f175515221e8309f612cee30d5 100644 (file)
--- a/src/dev/pcidev.cc
+++ b/src/dev/pcidev.cc
@@ -67,6 +67,8 @@ PciDev::PciConfigPort::recvAtomic(PacketPtr pkt)
 {
     assert(pkt->getAddr() >= configAddr &&
            pkt->getAddr() < configAddr + PCI_CONFIG_SIZE);
+    // @todo someone should pay for this
+    pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
     return pkt->isRead() ? device->readConfig(pkt) : device->writeConfig(pkt);
 }
 

diff --git a/src/dev/x86/intdev.hh b/src/dev/x86/intdev.hh
index a32182a927e561e29ae803451500dfe9c4aecadc..a94ca47cc57acb9acf1ba3bf2de4cdcbe8d4dc0b 100644 (file)
--- a/src/dev/x86/intdev.hh
+++ b/src/dev/x86/intdev.hh
@@ -81,6 +81,8 @@ class IntDev
 
         Tick recvMessage(PacketPtr pkt)
         {
+            // @todo someone should pay for this
+            pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
             return device->recvMessage(pkt);
         }
     };

diff --git a/src/mem/bridge.cc b/src/mem/bridge.cc
index bfe7e795cfca65abfd77cd0e2dbc84449f71d2dc..1a8437aa156b58b799a87ae75baf451b1a57424a 100644 (file)
--- a/src/mem/bridge.cc
+++ b/src/mem/bridge.cc
@@ -141,6 +141,9 @@ Bridge::BridgeMasterPort::recvTimingResp(PacketPtr pkt)
 
     DPRINTF(Bridge, "Request queue size: %d\n", transmitList.size());
 
+    // @todo: We need to pay for this and not just zero it out
+    pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
+
     slavePort.schedTimingResp(pkt, bridge.clockEdge(delay));
 
     return true;
@@ -171,6 +174,10 @@ Bridge::BridgeSlavePort::recvTimingReq(PacketPtr pkt)
             assert(outstandingResponses != respQueueLimit);
             ++outstandingResponses;
             retryReq = false;
+
+            // @todo: We need to pay for this and not just zero it out
+            pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
+
             masterPort.schedTimingReq(pkt, bridge.clockEdge(delay));
         }
     }

diff --git a/src/mem/bus.cc b/src/mem/bus.cc
index 690d8537322ab0b48bd1597b416fbce52019971a..1de1ac1e36b6bc6a3bf4a9202c4af08abff1a7f1 100644 (file)
--- a/src/mem/bus.cc
+++ b/src/mem/bus.cc
@@ -140,6 +140,13 @@ BaseBus::calcPacketTiming(PacketPtr pkt)
     // determine how many cycles are needed to send the data
     unsigned dataCycles = pkt->hasData() ? divCeil(pkt->getSize(), width) : 0;
 
+    // before setting the bus delay fields of the packet, ensure that
+    // the delay from any previous bus has been accounted for
+    if (pkt->busFirstWordDelay != 0 || pkt->busLastWordDelay != 0)
+        panic("Packet %s already has bus delay (%d, %d) that should be "
+              "accounted for.\n", pkt->cmdString(), pkt->busFirstWordDelay,
+              pkt->busLastWordDelay);
+
     // The first word will be delivered on the cycle after the header.
     pkt->busFirstWordDelay = (headerCycles + 1) * clockPeriod() + offset;
 

diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index 8fd28728bc44cf031fce6c3ac60cf274fc59dd9c..b3013274893373383f8449a654042eb1949bfa12 100644 (file)
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -381,6 +381,8 @@ Cache<TagStore>::recvTimingSnoopResp(PacketPtr pkt)
 
     pkt->setDest(rec->prevSrc);
     delete rec;
+    // @todo someone should pay for this
+    pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
     memSidePort->schedTimingSnoopResp(pkt, time);
 }
 
@@ -419,6 +421,9 @@ Cache<TagStore>::recvTimingReq(PacketPtr pkt)
         // supplier had exclusive copy to begin with.
         if (pkt->needsExclusive() && !pkt->isSupplyExclusive()) {
             Packet *snoopPkt = new Packet(pkt, true);  // clear flags
+            // also reset the bus time that the original packet has
+            // not yet paid for
+            snoopPkt->busFirstWordDelay = snoopPkt->busLastWordDelay = 0;
             snoopPkt->setExpressSnoop();
             snoopPkt->assertMemInhibit();
             memSidePort->sendTimingReq(snoopPkt);
@@ -437,6 +442,9 @@ Cache<TagStore>::recvTimingReq(PacketPtr pkt)
     if (pkt->req->isUncacheable()) {
         uncacheableFlush(pkt);
 
+        // @todo: someone should pay for this
+        pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
+
         // writes go in write buffer, reads use MSHR
         if (pkt->isWrite() && !pkt->isRead()) {
             allocateWriteBuffer(pkt, time, true);
@@ -489,6 +497,8 @@ Cache<TagStore>::recvTimingReq(PacketPtr pkt)
 
         if (needsResponse) {
             pkt->makeTimingResponse();
+            // @todo: Make someone pay for this
+            pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
             cpuSidePort->schedTimingResp(pkt, clockEdge(lat));
         } else {
             /// @todo nominally we should just delete the packet here,
@@ -499,6 +509,9 @@ Cache<TagStore>::recvTimingReq(PacketPtr pkt)
     } else {
         // miss
 
+        // @todo: Make someone pay for this
+        pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
+
         Addr blk_addr = blockAlign(pkt->getAddr());
         MSHR *mshr = mshrQueue.findMatch(blk_addr);
 
@@ -946,6 +959,8 @@ Cache<TagStore>::recvTimingResp(PacketPtr pkt)
                 // isInvalidate() set otherwise.
                 target->pkt->cmd = MemCmd::ReadRespWithInvalidate;
             }
+            // reset the bus additional time as it is now accounted for
+            target->pkt->busFirstWordDelay = target->pkt->busLastWordDelay = 0;
             cpuSidePort->schedTimingResp(target->pkt, completion_time);
             break;
 
@@ -1250,6 +1265,8 @@ doTimingSupplyResponse(PacketPtr req_pkt, uint8_t *blk_data,
     assert(req_pkt->isInvalidate() || pkt->sharedAsserted());
     pkt->allocate();
     pkt->makeTimingResponse();
+    // @todo Make someone pay for this
+    pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
     if (pkt->isRead()) {
         pkt->setDataFromBlock(blk_data, blkSize);
     }
@@ -1293,6 +1310,9 @@ Cache<TagStore>::handleSnoop(PacketPtr pkt, BlkType *blk,
             Packet snoopPkt(pkt, true);  // clear flags
             snoopPkt.setExpressSnoop();
             snoopPkt.pushSenderState(new ForwardResponseRecord(pkt->getSrc()));
+            // the snoop packet does not need to wait any additional
+            // time
+            snoopPkt.busFirstWordDelay = snoopPkt.busLastWordDelay = 0;
             cpuSidePort->sendTimingSnoopReq(&snoopPkt);
             if (snoopPkt.memInhibitAsserted()) {
                 // cache-to-cache response from some upper cache

diff --git a/src/mem/coherent_bus.cc b/src/mem/coherent_bus.cc
index b57484ab3b3d826f546f0c13c64e7cc7a54342b3..0166872a78f2fd99c534d92fc4abf9ac30e6438b 100644 (file)
--- a/src/mem/coherent_bus.cc
+++ b/src/mem/coherent_bus.cc
@@ -179,6 +179,9 @@ CoherentBus::recvTimingReq(PacketPtr pkt, PortID slave_port_id)
             if (add_outstanding)
                 outstandingReq.erase(pkt->req);
 
+            // undo the calculation so we can check for 0 again
+            pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
+
             DPRINTF(CoherentBus, "recvTimingReq: src %s %s 0x%x RETRY\n",
                     src_port->name(), pkt->cmdString(), pkt->getAddr());
 

diff --git a/src/mem/noncoherent_bus.cc b/src/mem/noncoherent_bus.cc
index 4f6751512c926cedea316490f86e390bc414c852..f0955bb8f3ac07d781e2fad82ac4cacdd4a0f86a 100644 (file)
--- a/src/mem/noncoherent_bus.cc
+++ b/src/mem/noncoherent_bus.cc
@@ -124,6 +124,9 @@ NoncoherentBus::recvTimingReq(PacketPtr pkt, PortID slave_port_id)
         DPRINTF(NoncoherentBus, "recvTimingReq: src %s %s 0x%x RETRY\n",
                 src_port->name(), pkt->cmdString(), pkt->getAddr());
 
+        // undo the calculation so we can check for 0 again
+        pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
+
         // occupy until the header is sent
         reqLayer.failedTiming(src_port, clockEdge(Cycles(headerCycles)));
 

diff --git a/src/mem/simple_dram.cc b/src/mem/simple_dram.cc
index 32a13eef01e0eef75f490e6c1415f7205604e209..d822fbeffbfdcce9efb65ff8f2eeeea55e2e92f3 100644 (file)
--- a/src/mem/simple_dram.cc
+++ b/src/mem/simple_dram.cc
@@ -741,6 +741,9 @@ SimpleDRAM::accessAndRespond(PacketPtr pkt)
         // access already turned the packet into a response
         assert(pkt->isResponse());
 
+        // @todo someone should pay for this
+        pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
+
         // queue the packet in the response queue to be sent out the
         // next tick
         port.schedTimingResp(pkt, curTick() + 1);

diff --git a/src/mem/simple_mem.cc b/src/mem/simple_mem.cc
index 7dd0fd1011d55d98863d67749534498faee18e9d..3492360cdc4cd898a6ebd84161a2d7820c529749 100644 (file)
--- a/src/mem/simple_mem.cc
+++ b/src/mem/simple_mem.cc
@@ -121,6 +121,9 @@ SimpleMemory::recvTimingReq(PacketPtr pkt)
         return false;
     }
 
+    // @todo someone should pay for this
+    pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
+
     // update the release time according to the bandwidth limit, and
     // do so with respect to the time it takes to finish this request
     // rather than long term as it is the short term data rate that is