Bus: Turn the PortId into a transport function parameter

The main aim of this patch is to arrive at a suitable port interface
for vector ports, including both the packet and the port id. This
patch changes the bus transport functions
(recvFunctional/Atomic/Timing) to require a PortId parameter
indicating the source port. Previously this information was passed by
setting the source field of the packet, and this is only required in
the case of a timing request.

With this patch, the use of the source and destination field is also
more restrictive, as they are only needed for timing accesses. The
modifications to these fields for atomic snoops is now removed
entirely, also making minor modifications to the cache.

3 files changed, 85 insertions, 81 deletions

diff --git a/src/mem/bus.cc b/src/mem/bus.cc
index e24b54b0e..e2764b63d 100644
--- a/src/mem/bus.cc
+++ b/src/mem/bus.cc
@@ -198,10 +198,10 @@ Bus::isOccupied(PacketPtr pkt, Port* port)
 }
 
 bool
-Bus::recvTimingReq(PacketPtr pkt)
+Bus::recvTimingReq(PacketPtr pkt, PortID slave_port_id)
 {
     // determine the source port based on the id
-    SlavePort *src_port = slavePorts[pkt->getSrc()];
+    SlavePort *src_port = slavePorts[slave_port_id];
 
     // test if the bus should be considered occupied for the current
     // packet, and exclude express snoops from the check
@@ -214,6 +214,9 @@ Bus::recvTimingReq(PacketPtr pkt)
     DPRINTF(Bus, "recvTimingReq: src %s %s 0x%x\n",
             src_port->name(), pkt->cmdString(), pkt->getAddr());
 
+    // set the source port for routing of the response
+    pkt->setSrc(slave_port_id);
+
     Tick headerFinishTime = pkt->isExpressSnoop() ? 0 : calcPacketTiming(pkt);
     Tick packetFinishTime = pkt->isExpressSnoop() ? 0 : pkt->finishTime;
 
@@ -221,7 +224,7 @@ Bus::recvTimingReq(PacketPtr pkt)
     if (!pkt->req->isUncacheable()) {
         // the packet is a memory-mapped request and should be
         // broadcasted to our snoopers but the source
-        forwardTiming(pkt, pkt->getSrc());
+        forwardTiming(pkt, slave_port_id);
     }
 
     // remember if we add an outstanding req so we can undo it if
@@ -268,10 +271,10 @@ Bus::recvTimingReq(PacketPtr pkt)
 }
 
 bool
-Bus::recvTimingResp(PacketPtr pkt)
+Bus::recvTimingResp(PacketPtr pkt, PortID master_port_id)
 {
     // determine the source port based on the id
-    MasterPort *src_port = masterPorts[pkt->getSrc()];
+    MasterPort *src_port = masterPorts[master_port_id];
 
     // test if the bus should be considered occupied for the current
     // packet
@@ -308,15 +311,18 @@ Bus::recvTimingResp(PacketPtr pkt)
 }
 
 void
-Bus::recvTimingSnoopReq(PacketPtr pkt)
+Bus::recvTimingSnoopReq(PacketPtr pkt, PortID master_port_id)
 {
     DPRINTF(Bus, "recvTimingSnoopReq: src %s %s 0x%x\n",
-            masterPorts[pkt->getSrc()]->name(), pkt->cmdString(),
+            masterPorts[master_port_id]->name(), pkt->cmdString(),
             pkt->getAddr());
 
     // we should only see express snoops from caches
     assert(pkt->isExpressSnoop());
 
+    // set the source port for routing of the response
+    pkt->setSrc(master_port_id);
+
     // forward to all snoopers
     forwardTiming(pkt, InvalidPortID);
 
@@ -325,17 +331,17 @@ Bus::recvTimingSnoopReq(PacketPtr pkt)
     // device responsible for the address range something is
     // wrong, hence there is nothing further to do as the packet
     // would be going back to where it came from
-    assert(pkt->getSrc() == findPort(pkt->getAddr()));
+    assert(master_port_id == findPort(pkt->getAddr()));
 
     // this is an express snoop and is never forced to retry
     assert(!inRetry);
 }
 
 bool
-Bus::recvTimingSnoopResp(PacketPtr pkt)
+Bus::recvTimingSnoopResp(PacketPtr pkt, PortID slave_port_id)
 {
     // determine the source port based on the id
-    SlavePort* src_port = slavePorts[pkt->getSrc()];
+    SlavePort* src_port = slavePorts[slave_port_id];
 
     if (isOccupied(pkt, src_port)) {
         DPRINTF(Bus, "recvTimingSnoopResp: src %s %s 0x%x BUSY\n",
@@ -377,7 +383,7 @@ Bus::recvTimingSnoopResp(PacketPtr pkt)
         // request, hence it should never go back to where the
         // snoop response came from, but instead to where the
         // original request came from
-        assert(pkt->getSrc() != dest);
+        assert(slave_port_id != dest);
 
         // as a normal response, it should go back to a master
         // through one of our slave ports
@@ -481,7 +487,7 @@ Bus::retryWaiting()
 }
 
 void
-Bus::recvRetry(PortID id)
+Bus::recvRetry()
 {
     // we got a retry from a peer that we tried to send something to
     // and failed, but we sent it on the account of someone else, and
@@ -538,10 +544,10 @@ Bus::findPort(Addr addr)
 }
 
 Tick
-Bus::recvAtomic(PacketPtr pkt)
+Bus::recvAtomic(PacketPtr pkt, PortID slave_port_id)
 {
     DPRINTF(Bus, "recvAtomic: packet src %s addr 0x%x cmd %s\n",
-            slavePorts[pkt->getSrc()]->name(), pkt->getAddr(),
+            slavePorts[slave_port_id]->name(), pkt->getAddr(),
             pkt->cmdString());
 
     MemCmd snoop_response_cmd = MemCmd::InvalidCmd;
@@ -551,7 +557,7 @@ Bus::recvAtomic(PacketPtr pkt)
     if (!pkt->req->isUncacheable()) {
         // forward to all snoopers but the source
         std::pair<MemCmd, Tick> snoop_result =
-            forwardAtomic(pkt, pkt->getSrc());
+            forwardAtomic(pkt, slave_port_id);
         snoop_response_cmd = snoop_result.first;
         snoop_response_latency = snoop_result.second;
     }
@@ -576,10 +582,10 @@ Bus::recvAtomic(PacketPtr pkt)
 }
 
 Tick
-Bus::recvAtomicSnoop(PacketPtr pkt)
+Bus::recvAtomicSnoop(PacketPtr pkt, PortID master_port_id)
 {
     DPRINTF(Bus, "recvAtomicSnoop: packet src %s addr 0x%x cmd %s\n",
-            masterPorts[pkt->getSrc()]->name(), pkt->getAddr(),
+            masterPorts[master_port_id]->name(), pkt->getAddr(),
             pkt->cmdString());
 
     // forward to all snoopers
@@ -598,10 +604,9 @@ Bus::recvAtomicSnoop(PacketPtr pkt)
 std::pair<MemCmd, Tick>
 Bus::forwardAtomic(PacketPtr pkt, PortID exclude_slave_port_id)
 {
-    // the packet may be changed on snoops, record the original source
-    // and command to enable us to restore it between snoops so that
+    // the packet may be changed on snoops, record the original
+    // command to enable us to restore it between snoops so that
     // additional snoops can take place properly
-    PortID orig_src_id = pkt->getSrc();
     MemCmd orig_cmd = pkt->cmd;
     MemCmd snoop_response_cmd = MemCmd::InvalidCmd;
     Tick snoop_response_latency = 0;
@@ -629,8 +634,6 @@ Bus::forwardAtomic(PacketPtr pkt, PortID exclude_slave_port_id)
                 snoop_response_latency = latency;
                 // restore original packet state for remaining snoopers
                 pkt->cmd = orig_cmd;
-                pkt->setSrc(orig_src_id);
-                pkt->clearDest();
             }
         }
     }
@@ -641,20 +644,20 @@ Bus::forwardAtomic(PacketPtr pkt, PortID exclude_slave_port_id)
 }
 
 void
-Bus::recvFunctional(PacketPtr pkt)
+Bus::recvFunctional(PacketPtr pkt, PortID slave_port_id)
 {
     if (!pkt->isPrint()) {
         // don't do DPRINTFs on PrintReq as it clutters up the output
         DPRINTF(Bus,
                 "recvFunctional: packet src %s addr 0x%x cmd %s\n",
-                slavePorts[pkt->getSrc()]->name(), pkt->getAddr(),
+                slavePorts[slave_port_id]->name(), pkt->getAddr(),
                 pkt->cmdString());
     }
 
     // uncacheable requests need never be snooped
     if (!pkt->req->isUncacheable()) {
         // forward to all snoopers but the source
-        forwardFunctional(pkt, pkt->getSrc());
+        forwardFunctional(pkt, slave_port_id);
     }
 
     // there is no need to continue if the snooping has found what we
@@ -667,13 +670,13 @@ Bus::recvFunctional(PacketPtr pkt)
 }
 
 void
-Bus::recvFunctionalSnoop(PacketPtr pkt)
+Bus::recvFunctionalSnoop(PacketPtr pkt, PortID master_port_id)
 {
     if (!pkt->isPrint()) {
         // don't do DPRINTFs on PrintReq as it clutters up the output
         DPRINTF(Bus,
                 "recvFunctionalSnoop: packet src %s addr 0x%x cmd %s\n",
-                masterPorts[pkt->getSrc()]->name(), pkt->getAddr(),
+                masterPorts[master_port_id]->name(), pkt->getAddr(),
                 pkt->cmdString());
     }
 
@@ -703,24 +706,25 @@ Bus::forwardFunctional(PacketPtr pkt, PortID exclude_slave_port_id)
 
 /** Function called by the port when the bus is receiving a range change.*/
 void
-Bus::recvRangeChange(PortID id)
+Bus::recvRangeChange(PortID master_port_id)
 {
     AddrRangeList ranges;
     AddrRangeIter iter;
 
-    if (inRecvRangeChange.count(id))
+    if (inRecvRangeChange.count(master_port_id))
         return;
-    inRecvRangeChange.insert(id);
+    inRecvRangeChange.insert(master_port_id);
 
-    DPRINTF(BusAddrRanges, "received RangeChange from device id %d\n", id);
+    DPRINTF(BusAddrRanges, "received RangeChange from device id %d\n",
+            master_port_id);
 
     clearPortCache();
-    if (id == defaultPortID) {
+    if (master_port_id == defaultPortID) {
         defaultRange.clear();
         // Only try to update these ranges if the user set a default responder.
         if (useDefaultRange) {
             AddrRangeList ranges =
-                masterPorts[id]->getSlavePort().getAddrRanges();
+                masterPorts[master_port_id]->getSlavePort().getAddrRanges();
             for(iter = ranges.begin(); iter != ranges.end(); iter++) {
                 defaultRange.push_back(*iter);
                 DPRINTF(BusAddrRanges, "Adding range %#llx - %#llx for default range\n",
@@ -729,13 +733,13 @@ Bus::recvRangeChange(PortID id)
         }
     } else {
 
-        assert(id < masterPorts.size() && id >= 0);
-        MasterPort *port = masterPorts[id];
+        assert(master_port_id < masterPorts.size() && master_port_id >= 0);
+        MasterPort *port = masterPorts[master_port_id];
 
         // Clean out any previously existent ids
         for (PortIter portIter = portMap.begin();
              portIter != portMap.end(); ) {
-            if (portIter->second == id)
+            if (portIter->second == master_port_id)
                 portMap.erase(portIter++);
             else
                 portIter++;
@@ -745,11 +749,12 @@ Bus::recvRangeChange(PortID id)
 
         for (iter = ranges.begin(); iter != ranges.end(); iter++) {
             DPRINTF(BusAddrRanges, "Adding range %#llx - %#llx for id %d\n",
-                    iter->start, iter->end, id);
-            if (portMap.insert(*iter, id) == portMap.end()) {
+                    iter->start, iter->end, master_port_id);
+            if (portMap.insert(*iter, master_port_id) == portMap.end()) {
                 PortID conflict_id = portMap.find(*iter)->second;
                 fatal("%s has two ports with same range:\n\t%s\n\t%s\n",
-                      name(), masterPorts[id]->getSlavePort().name(),
+                      name(),
+                      masterPorts[master_port_id]->getSlavePort().name(),
                       masterPorts[conflict_id]->getSlavePort().name());
             }
         }
@@ -762,11 +767,11 @@ Bus::recvRangeChange(PortID id)
          ++p)
         (*p)->sendRangeChange();
 
-    inRecvRangeChange.erase(id);
+    inRecvRangeChange.erase(master_port_id);
 }
 
 AddrRangeList
-Bus::getAddrRanges(PortID id)
+Bus::getAddrRanges()
 {
     AddrRangeList ranges;
 
@@ -796,7 +801,7 @@ Bus::getAddrRanges(PortID id)
                     portIter->first.start, portIter->first.end);
             }
         }
-        if (portIter->second != id && !subset) {
+        if (!subset) {
             ranges.push_back(portIter->first);
             DPRINTF(BusAddrRanges, "  -- %#llx : %#llx\n",
                     portIter->first.start, portIter->first.end);
@@ -807,14 +812,14 @@ Bus::getAddrRanges(PortID id)
 }
 
 bool
-Bus::isSnooping(PortID id) const
+Bus::isSnooping() const
 {
     // in essence, answer the question if there are snooping ports
     return !snoopPorts.empty();
 }
 
 unsigned
-Bus::findBlockSize(PortID id)
+Bus::findBlockSize()
 {
     if (cachedBlockSizeValid)
         return cachedBlockSize;
diff --git a/src/mem/bus.hh b/src/mem/bus.hh
index b5d7a3801..dc5051fc2 100644
--- a/src/mem/bus.hh
+++ b/src/mem/bus.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 ARM Limited
+ * Copyright (c) 2011-2012 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -91,25 +91,25 @@ class Bus : public MemObject
          * When receiving a timing request, pass it to the bus.
          */
         virtual bool recvTimingReq(PacketPtr pkt)
-        { pkt->setSrc(id); return bus->recvTimingReq(pkt); }
+        { return bus->recvTimingReq(pkt, id); }
 
         /**
          * When receiving a timing snoop response, pass it to the bus.
          */
         virtual bool recvTimingSnoopResp(PacketPtr pkt)
-        { pkt->setSrc(id); return bus->recvTimingSnoopResp(pkt); }
+        { return bus->recvTimingSnoopResp(pkt, id); }
 
         /**
          * When receiving an atomic request, pass it to the bus.
          */
         virtual Tick recvAtomic(PacketPtr pkt)
-        { pkt->setSrc(id); return bus->recvAtomic(pkt); }
+        { return bus->recvAtomic(pkt, id); }
 
         /**
          * When receiving a functional request, pass it to the bus.
          */
         virtual void recvFunctional(PacketPtr pkt)
-        { pkt->setSrc(id); bus->recvFunctional(pkt); }
+        { bus->recvFunctional(pkt, id); }
 
         /**
          * When receiving a retry, pass it to the bus.
@@ -122,13 +122,13 @@ class Bus : public MemObject
         // the 'owned' address ranges of all the other interfaces on
         // this bus...
         virtual AddrRangeList getAddrRanges()
-        { return bus->getAddrRanges(id); }
+        { return bus->getAddrRanges(); }
 
         // Ask the bus to ask everyone on the bus what their block size is and
         // take the max of it. This might need to be changed a bit if we ever
         // support multiple block sizes.
         virtual unsigned deviceBlockSize() const
-        { return bus->findBlockSize(id); }
+        { return bus->findBlockSize(); }
 
     };
 
@@ -157,7 +157,7 @@ class Bus : public MemObject
          * @return a boolean that is true if this port is snooping
          */
         virtual bool isSnooping() const
-        { return bus->isSnooping(id); }
+        { return bus->isSnooping(); }
 
       protected:
 
@@ -165,25 +165,25 @@ class Bus : public MemObject
          * When receiving a timing response, pass it to the bus.
          */
         virtual bool recvTimingResp(PacketPtr pkt)
-        { pkt->setSrc(id); return bus->recvTimingResp(pkt); }
+        { return bus->recvTimingResp(pkt, id); }
 
         /**
          * When receiving a timing snoop request, pass it to the bus.
          */
         virtual void recvTimingSnoopReq(PacketPtr pkt)
-        { pkt->setSrc(id); return bus->recvTimingSnoopReq(pkt); }
+        { return bus->recvTimingSnoopReq(pkt, id); }
 
         /**
          * When receiving an atomic snoop request, pass it to the bus.
          */
         virtual Tick recvAtomicSnoop(PacketPtr pkt)
-        { pkt->setSrc(id); return bus->recvAtomicSnoop(pkt); }
+        { return bus->recvAtomicSnoop(pkt, id); }
 
         /**
          * When receiving a functional snoop request, pass it to the bus.
          */
         virtual void recvFunctionalSnoop(PacketPtr pkt)
-        { pkt->setSrc(id); bus->recvFunctionalSnoop(pkt); }
+        { bus->recvFunctionalSnoop(pkt, id); }
 
         /** When reciving a range change from the peer port (at id),
             pass it to the bus. */
@@ -193,13 +193,13 @@ class Bus : public MemObject
         /** When reciving a retry from the peer port (at id),
             pass it to the bus. */
         virtual void recvRetry()
-        { bus->recvRetry(id); }
+        { bus->recvRetry(); }
 
         // Ask the bus to ask everyone on the bus what their block size is and
         // take the max of it. This might need to be changed a bit if we ever
         // support multiple block sizes.
         virtual unsigned deviceBlockSize() const
-        { return bus->findBlockSize(id); }
+        { return bus->findBlockSize(); }
 
     };
 
@@ -230,19 +230,19 @@ class Bus : public MemObject
 
     /** Function called by the port when the bus is recieving a Timing
       request packet.*/
-    bool recvTimingReq(PacketPtr pkt);
+    bool recvTimingReq(PacketPtr pkt, PortID slave_port_id);
 
     /** Function called by the port when the bus is recieving a Timing
       response packet.*/
-    bool recvTimingResp(PacketPtr pkt);
+    bool recvTimingResp(PacketPtr pkt, PortID master_port_id);
 
     /** Function called by the port when the bus is recieving a timing
         snoop request.*/
-    void recvTimingSnoopReq(PacketPtr pkt);
+    void recvTimingSnoopReq(PacketPtr pkt, PortID master_port_id);
 
     /** Function called by the port when the bus is recieving a timing
         snoop response.*/
-    bool recvTimingSnoopResp(PacketPtr pkt);
+    bool recvTimingSnoopResp(PacketPtr pkt, PortID slave_port_id);
 
     /**
      * Forward a timing packet to our snoopers, potentially excluding
@@ -277,11 +277,11 @@ class Bus : public MemObject
 
     /** Function called by the port when the bus is recieving a Atomic
       transaction.*/
-    Tick recvAtomic(PacketPtr pkt);
+    Tick recvAtomic(PacketPtr pkt, PortID slave_port_id);
 
     /** Function called by the port when the bus is recieving an
         atomic snoop transaction.*/
-    Tick recvAtomicSnoop(PacketPtr pkt);
+    Tick recvAtomicSnoop(PacketPtr pkt, PortID master_port_id);
 
     /**
      * Forward an atomic packet to our snoopers, potentially excluding
@@ -298,11 +298,11 @@ class Bus : public MemObject
 
     /** Function called by the port when the bus is recieving a Functional
         transaction.*/
-    void recvFunctional(PacketPtr pkt);
+    void recvFunctional(PacketPtr pkt, PortID slave_port_id);
 
     /** Function called by the port when the bus is recieving a functional
         snoop transaction.*/
-    void recvFunctionalSnoop(PacketPtr pkt);
+    void recvFunctionalSnoop(PacketPtr pkt, PortID master_port_id);
 
     /**
      * Forward a functional packet to our snoopers, potentially
@@ -316,10 +316,14 @@ class Bus : public MemObject
 
     /** Timing function called by port when it is once again able to process
      * requests. */
-    void recvRetry(PortID id);
+    void recvRetry();
 
-    /** Function called by the port when the bus is recieving a range change.*/
-    void recvRangeChange(PortID id);
+    /**
+     * Function called by the port when the bus is recieving a range change.
+     *
+     * @param master_port_id id of the port that received the change
+     */
+    void recvRangeChange(PortID master_port_id);
 
     /** Find which port connected to this bus (if any) should be given a packet
      * with this address.
@@ -383,22 +387,18 @@ class Bus : public MemObject
     }
 
     /**
-     * Return the address ranges this port is responsible for.
-     *
-     * @param id id of the bus port that made the request
+     * Return the address ranges the bus is responsible for.
      *
      * @return a list of non-overlapping address ranges
      */
-    AddrRangeList getAddrRanges(PortID id);
+    AddrRangeList getAddrRanges();
 
     /**
      * Determine if the bus port is snooping or not.
      *
-     * @param id id of the bus port that made the request
-     *
      * @return a boolean indicating if this port is snooping or not
      */
-    bool isSnooping(PortID id) const;
+    bool isSnooping() const;
 
     /** Calculate the timing parameters for the packet.  Updates the
      * firstWordTime and finishTime fields of the packet object.
@@ -423,11 +423,12 @@ class Bus : public MemObject
      */
     void retryWaiting();
 
-    /** Ask everyone on the bus what their size is
-     * @param id id of the busport that made the request
+    /**
+     * Ask everyone on the bus what their size is
+     *
      * @return the max of all the sizes
      */
-    unsigned findBlockSize(PortID id);
+    unsigned findBlockSize();
 
     // event used to schedule a release of the bus
     EventWrapper<Bus, &Bus::releaseBus> busIdleEvent;
diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index c7eb3f80b..3b9bfd35a 100644
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -1193,14 +1193,12 @@ Cache<TagStore>::handleSnoop(PacketPtr pkt, BlkType *blk,
                 pkt->assertShared();
             }
         } else {
-            PortID origSrc = pkt->getSrc();
             cpuSidePort->sendAtomicSnoop(pkt);
             if (!alreadyResponded && pkt->memInhibitAsserted()) {
                 // cache-to-cache response from some upper cache:
                 // forward response to original requester
                 assert(pkt->isResponse());
             }
-            pkt->setSrc(origSrc);
         }
     }