From 860155a5fc48f983e9af40c19bf8db8250709c26 Mon Sep 17 00:00:00 2001 From: Andreas Hansson Date: Tue, 19 Feb 2013 05:56:06 -0500 Subject: [PATCH] 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. --- src/dev/io_device.cc | 3 +++ src/dev/pcidev.cc | 2 ++ src/dev/x86/intdev.hh | 2 ++ src/mem/bridge.cc | 7 +++++++ src/mem/bus.cc | 7 +++++++ src/mem/cache/cache_impl.hh | 20 ++++++++++++++++++++ src/mem/coherent_bus.cc | 3 +++ src/mem/noncoherent_bus.cc | 3 +++ src/mem/simple_dram.cc | 3 +++ src/mem/simple_mem.cc | 3 +++ 10 files changed, 53 insertions(+) diff --git a/src/dev/io_device.cc b/src/dev/io_device.cc index 988f8344a..6f76f4f27 100644 --- 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 592852e29..af78f5180 100644 --- 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 a32182a92..a94ca47cc 100644 --- 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 bfe7e795c..1a8437aa1 100644 --- 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 690d85373..1de1ac1e3 100644 --- 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 8fd28728b..b30132748 100644 --- a/src/mem/cache/cache_impl.hh +++ b/src/mem/cache/cache_impl.hh @@ -381,6 +381,8 @@ Cache::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::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::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::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::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::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::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 b57484ab3..0166872a7 100644 --- 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 4f6751512..f0955bb8f 100644 --- 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 32a13eef0..d822fbeff 100644 --- 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 7dd0fd101..3492360cd 100644 --- 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