diff -r a894d72ea468 -r c7358871f1e5 src/arch/x86/pagetable_walker.hh --- a/src/arch/x86/pagetable_walker.hh Mon Feb 18 09:47:18 2013 +0000 +++ b/src/arch/x86/pagetable_walker.hh Mon Feb 18 12:03:37 2013 +0000 @@ -157,10 +157,8 @@ struct WalkerSenderState : public Packet::SenderState { WalkerState * senderWalk; - Packet::SenderState * saved; - WalkerSenderState(WalkerState * _senderWalk, - Packet::SenderState * _saved) : - senderWalk(_senderWalk), saved(_saved) {} + WalkerSenderState(WalkerState * _senderWalk) : + senderWalk(_senderWalk) {} }; public: diff -r a894d72ea468 -r c7358871f1e5 src/arch/x86/pagetable_walker.cc --- a/src/arch/x86/pagetable_walker.cc Mon Feb 18 09:47:18 2013 +0000 +++ b/src/arch/x86/pagetable_walker.cc Mon Feb 18 12:03:37 2013 +0000 @@ -123,8 +123,7 @@ Walker::recvTimingResp(PacketPtr pkt) { WalkerSenderState * senderState = - dynamic_cast(pkt->senderState); - pkt->senderState = senderState->saved; + dynamic_cast(pkt->popSenderState()); WalkerState * senderWalk = senderState->senderWalk; bool walkComplete = senderWalk->recvPacket(pkt); delete senderState; @@ -169,7 +168,7 @@ bool Walker::sendTiming(WalkerState* sendingState, PacketPtr pkt) { - pkt->senderState = new WalkerSenderState(sendingState, pkt->senderState); + pkt->pushSenderState(new WalkerSenderState(sendingState)); return port.sendTimingReq(pkt); } diff -r a894d72ea468 -r c7358871f1e5 src/cpu/testers/rubytest/Check.cc --- a/src/cpu/testers/rubytest/Check.cc Mon Feb 18 09:47:18 2013 +0000 +++ b/src/cpu/testers/rubytest/Check.cc Mon Feb 18 12:03:37 2013 +0000 @@ -111,16 +111,13 @@ // push the subblock onto the sender state. The sequencer will // update the subblock on the return - pkt->senderState = - new SenderState(m_address, req->getSize(), pkt->senderState); + pkt->senderState = new SenderState(m_address, req->getSize()); if (port->sendTimingReq(pkt)) { DPRINTF(RubyTest, "successfully initiated prefetch.\n"); } else { // If the packet did not issue, must delete - SenderState* senderState = safe_cast(pkt->senderState); - pkt->senderState = senderState->saved; - delete senderState; + delete pkt->senderState; delete pkt->req; delete pkt; @@ -151,8 +148,7 @@ // push the subblock onto the sender state. The sequencer will // update the subblock on the return - pkt->senderState = - new SenderState(m_address, req->getSize(), pkt->senderState); + pkt->senderState = new SenderState(m_address, req->getSize()); if (port->sendTimingReq(pkt)) { DPRINTF(RubyTest, "initiating Flush - successful\n"); @@ -198,8 +194,7 @@ // push the subblock onto the sender state. The sequencer will // update the subblock on the return - pkt->senderState = - new SenderState(writeAddr, req->getSize(), pkt->senderState); + pkt->senderState = new SenderState(writeAddr, req->getSize()); if (port->sendTimingReq(pkt)) { DPRINTF(RubyTest, "initiating action - successful\n"); @@ -210,9 +205,7 @@ // If the packet did not issue, must delete // Note: No need to delete the data, the packet destructor // will delete it - SenderState* senderState = safe_cast(pkt->senderState); - pkt->senderState = senderState->saved; - delete senderState; + delete pkt->senderState; delete pkt->req; delete pkt; @@ -250,8 +243,7 @@ // push the subblock onto the sender state. The sequencer will // update the subblock on the return - pkt->senderState = - new SenderState(m_address, req->getSize(), pkt->senderState); + pkt->senderState = new SenderState(m_address, req->getSize()); if (port->sendTimingReq(pkt)) { DPRINTF(RubyTest, "initiating check - successful\n"); @@ -262,9 +254,7 @@ // If the packet did not issue, must delete // Note: No need to delete the data, the packet destructor // will delete it - SenderState* senderState = safe_cast(pkt->senderState); - pkt->senderState = senderState->saved; - delete senderState; + delete pkt->senderState; delete pkt->req; delete pkt; diff -r a894d72ea468 -r c7358871f1e5 src/cpu/testers/rubytest/RubyTester.hh --- a/src/cpu/testers/rubytest/RubyTester.hh Mon Feb 18 09:47:18 2013 +0000 +++ b/src/cpu/testers/rubytest/RubyTester.hh Mon Feb 18 12:03:37 2013 +0000 @@ -1,4 +1,16 @@ /* + * Copyright (c) 2013 ARM Limited + * All rights reserved + * + * The license below extends only to copyright in the software and shall + * not be construed as granting a license to any other intellectual + * property including but not limited to intellectual property relating + * to a hardware implementation of the functionality of the software + * licensed hereunder. You may use the software subject to the license + * terms below provided that you ensure that this notice is replicated + * unmodified and in its entirety in all distributions of the software, + * modified or unmodified, in source code or in binary form. + * * Copyright (c) 1999-2008 Mark D. Hill and David A. Wood * Copyright (c) 2009 Advanced Micro Devices, Inc. * All rights reserved. @@ -69,20 +81,10 @@ struct SenderState : public Packet::SenderState { - SubBlock* subBlock; - Packet::SenderState *saved; + SubBlock subBlock; - SenderState(Address addr, int size, - Packet::SenderState *sender_state = NULL) - : saved(sender_state) - { - subBlock = new SubBlock(addr, size); - } + SenderState(Address addr, int size) : subBlock(addr, size) {} - ~SenderState() - { - delete subBlock; - } }; typedef RubyTesterParams Params; diff -r a894d72ea468 -r c7358871f1e5 src/cpu/testers/rubytest/RubyTester.cc --- a/src/cpu/testers/rubytest/RubyTester.cc Mon Feb 18 09:47:18 2013 +0000 +++ b/src/cpu/testers/rubytest/RubyTester.cc Mon Feb 18 12:03:37 2013 +0000 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012 ARM Limited + * Copyright (c) 2012-2013 ARM Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -149,17 +149,13 @@ // retrieve the subblock and call hitCallback RubyTester::SenderState* senderState = safe_cast(pkt->senderState); - SubBlock* subblock = senderState->subBlock; - assert(subblock != NULL); + SubBlock& subblock = senderState->subBlock; - // pop the sender state from the packet - pkt->senderState = senderState->saved; - - tester->hitCallback(id, subblock); + tester->hitCallback(id, &subblock); // Now that the tester has completed, delete the senderState // (includes sublock) and the packet, then return - delete senderState; + delete pkt->senderState; delete pkt->req; delete pkt; return true; diff -r a894d72ea468 -r c7358871f1e5 src/mem/addr_mapper.hh --- a/src/mem/addr_mapper.hh Mon Feb 18 09:47:18 2013 +0000 +++ b/src/mem/addr_mapper.hh Mon Feb 18 12:03:37 2013 +0000 @@ -87,23 +87,16 @@ public: /** - * Construct a new sender state and remember the original one - * so that we can implement a stack. + * Construct a new sender state to remember the original address. * - * @param _origSenderState Sender state to remember * @param _origAddr Address before remapping */ - AddrMapperSenderState(SenderState* _origSenderState, - Addr _origAddr) - : origSenderState(_origSenderState), origAddr(_origAddr) + AddrMapperSenderState(Addr _origAddr) : origAddr(_origAddr) { } /** Destructor */ ~AddrMapperSenderState() { } - /** Pointer to old sender state of packet */ - SenderState* origSenderState; - /** The original address the packet was destined for */ Addr origAddr; diff -r a894d72ea468 -r c7358871f1e5 src/mem/addr_mapper.cc --- a/src/mem/addr_mapper.cc Mon Feb 18 09:47:18 2013 +0000 +++ b/src/mem/addr_mapper.cc Mon Feb 18 12:03:37 2013 +0000 @@ -123,10 +123,9 @@ Addr orig_addr = pkt->getAddr(); bool needsResponse = pkt->needsResponse(); bool memInhibitAsserted = pkt->memInhibitAsserted(); - Packet::SenderState* senderState = pkt->senderState; if (needsResponse && !memInhibitAsserted) { - pkt->senderState = new AddrMapperSenderState(senderState, orig_addr); + pkt->pushSenderState(new AddrMapperSenderState(orig_addr)); } pkt->setAddr(remapAddr(orig_addr)); @@ -137,8 +136,7 @@ // If not successful, restore the sender state if (!successful && needsResponse) { - delete pkt->senderState; - pkt->senderState = senderState; + delete pkt->popSenderState(); } return successful; @@ -158,7 +156,7 @@ Addr remapped_addr = pkt->getAddr(); // Restore the state and address - pkt->senderState = receivedState->origSenderState; + pkt->senderState = receivedState->predecessor; pkt->setAddr(receivedState->origAddr); // Attempt to send the packet diff -r a894d72ea468 -r c7358871f1e5 src/mem/bridge.hh --- a/src/mem/bridge.hh Mon Feb 18 09:47:18 2013 +0000 +++ b/src/mem/bridge.hh Mon Feb 18 12:03:37 2013 +0000 @@ -84,20 +84,11 @@ public: - Packet::SenderState *origSenderState; PortID origSrc; - RequestState(PacketPtr _pkt) - : origSenderState(_pkt->senderState), - origSrc(_pkt->getSrc()) + RequestState(PortID orig_src) : origSrc(orig_src) { } - void fixResponse(PacketPtr pkt) - { - assert(pkt->senderState == this); - pkt->setDest(origSrc); - pkt->senderState = origSenderState; - } }; /** diff -r a894d72ea468 -r c7358871f1e5 src/mem/bridge.cc --- a/src/mem/bridge.cc Mon Feb 18 09:47:18 2013 +0000 +++ b/src/mem/bridge.cc Mon Feb 18 12:03:37 2013 +0000 @@ -201,8 +201,7 @@ if (!pkt->memInhibitAsserted() && pkt->needsResponse()) { // Update the sender state so we can deal with the response // appropriately - RequestState *req_state = new RequestState(pkt); - pkt->senderState = req_state; + pkt->pushSenderState(new RequestState(pkt->getSrc())); } // If we're about to put this packet at the head of the queue, we @@ -225,11 +224,10 @@ // This is a response for a request we forwarded earlier. The // corresponding request state should be stored in the packet's // senderState field. - RequestState *req_state = dynamic_cast(pkt->senderState); + RequestState *req_state = + dynamic_cast(pkt->popSenderState()); assert(req_state != NULL); - // set up new packet dest & senderState based on values saved - // from original request - req_state->fixResponse(pkt); + pkt->setDest(req_state->origSrc); delete req_state; // the bridge assumes that at least one bus has set the diff -r a894d72ea468 -r c7358871f1e5 src/mem/cache/cache_impl.hh --- a/src/mem/cache/cache_impl.hh Mon Feb 18 09:47:18 2013 +0000 +++ b/src/mem/cache/cache_impl.hh Mon Feb 18 12:03:37 2013 +0000 @@ -348,24 +348,12 @@ class ForwardResponseRecord : public Packet::SenderState { - Packet::SenderState *prevSenderState; + public: + PortID prevSrc; -#ifndef NDEBUG - BaseCache *cache; -#endif - public: - ForwardResponseRecord(Packet *pkt, BaseCache *_cache) - : prevSenderState(pkt->senderState), prevSrc(pkt->getSrc()) -#ifndef NDEBUG - , cache(_cache) -#endif + + ForwardResponseRecord(PortID prev_src) : prevSrc(prev_src) {} - void restore(Packet *pkt, BaseCache *_cache) - { - assert(_cache == cache); - pkt->senderState = prevSenderState; - pkt->setDest(prevSrc); - } }; @@ -389,7 +377,7 @@ if (pkt->isResponse()) { // must be cache-to-cache response from upper to lower level ForwardResponseRecord *rec = - dynamic_cast(pkt->senderState); + dynamic_cast(pkt->popSenderState()); assert(!system->bypassCaches()); if (rec == NULL) { @@ -402,7 +390,7 @@ return true; } - rec->restore(pkt, this); + pkt->setDest(rec->prevSrc); delete rec; memSidePort->schedTimingSnoopResp(pkt, time); return true; @@ -1293,14 +1281,14 @@ if (is_timing) { Packet snoopPkt(pkt, true); // clear flags snoopPkt.setExpressSnoop(); - snoopPkt.senderState = new ForwardResponseRecord(pkt, this); + snoopPkt.pushSenderState(new ForwardResponseRecord(pkt->getSrc())); cpuSidePort->sendTimingSnoopReq(&snoopPkt); if (snoopPkt.memInhibitAsserted()) { // cache-to-cache response from some upper cache assert(!alreadyResponded); pkt->assertMemInhibit(); } else { - delete snoopPkt.senderState; + delete snoopPkt.popSenderState(); } if (snoopPkt.sharedAsserted()) { pkt->assertShared(); diff -r a894d72ea468 -r c7358871f1e5 src/mem/comm_monitor.hh --- a/src/mem/comm_monitor.hh Mon Feb 18 09:47:18 2013 +0000 +++ b/src/mem/comm_monitor.hh Mon Feb 18 12:03:37 2013 +0000 @@ -107,23 +107,18 @@ public: /** - * Construct a new sender state and remember the original one - * so that we can implement a stack. + * Construct a new sender state and store the time so we can + * calculate round-trip latency. * - * @param _origSenderState Sender state to remember * @param _transmitTime Time of packet transmission */ - CommMonitorSenderState(SenderState* _origSenderState, - Tick _transmitTime) - : origSenderState(_origSenderState), transmitTime(_transmitTime) + CommMonitorSenderState(Tick _transmitTime) + : transmitTime(_transmitTime) { } /** Destructor */ ~CommMonitorSenderState() { } - /** Pointer to old sender state of packet */ - SenderState* origSenderState; - /** Tick when request is transmitted */ Tick transmitTime; diff -r a894d72ea468 -r c7358871f1e5 src/mem/comm_monitor.cc --- a/src/mem/comm_monitor.cc Mon Feb 18 09:47:18 2013 +0000 +++ b/src/mem/comm_monitor.cc Mon Feb 18 12:03:37 2013 +0000 @@ -167,15 +167,13 @@ Addr addr = pkt->getAddr(); bool needsResponse = pkt->needsResponse(); bool memInhibitAsserted = pkt->memInhibitAsserted(); - Packet::SenderState* senderState = pkt->senderState; // If a cache miss is served by a cache, a monitor near the memory // would see a request which needs a response, but this response // would be inhibited and not come back from the memory. Therefore // we additionally have to check the inhibit flag. if (needsResponse && !memInhibitAsserted && !stats.disableLatencyHists) { - pkt->senderState = new CommMonitorSenderState(senderState, - curTick()); + pkt->pushSenderState(new CommMonitorSenderState(curTick())); } // Attempt to send the packet (always succeeds for inhibited @@ -184,8 +182,7 @@ // If not successful, restore the sender state if (!successful && needsResponse && !stats.disableLatencyHists) { - delete pkt->senderState; - pkt->senderState = senderState; + delete pkt->popSenderState(); } if (successful && traceStream != NULL) { @@ -306,7 +303,7 @@ panic("Monitor got a response without monitor sender state\n"); // Restore the sate - pkt->senderState = commReceivedState->origSenderState; + pkt->senderState = commReceivedState->predecessor; } // Attempt to send the packet diff -r a894d72ea468 -r c7358871f1e5 src/mem/packet.hh --- a/src/mem/packet.hh Mon Feb 18 09:47:18 2013 +0000 +++ b/src/mem/packet.hh Mon Feb 18 12:03:37 2013 +0000 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012 ARM Limited + * Copyright (c) 2012-2013 ARM Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -340,15 +340,25 @@ /** * 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 - * response so that the sender can quickly look up the state - * needed to process it. A specific subclass would be derived - * from this to carry state specific to a particular sending - * device. + * with the packet (e.g., an MSHR), specific to a MemObject that + * sees the packet. A pointer to this state is returned in the + * packet's response so that the MemObject in question can quickly + * look up the state needed to process it. A specific subclass + * would be derived from this to carry state specific to a + * particular sending device. + * + * As multiple MemObjects may add their SenderState throughout the + * memory system, the SenderStates create a stack, where a + * MemObject can add a new Senderstate, as long as the + * predecessing SenderState is restored when the response comes + * back. For this reason, the predecessor should always be + * populated with the current SenderState of a packet before + * modifying the senderState field in the request packet. */ struct SenderState { + SenderState* predecessor; + SenderState() : predecessor(NULL) {} virtual ~SenderState() {} }; @@ -418,12 +428,32 @@ * This packet's sender state. Devices should use dynamic_cast<> * to cast to the state appropriate to the sender. The intent of * this variable is to allow a device to attach extra information - * to a request. A response packet must return the sender state + * to a request. A response packet must return the sender state * that was attached to the original request (even if a new packet * is created). */ SenderState *senderState; + /** + * Push a new sender state to the packet and make the current + * sender state the predecessor of the new one. This should be + * prefered over direct manipulation of the senderState member + * variable. + * + * @param sender_state SenderState to push at the top of the stack + */ + void pushSenderState(SenderState *sender_state); + + /** + * Pop the top of the state stack and return a pointer to it. This + * assumes the current sender state is not NULL. This should be + * preferred over direct manipulation of the senderState member + * variable. + * + * @return The current top of the stack + */ + SenderState *popSenderState(); + /// Return the string name of the cmd field (for debugging and /// tracing). const std::string &cmdString() const { return cmd.toString(); } diff -r a894d72ea468 -r c7358871f1e5 src/mem/packet.cc --- a/src/mem/packet.cc Mon Feb 18 09:47:18 2013 +0000 +++ b/src/mem/packet.cc Mon Feb 18 12:03:37 2013 +0000 @@ -316,6 +316,24 @@ } void +Packet::pushSenderState(Packet::SenderState *sender_state) +{ + assert(sender_state != NULL); + sender_state->predecessor = senderState; + senderState = sender_state; +} + +Packet::SenderState * +Packet::popSenderState() +{ + assert(senderState != NULL); + SenderState *sender_state = senderState; + senderState = sender_state->predecessor; + sender_state->predecessor = NULL; + return sender_state; +} + +void Packet::print(ostream &o, const int verbosity, const string &prefix) const { ccprintf(o, "%s[%x:%x] %s\n", prefix, diff -r a894d72ea468 -r c7358871f1e5 src/mem/ruby/system/RubyPort.hh --- a/src/mem/ruby/system/RubyPort.hh Mon Feb 18 09:47:18 2013 +0000 +++ b/src/mem/ruby/system/RubyPort.hh Mon Feb 18 12:03:37 2013 +0000 @@ -113,10 +113,8 @@ struct SenderState : public Packet::SenderState { M5Port* port; - Packet::SenderState *saved; - SenderState(M5Port* _port, Packet::SenderState *sender_state = NULL) - : port(_port), saved(sender_state) + SenderState(M5Port* _port) : port(_port) {} }; diff -r a894d72ea468 -r c7358871f1e5 src/mem/ruby/system/RubyPort.cc --- a/src/mem/ruby/system/RubyPort.cc Mon Feb 18 09:47:18 2013 +0000 +++ b/src/mem/ruby/system/RubyPort.cc Mon Feb 18 12:03:37 2013 +0000 @@ -151,12 +151,9 @@ // First we must retrieve the request port from the sender State RubyPort::SenderState *senderState = - safe_cast(pkt->senderState); + safe_cast(pkt->popSenderState()); M5Port *port = senderState->port; assert(port != NULL); - - // pop the sender state from the packet - pkt->senderState = senderState->saved; delete senderState; port->sendTimingResp(pkt); @@ -187,7 +184,7 @@ // Save the port in the sender state object to be used later to // route the response - pkt->senderState = new SenderState(this, pkt->senderState); + pkt->pushSenderState(new SenderState(this)); // Check for pio requests and directly send them to the dedicated // pio port. @@ -230,7 +227,7 @@ pkt->getAddr(), RequestStatus_to_string(requestStatus)); SenderState* senderState = safe_cast(pkt->senderState); - pkt->senderState = senderState->saved; + pkt->senderState = senderState->predecessor; delete senderState; return false; } @@ -305,7 +302,7 @@ assert(port != NULL); // pop the sender state from the packet - pkt->senderState = senderState->saved; + pkt->senderState = senderState->predecessor; delete senderState; port->hitCallback(pkt); diff -r a894d72ea468 -r c7358871f1e5 src/mem/ruby/system/Sequencer.cc --- a/src/mem/ruby/system/Sequencer.cc Mon Feb 18 09:47:18 2013 +0000 +++ b/src/mem/ruby/system/Sequencer.cc Mon Feb 18 12:03:37 2013 +0000 @@ -530,11 +530,13 @@ // Note: RubyPort will access it's sender state before the // RubyTester. if (m_usingRubyTester) { - RubyPort::SenderState *requestSenderState = + RubyPort::SenderState *reqSenderState = safe_cast(pkt->senderState); + // @todo This is a dangerous assumption on nothing else + // modifying the senderState RubyTester::SenderState* testerSenderState = - safe_cast(requestSenderState->saved); - testerSenderState->subBlock->mergeFrom(data); + safe_cast(reqSenderState->predecessor); + testerSenderState->subBlock.mergeFrom(data); } delete srequest;