From 6ab53415efe3e06c06589a8a6ef38185ff6f94b7 Mon Sep 17 00:00:00 2001 From: Steve Reinhardt Date: Sat, 30 Jun 2007 10:16:18 -0700 Subject: Get rid of Packet result field. Error responses are now encoded in cmd field. --HG-- extra : convert_revision : d67819b7e3ee4b9a5bf08541104de0a89485e90b --- src/mem/bridge.cc | 17 +++-- src/mem/bridge.hh | 2 +- src/mem/bus.cc | 30 ++++---- src/mem/cache/base_cache.cc | 2 +- src/mem/cache/cache_impl.hh | 12 ++-- src/mem/packet.cc | 19 +++-- src/mem/packet.hh | 169 ++++++++++++++++++++------------------------ src/mem/physical.cc | 2 +- src/mem/port.cc | 3 +- src/mem/tport.cc | 21 ++++-- 10 files changed, 128 insertions(+), 149 deletions(-) (limited to 'src/mem') diff --git a/src/mem/bridge.cc b/src/mem/bridge.cc index fb4574844..77178d518 100644 --- a/src/mem/bridge.cc +++ b/src/mem/bridge.cc @@ -121,14 +121,13 @@ Bridge::BridgePort::recvTiming(PacketPtr pkt) otherPort->sendQueue.size(), otherPort->queuedRequests, otherPort->outstandingResponses); - if (pkt->isRequest() && otherPort->reqQueueFull() && pkt->result != - Packet::Nacked) { + if (pkt->isRequest() && otherPort->reqQueueFull() && !pkt->wasNacked()) { DPRINTF(BusBridge, "Remote queue full, nacking\n"); nackRequest(pkt); return true; } - if (pkt->needsResponse() && pkt->result != Packet::Nacked) + if (pkt->needsResponse() && !pkt->wasNacked()) if (respQueueFull()) { DPRINTF(BusBridge, "Local queue full, no space for response, nacking\n"); DPRINTF(BusBridge, "queue size: %d outreq: %d outstanding resp: %d\n", @@ -149,7 +148,7 @@ void Bridge::BridgePort::nackRequest(PacketPtr pkt) { // Nack the packet - pkt->result = Packet::Nacked; + pkt->setNacked(); pkt->setDest(pkt->getSrc()); //put it on the list to send @@ -194,7 +193,7 @@ Bridge::BridgePort::nackRequest(PacketPtr pkt) void Bridge::BridgePort::queueForSendTiming(PacketPtr pkt) { - if (pkt->isResponse() || pkt->result == Packet::Nacked) { + if (pkt->isResponse() || pkt->wasNacked()) { // This is a response for a request we forwarded earlier. The // corresponding PacketBuffer should be stored in the packet's // senderState field. @@ -206,7 +205,7 @@ Bridge::BridgePort::queueForSendTiming(PacketPtr pkt) // Check if this packet was expecting a response and it's a nacked // packet, in which case we will never being seeing it - if (buf->expectResponse && pkt->result == Packet::Nacked) + if (buf->expectResponse && pkt->wasNacked()) --outstandingResponses; @@ -217,7 +216,7 @@ Bridge::BridgePort::queueForSendTiming(PacketPtr pkt) } - if (pkt->isRequest() && pkt->result != Packet::Nacked) { + if (pkt->isRequest() && !pkt->wasNacked()) { ++queuedRequests; } @@ -251,7 +250,7 @@ Bridge::BridgePort::trySend() // Ugly! @todo When multilevel coherence works this will be removed if (pkt->cmd == MemCmd::WriteInvalidateReq && fixPartialWrite && - pkt->result != Packet::Nacked) { + !pkt->wasNacked()) { PacketPtr funcPkt = new Packet(pkt->req, MemCmd::WriteReq, Packet::Broadcast); funcPkt->dataStatic(pkt->getPtr()); @@ -264,7 +263,7 @@ Bridge::BridgePort::trySend() buf->origSrc, pkt->getDest(), pkt->getAddr()); bool wasReq = pkt->isRequest(); - bool wasNacked = pkt->result == Packet::Nacked; + bool wasNacked = pkt->wasNacked(); if (sendTiming(pkt)) { // send successful diff --git a/src/mem/bridge.hh b/src/mem/bridge.hh index 89d626611..7af764437 100644 --- a/src/mem/bridge.hh +++ b/src/mem/bridge.hh @@ -86,7 +86,7 @@ class Bridge : public MemObject expectResponse(_pkt->needsResponse() && !nack) { - if (!pkt->isResponse() && !nack && pkt->result != Packet::Nacked) + if (!pkt->isResponse() && !nack && !pkt->wasNacked()) pkt->senderState = this; } diff --git a/src/mem/bus.cc b/src/mem/bus.cc index ffd5e25a7..83ce0f87d 100644 --- a/src/mem/bus.cc +++ b/src/mem/bus.cc @@ -173,9 +173,8 @@ bool Bus::recvTiming(PacketPtr pkt) { Port *port; - DPRINTF(Bus, "recvTiming: packet src %d dest %d addr 0x%x cmd %s result %d\n", - pkt->getSrc(), pkt->getDest(), pkt->getAddr(), pkt->cmdString(), - pkt->result); + DPRINTF(Bus, "recvTiming: packet src %d dest %d addr 0x%x cmd %s\n", + pkt->getSrc(), pkt->getDest(), pkt->getAddr(), pkt->cmdString()); BusPort *pktPort; if (pkt->getSrc() == defaultId) @@ -329,6 +328,8 @@ Bus::functionalSnoop(PacketPtr pkt, Port *responder) // id after each int src_id = pkt->getSrc(); + assert(pkt->isRequest()); // hasn't already been satisfied + for (SnoopIter s_iter = snoopPorts.begin(); s_iter != snoopPorts.end(); s_iter++) { @@ -336,7 +337,7 @@ Bus::functionalSnoop(PacketPtr pkt, Port *responder) if (p != responder && p->getId() != src_id) { p->sendFunctional(pkt); } - if (pkt->result == Packet::Success) { + if (pkt->isResponse()) { break; } pkt->setSrc(src_id); @@ -369,14 +370,15 @@ Bus::recvAtomic(PacketPtr pkt) DPRINTF(Bus, "recvAtomic: packet src %d dest %d addr 0x%x cmd %s\n", pkt->getSrc(), pkt->getDest(), pkt->getAddr(), pkt->cmdString()); assert(pkt->getDest() == Packet::Broadcast); + assert(pkt->isRequest()); // Variables for recording original command and snoop response (if // any)... if a snooper respondes, we will need to restore // original command so that additional snoops can take place // properly MemCmd orig_cmd = pkt->cmd; - Packet::Result response_result = Packet::Unknown; MemCmd response_cmd = MemCmd::InvalidCmd; + int orig_src = pkt->getSrc(); Port *target_port = findPort(pkt->getAddr(), pkt->getSrc()); @@ -387,20 +389,18 @@ Bus::recvAtomic(PacketPtr pkt) assert(p != target_port); if (p->getId() != pkt->getSrc()) { p->sendAtomic(pkt); - if (pkt->result != Packet::Unknown) { + if (pkt->isResponse()) { // response from snoop agent assert(pkt->cmd != orig_cmd); assert(pkt->memInhibitAsserted()); - assert(pkt->isResponse()); // should only happen once - assert(response_result == Packet::Unknown); assert(response_cmd == MemCmd::InvalidCmd); // save response state - response_result = pkt->result; response_cmd = pkt->cmd; // restore original packet state for remaining snoopers pkt->cmd = orig_cmd; - pkt->result = Packet::Unknown; + pkt->setSrc(orig_src); + pkt->setDest(Packet::Broadcast); } } } @@ -408,13 +408,11 @@ Bus::recvAtomic(PacketPtr pkt) Tick response_time = target_port->sendAtomic(pkt); // if we got a response from a snooper, restore it here - if (response_result != Packet::Unknown) { - assert(response_cmd != MemCmd::InvalidCmd); + if (response_cmd != MemCmd::InvalidCmd) { // no one else should have responded - assert(pkt->result == Packet::Unknown); + assert(!pkt->isResponse()); assert(pkt->cmd == orig_cmd); pkt->cmd = response_cmd; - pkt->result = response_result; } // why do we have this packet field and the return value both??? @@ -434,8 +432,8 @@ Bus::recvFunctional(PacketPtr pkt) Port* port = findPort(pkt->getAddr(), pkt->getSrc()); functionalSnoop(pkt, port ? port : interfaces[pkt->getSrc()]); - // If the snooping found what we were looking for, we're done. - if (pkt->result != Packet::Success && port) { + // If the snooping hasn't found what we were looking for, keep going. + if (!pkt->isResponse() && port) { port->sendFunctional(pkt); } } diff --git a/src/mem/cache/base_cache.cc b/src/mem/cache/base_cache.cc index 5062d6e87..870658675 100644 --- a/src/mem/cache/base_cache.cc +++ b/src/mem/cache/base_cache.cc @@ -82,7 +82,7 @@ void BaseCache::CachePort::checkAndSendFunctional(PacketPtr pkt) { checkFunctional(pkt); - if (pkt->result != Packet::Success) + if (!pkt->isResponse()) sendFunctional(pkt); } diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh index b76d7e392..1823ea6b9 100644 --- a/src/mem/cache/cache_impl.hh +++ b/src/mem/cache/cache_impl.hh @@ -502,7 +502,6 @@ Cache::atomicAccess(PacketPtr pkt) if (pkt->needsResponse()) { pkt->makeAtomicResponse(); - pkt->result = Packet::Success; } return lat; @@ -648,14 +647,13 @@ Cache::handleResponse(PacketPtr pkt) MSHR *mshr = dynamic_cast(pkt->senderState); assert(mshr); - if (pkt->result == Packet::Nacked) { + if (pkt->wasNacked()) { //pkt->reinitFromRequest(); warn("NACKs from devices not connected to the same bus " "not implemented\n"); return; } - assert(pkt->result != Packet::BadAddress); - assert(pkt->result == Packet::Success); + assert(!pkt->isError()); DPRINTF(Cache, "Handling response to %x\n", pkt->getAddr()); MSHRQueue *mq = mshr->queue; @@ -1142,7 +1140,7 @@ void Cache::CpuSidePort::recvFunctional(PacketPtr pkt) { checkFunctional(pkt); - if (pkt->result != Packet::Success) + if (!pkt->isResponse()) myCache()->functionalAccess(pkt, cache->memSidePort); } @@ -1180,7 +1178,7 @@ Cache::MemSidePort::recvTiming(PacketPtr pkt) // this needs to be fixed so that the cache updates the mshr and sends the // packet back out on the link, but it probably won't happen so until this // gets fixed, just panic when it does - if (pkt->result == Packet::Nacked) + if (pkt->wasNacked()) panic("Need to implement cache resending nacked packets!\n"); if (pkt->isRequest() && blocked) { @@ -1216,7 +1214,7 @@ void Cache::MemSidePort::recvFunctional(PacketPtr pkt) { checkFunctional(pkt); - if (pkt->result != Packet::Success) + if (!pkt->isResponse()) myCache()->functionalAccess(pkt, cache->cpuSidePort); } diff --git a/src/mem/packet.cc b/src/mem/packet.cc index cd0ed8a2e..55fe13f3c 100644 --- a/src/mem/packet.cc +++ b/src/mem/packet.cc @@ -115,7 +115,13 @@ MemCmd::commandInfo[] = SwapResp, "SwapReq" }, /* SwapResp -- for Swap ldstub type operations */ { SET5(IsRead, IsWrite, NeedsExclusive, IsResponse, HasData), - InvalidCmd, "SwapResp" } + InvalidCmd, "SwapResp" }, + /* NetworkNackError -- nacked at network layer (not by protocol) */ + { SET2(IsRequest, IsError), InvalidCmd, "NetworkNackError" }, + /* InvalidDestError -- packet dest field invalid */ + { SET2(IsRequest, IsError), InvalidCmd, "InvalidDestError" }, + /* BadAddressError -- memory address invalid */ + { SET2(IsRequest, IsError), InvalidCmd, "BadAddressError" } }; @@ -205,7 +211,7 @@ Packet::checkFunctional(Addr addr, int size, uint8_t *data) if (func_start >= val_start && func_end <= val_end) { allocate(); std::memcpy(getPtr(), data + offset, getSize()); - result = Packet::Success; + makeResponse(); return true; } else { // In this case the timing packet only partially satisfies @@ -245,15 +251,6 @@ operator<<(std::ostream &o, const Packet &p) o << p.getAddr() + p.getSize() - 1 << "] "; o.unsetf(std::ios_base::hex| std::ios_base::showbase); - if (p.result == Packet::Success) - o << "Successful "; - if (p.result == Packet::BadAddress) - o << "BadAddress "; - if (p.result == Packet::Nacked) - o << "Nacked "; - if (p.result == Packet::Unknown) - o << "Inflight "; - if (p.isRead()) o << "Read "; if (p.isWrite()) diff --git a/src/mem/packet.hh b/src/mem/packet.hh index fc1c283ed..10b9f490c 100644 --- a/src/mem/packet.hh +++ b/src/mem/packet.hh @@ -84,6 +84,13 @@ class MemCmd StoreCondResp, SwapReq, SwapResp, + // Error responses + // @TODO these should be classified as responses rather than + // requests; coding them as requests initially for backwards + // compatibility + NetworkNackError, // nacked at network layer (not by protocol) + InvalidDestError, // packet dest field invalid + BadAddressError, // memory address invalid NUM_MEM_CMDS }; @@ -103,6 +110,7 @@ class MemCmd IsHWPrefetch, IsLocked, //!< Alpha/MIPS LL or SC access HasData, //!< There is an associated payload + IsError, //!< Error response NUM_COMMAND_ATTRIBUTES }; @@ -135,12 +143,13 @@ class MemCmd bool isWrite() const { return testCmdAttrib(IsWrite); } bool isRequest() const { return testCmdAttrib(IsRequest); } bool isResponse() const { return testCmdAttrib(IsResponse); } - bool needsExclusive() const { return testCmdAttrib(NeedsExclusive); } + bool needsExclusive() const { return testCmdAttrib(NeedsExclusive); } bool needsResponse() const { return testCmdAttrib(NeedsResponse); } bool isInvalidate() const { return testCmdAttrib(IsInvalidate); } bool hasData() const { return testCmdAttrib(HasData); } bool isReadWrite() const { return isRead() && isWrite(); } bool isLocked() const { return testCmdAttrib(IsLocked); } + bool isError() const { return testCmdAttrib(IsError); } const Command responseCommand() const { return commandInfo[cmd].response; @@ -184,6 +193,12 @@ class Packet : public FastAlloc typedef MemCmd::Command Command; + /** The command field of the packet. */ + MemCmd cmd; + + /** A pointer to the original request. */ + RequestPtr req; + private: /** A pointer to the data being transfered. It can be differnt * sizes at each level of the heirarchy so it belongs in the @@ -223,19 +238,28 @@ class Packet : public FastAlloc * (unlike * addr, size, and src). */ short dest; + /** The original value of the command field. Only valid when the + * current command field is an error condition; in that case, the + * previous contents of the command field are copied here. This + * field is *not* set on non-error responses. + */ + MemCmd origCmd; + /** Are the 'addr' and 'size' fields valid? */ bool addrSizeValid; /** Is the 'src' field valid? */ bool srcValid; + bool destValid; - enum SnoopFlag { + enum Flag { + // Snoop flags MemInhibit, Shared, - NUM_SNOOP_FLAGS + NUM_PACKET_FLAGS }; - /** Coherence snoopFlags for snooping */ - std::bitset snoopFlags; + /** Status flags */ + std::bitset flags; public: @@ -252,22 +276,6 @@ class Packet : public FastAlloc * should be routed based on its address. */ static const short Broadcast = -1; - /** A pointer to the original request. */ - RequestPtr req; - - /** A virtual base opaque structure used to hold coherence-related - * state. A specific subclass would be derived from this to - * carry state specific to a particular coherence protocol. */ - class CoherenceState : public FastAlloc { - public: - virtual ~CoherenceState() {} - }; - - /** This packet's coherence state. Caches should use - * dynamic_cast<> to cast to the state appropriate for the - * system's coherence protocol. */ - CoherenceState *coherence; - /** A virtual base opaque structure used to hold state associated * with the packet but specific to the sending device (e.g., an * MSHR). A pointer to this state is returned in the packet's @@ -284,11 +292,6 @@ class Packet : public FastAlloc * to cast to the state appropriate to the sender. */ SenderState *senderState; - public: - - /** The command field of the packet. */ - MemCmd cmd; - /** Return the string name of the cmd field (for debugging and * tracing). */ const std::string &cmdString() const { return cmd.toString(); } @@ -296,68 +299,59 @@ class Packet : public FastAlloc /** Return the index of this command. */ inline int cmdToIndex() const { return cmd.toInt(); } - public: - bool isRead() const { return cmd.isRead(); } bool isWrite() const { return cmd.isWrite(); } bool isRequest() const { return cmd.isRequest(); } bool isResponse() const { return cmd.isResponse(); } - bool needsExclusive() const { return cmd.needsExclusive(); } + bool needsExclusive() const { return cmd.needsExclusive(); } bool needsResponse() const { return cmd.needsResponse(); } bool isInvalidate() const { return cmd.isInvalidate(); } bool hasData() const { return cmd.hasData(); } bool isReadWrite() const { return cmd.isReadWrite(); } bool isLocked() const { return cmd.isLocked(); } - - void assertMemInhibit() { snoopFlags[MemInhibit] = true; } - void assertShared() { snoopFlags[Shared] = true; } - bool memInhibitAsserted() { return snoopFlags[MemInhibit]; } - bool sharedAsserted() { return snoopFlags[Shared]; } + bool isError() const { return cmd.isError(); } + + // Snoop flags + void assertMemInhibit() { flags[MemInhibit] = true; } + void assertShared() { flags[Shared] = true; } + bool memInhibitAsserted() { return flags[MemInhibit]; } + bool sharedAsserted() { return flags[Shared]; } + + // Network error conditions... encapsulate them as methods since + // their encoding keeps changing (from result field to command + // field, etc.) + void setNacked() { origCmd = cmd; cmd = MemCmd::NetworkNackError; } + void setBadAddress() { origCmd = cmd; cmd = MemCmd::BadAddressError; } + bool wasNacked() { return cmd == MemCmd::NetworkNackError; } + bool hadBadAddress() { return cmd == MemCmd::BadAddressError; } bool nic_pkt() { panic("Unimplemented"); M5_DUMMY_RETURN } - /** Possible results of a packet's request. */ - enum Result - { - Success, - BadAddress, - Nacked, - Unknown - }; - - /** The result of this packet's request. */ - Result result; - /** Accessor function that returns the source index of the packet. */ - short getSrc() const { assert(srcValid); return src; } + short getSrc() const { assert(srcValid); return src; } void setSrc(short _src) { src = _src; srcValid = true; } /** Reset source field, e.g. to retransmit packet on different bus. */ void clearSrc() { srcValid = false; } /** Accessor function that returns the destination index of the packet. */ - short getDest() const { return dest; } - void setDest(short _dest) { dest = _dest; } + short getDest() const { assert(destValid); return dest; } + void setDest(short _dest) { dest = _dest; destValid = true; } Addr getAddr() const { assert(addrSizeValid); return addr; } - int getSize() const { assert(addrSizeValid); return size; } + int getSize() const { assert(addrSizeValid); return size; } Addr getOffset(int blkSize) const { return addr & (Addr)(blkSize - 1); } - void addrOverride(Addr newAddr) { assert(addrSizeValid); addr = newAddr; } - void cmdOverride(MemCmd newCmd) { cmd = newCmd; } - /** Constructor. Note that a Request object must be constructed * first, but the Requests's physical address and size fields * need not be valid. The command and destination addresses * must be supplied. */ Packet(Request *_req, MemCmd _cmd, short _dest) - : data(NULL), staticData(false), dynamicData(false), arrayData(false), + : cmd(_cmd), req(_req), + data(NULL), staticData(false), dynamicData(false), arrayData(false), addr(_req->paddr), size(_req->size), dest(_dest), - addrSizeValid(_req->validPaddr), srcValid(false), - snoopFlags(0), - time(curTick), - req(_req), coherence(NULL), senderState(NULL), cmd(_cmd), - result(Unknown) + addrSizeValid(_req->validPaddr), srcValid(false), destValid(true), + flags(0), time(curTick), senderState(NULL) { } @@ -365,13 +359,11 @@ class Packet : public FastAlloc * a request that is for a whole block, not the address from the req. * this allows for overriding the size/addr of the req.*/ Packet(Request *_req, MemCmd _cmd, short _dest, int _blkSize) - : data(NULL), staticData(false), dynamicData(false), arrayData(false), + : cmd(_cmd), req(_req), + data(NULL), staticData(false), dynamicData(false), arrayData(false), addr(_req->paddr & ~(_blkSize - 1)), size(_blkSize), dest(_dest), - addrSizeValid(_req->validPaddr), srcValid(false), - snoopFlags(0), - time(curTick), - req(_req), coherence(NULL), senderState(NULL), cmd(_cmd), - result(Unknown) + addrSizeValid(_req->validPaddr), srcValid(false), destValid(true), + flags(0), time(curTick), senderState(NULL) { } @@ -382,15 +374,14 @@ class Packet : public FastAlloc * dynamic data, user must guarantee that the new packet's * lifetime is less than that of the original packet. */ Packet(Packet *origPkt) - : data(NULL), staticData(false), dynamicData(false), arrayData(false), + : cmd(origPkt->cmd), req(origPkt->req), + data(NULL), staticData(false), dynamicData(false), arrayData(false), addr(origPkt->addr), size(origPkt->size), src(origPkt->src), dest(origPkt->dest), - addrSizeValid(origPkt->addrSizeValid), srcValid(origPkt->srcValid), - snoopFlags(origPkt->snoopFlags), - time(curTick), - req(origPkt->req), coherence(origPkt->coherence), - senderState(origPkt->senderState), cmd(origPkt->cmd), - result(origPkt->result) + addrSizeValid(origPkt->addrSizeValid), + srcValid(origPkt->srcValid), destValid(origPkt->destValid), + flags(origPkt->flags), + time(curTick), senderState(origPkt->senderState) { } @@ -405,12 +396,11 @@ class Packet : public FastAlloc * multiple transactions. */ void reinitFromRequest() { assert(req->validPaddr); - snoopFlags = 0; + flags = 0; addr = req->paddr; size = req->size; time = req->time; addrSizeValid = true; - result = Unknown; if (dynamicData) { deleteData(); dynamicData = false; @@ -424,34 +414,24 @@ class Packet : public FastAlloc * destination fields are *not* modified, as is appropriate for * atomic accesses. */ - void makeAtomicResponse() + void makeResponse() { assert(needsResponse()); assert(isRequest()); - assert(result == Unknown); cmd = cmd.responseCommand(); - result = Success; + dest = src; + destValid = srcValid; + srcValid = false; } - /** - * Perform the additional work required for timing responses above - * and beyond atomic responses; i.e., change the destination to - * point back to the requester and clear the source field. - */ - void convertAtomicToTimingResponse() + void makeAtomicResponse() { - dest = getSrc(); - srcValid = false; + makeResponse(); } - /** - * Take a request packet and modify it in place to be suitable for - * returning as a response to a timing request. - */ void makeTimingResponse() { - makeAtomicResponse(); - convertAtomicToTimingResponse(); + makeResponse(); } /** @@ -462,9 +442,10 @@ class Packet : public FastAlloc void reinitNacked() { - assert(needsResponse() && result == Nacked); - dest = Broadcast; - result = Unknown; + assert(wasNacked()); + cmd = origCmd; + assert(needsResponse()); + setDest(Broadcast); } diff --git a/src/mem/physical.cc b/src/mem/physical.cc index 93cba96c4..2742eca51 100644 --- a/src/mem/physical.cc +++ b/src/mem/physical.cc @@ -322,7 +322,7 @@ PhysicalMemory::doFunctionalAccess(PacketPtr pkt) pkt->cmdString()); } - pkt->result = Packet::Success; + pkt->makeAtomicResponse(); } diff --git a/src/mem/port.cc b/src/mem/port.cc index e6ea773f2..ba4f23668 100644 --- a/src/mem/port.cc +++ b/src/mem/port.cc @@ -58,12 +58,11 @@ void Port::blobHelper(Addr addr, uint8_t *p, int size, MemCmd cmd) { Request req; - Packet pkt(&req, cmd, Packet::Broadcast); for (ChunkGenerator gen(addr, size, peerBlockSize()); !gen.done(); gen.next()) { req.setPhys(gen.addr(), gen.size(), 0); - pkt.reinitFromRequest(); + Packet pkt(&req, cmd, Packet::Broadcast); pkt.dataStatic(p); sendFunctional(&pkt); p += gen.size(); diff --git a/src/mem/tport.cc b/src/mem/tport.cc index 6c8c12ce2..d6ff64608 100644 --- a/src/mem/tport.cc +++ b/src/mem/tport.cc @@ -55,7 +55,7 @@ SimpleTimingPort::recvFunctional(PacketPtr pkt) checkFunctional(pkt); // Just do an atomic access and throw away the returned latency - if (pkt->result != Packet::Success) + if (!pkt->isResponse()) recvAtomic(pkt); } @@ -68,7 +68,6 @@ SimpleTimingPort::recvTiming(PacketPtr pkt) // correctly with the drain code, so that would need to be fixed // if we ever added it back. assert(pkt->isRequest()); - assert(pkt->result == Packet::Unknown); if (pkt->memInhibitAsserted()) { // snooper will supply based on copy of packet @@ -85,7 +84,6 @@ SimpleTimingPort::recvTiming(PacketPtr pkt) // recvAtomic() should already have turned packet into // atomic response assert(pkt->isResponse()); - pkt->convertAtomicToTimingResponse(); schedSendTiming(pkt, curTick + latency); } else { delete pkt->req; @@ -138,12 +136,15 @@ void SimpleTimingPort::sendDeferredPacket() { assert(deferredPacketReady()); - bool success = sendTiming(transmitList.front().pkt); + // take packet off list here; if recvTiming() on the other side + // calls sendTiming() back on us (like SimpleTimingCpu does), then + // we get confused by having a non-active packet on transmitList + DeferredPacket dp = transmitList.front(); + transmitList.pop_front(); + bool success = sendTiming(dp.pkt); if (success) { - //send successful, remove packet - transmitList.pop_front(); - if (!transmitList.empty()) { + if (!transmitList.empty() && !sendEvent->scheduled()) { Tick time = transmitList.front().tick; sendEvent->schedule(time <= curTick ? curTick+1 : time); } @@ -152,6 +153,12 @@ SimpleTimingPort::sendDeferredPacket() drainEvent->process(); drainEvent = NULL; } + } else { + // Unsuccessful, need to put back on transmitList. Callee + // should not have messed with it (since it didn't accept that + // packet), so we can just push it back on the front. + assert(!sendEvent->scheduled()); + transmitList.push_front(dp); } waitingOnRetry = !success; -- cgit v1.2.3