mem: Make the requests carried by packets const
authorAndreas Hansson <andreas.hansson@arm.com>
Tue, 2 Dec 2014 11:07:50 +0000 (06:07 -0500)
committerAndreas Hansson <andreas.hansson@arm.com>
Tue, 2 Dec 2014 11:07:50 +0000 (06:07 -0500)
This adds a basic level of sanity checking to the packet by ensuring
that a request is not modified once the packet is created. The only
issue that had to be worked around is the relaying of
software-prefetches in the cache. The specific situation is now solved
by first copying the request, and then creating a new packet
accordingly.

src/mem/cache/cache_impl.hh
src/mem/packet.hh

index 535dc81c20dab46a753cf017fe3f77cbecfd31bb..8e8079d58ce4dfcde1b59d33eca1792d3f01e73d 100644 (file)
@@ -611,18 +611,21 @@ Cache<TagStore>::recvTimingReq(PacketPtr pkt)
             assert(pkt->req->hasPaddr());
 
             // There's no reason to add a prefetch as an additional target
-            // to an existing MSHR.  If an outstanding request is already
+            // to an existing MSHR. If an outstanding request is already
             // in progress, there is nothing for the prefetch to do.
             // If this is the case, we don't even create a request at all.
-            PacketPtr pf = mshr ? NULL : new Packet(pkt);
-
-            if (pf) {
-                pf->req = new Request(pkt->req->getPaddr(),
-                                      pkt->req->getSize(),
-                                      pkt->req->getFlags(),
-                                      pkt->req->masterId());
-                // The core will clean up prior senderState; we need our own.
-                pf->senderState = NULL;
+            PacketPtr pf = nullptr;
+
+            if (!mshr) {
+                // copy the request and create a new SoftPFReq packet
+                RequestPtr req = new Request(pkt->req->getPaddr(),
+                                             pkt->req->getSize(),
+                                             pkt->req->getFlags(),
+                                             pkt->req->masterId());
+                pf = new Packet(req, pkt->cmd);
+                pf->allocate();
+                assert(pf->getAddr() == pkt->getAddr());
+                assert(pf->getSize() == pkt->getSize());
             }
 
             pkt->makeTimingResponse();
@@ -632,6 +635,8 @@ Cache<TagStore>::recvTimingReq(PacketPtr pkt)
             std::memset(pkt->getPtr<uint8_t>(), 0xFF, pkt->getSize());
             cpuSidePort->schedTimingResp(pkt, clockEdge(lat));
 
+            // If an outstanding request is in progress (we found an
+            // MSHR) this is set to null
             pkt = pf;
         }
 
index b212de7c8747986ddab696696cea7e691b3b26b7..357134c752b16542d727ca9a4abb5c49812ae4d3 100644 (file)
@@ -265,7 +265,7 @@ class Packet : public Printable
     MemCmd cmd;
 
     /// A pointer to the original request.
-    RequestPtr req;
+    const RequestPtr req;
 
   private:
    /**
@@ -600,7 +600,7 @@ class Packet : public Printable
      * first, but the Requests's physical address and size fields need
      * not be valid. The command must be supplied.
      */
-    Packet(Request *_req, MemCmd _cmd)
+    Packet(const RequestPtr _req, MemCmd _cmd)
         :  cmd(_cmd), req(_req), data(nullptr), addr(0), _isSecure(false),
            size(0), src(InvalidPortID), dest(InvalidPortID),
            bytesValidStart(0), bytesValidEnd(0),
@@ -623,7 +623,7 @@ class Packet : public Printable
      * 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, int _blkSize)
+    Packet(const RequestPtr _req, MemCmd _cmd, int _blkSize)
         :  cmd(_cmd), req(_req), data(nullptr), addr(0), _isSecure(false),
            src(InvalidPortID), dest(InvalidPortID),
            bytesValidStart(0), bytesValidEnd(0),
@@ -646,7 +646,7 @@ class Packet : public Printable
      * less than that of the original packet.  In this case the new
      * packet should allocate its own data.
      */
-    Packet(Packet *pkt, bool clearFlags = false)
+    Packet(PacketPtr pkt, bool clearFlags = false)
         :  cmd(pkt->cmd), req(pkt->req),
            data(pkt->flags.isSet(STATIC_DATA) ? pkt->data : NULL),
            addr(pkt->addr), _isSecure(pkt->_isSecure), size(pkt->size),
@@ -696,7 +696,7 @@ class Packet : public Printable
      * vanilla read or write.
      */
     static PacketPtr
-    createRead(Request *req)
+    createRead(const RequestPtr req)
     {
         PacketPtr pkt = new Packet(req, MemCmd::ReadReq);
         pkt->refineCommand();
@@ -704,7 +704,7 @@ class Packet : public Printable
     }
 
     static PacketPtr
-    createWrite(Request *req)
+    createWrite(const RequestPtr req)
     {
         PacketPtr pkt = new Packet(req, MemCmd::WriteReq);
         pkt->refineCommand();
@@ -724,33 +724,6 @@ class Packet : public Printable
         deleteData();
     }
 
-    /**
-     * Reinitialize packet address and size from the associated
-     * Request object, and reset other fields that may have been
-     * modified by a previous transaction.  Typically called when a
-     * statically allocated Request/Packet pair is reused for multiple
-     * transactions.
-     */
-    void
-    reinitFromRequest()
-    {
-        assert(req->hasPaddr());
-        flags = 0;
-        addr = req->getPaddr();
-        _isSecure = req->isSecure();
-        size = req->getSize();
-
-        src = InvalidPortID;
-        dest = InvalidPortID;
-        bytesValidStart = 0;
-        bytesValidEnd = 0;
-        firstWordDelay = 0;
-        lastWordDelay = 0;
-
-        flags.set(VALID_ADDR|VALID_SIZE);
-        deleteData();
-    }
-
     /**
      * Take a request packet and modify it in place to be suitable for
      * returning as a response to that request. The source field is