From daaae3744d119de836d5260205dd34990e741608 Mon Sep 17 00:00:00 2001
From: Andreas Hansson <andreas.hansson@arm.com>
Date: Fri, 21 Aug 2015 07:03:27 -0400
Subject: [PATCH] mem: Reflect that packet address and size are always valid

This patch simplifies the packet, and removes the possibility of
creating a packet without a valid address and/or size. Under no
circumstances are these fields set at a later point, and thus they
really have to be provided at construction time.

The patch also fixes a case there the MinorCPU creates a packet
without a valid address and size, only to later delete it.
---
 src/cpu/minor/lsq.cc |  6 ++++
 src/mem/packet.hh    | 71 +++++++++++---------------------------------
 2 files changed, 24 insertions(+), 53 deletions(-)

diff --git a/src/cpu/minor/lsq.cc b/src/cpu/minor/lsq.cc
index db57daa37..376e8a0ff 100644
--- a/src/cpu/minor/lsq.cc
+++ b/src/cpu/minor/lsq.cc
@@ -1578,6 +1578,12 @@ LSQ::LSQRequest::makePacket()
     if (packet)
         return;
 
+    // if the translation faulted, do not create a packet
+    if (fault != NoFault) {
+        assert(packet == NULL);
+        return;
+    }
+
     packet = makePacketForRequest(request, isLoad, this, data);
     /* Null the ret data so we know not to deallocate it when the
      * ret is destroyed.  The data now belongs to the ret and
diff --git a/src/mem/packet.hh b/src/mem/packet.hh
index 2b667d26d..879a43c2d 100644
--- a/src/mem/packet.hh
+++ b/src/mem/packet.hh
@@ -255,10 +255,6 @@ class Packet : public Printable
         // Snoop response flags
         MEM_INHIBIT            = 0x00000008,
 
-        /// Are the 'addr' and 'size' fields valid?
-        VALID_ADDR             = 0x00000100,
-        VALID_SIZE             = 0x00000200,
-
         /// Is the data pointer set to a value that shouldn't be freed
         /// when the packet is destroyed?
         STATIC_DATA            = 0x00001000,
@@ -523,7 +519,7 @@ class Packet : public Printable
 
     void copyError(Packet *pkt) { assert(pkt->isError()); cmd = pkt->cmd; }
 
-    Addr getAddr() const { assert(flags.isSet(VALID_ADDR)); return addr; }
+    Addr getAddr() const { return addr; }
     /**
      * Update the address of this packet mid-transaction. This is used
      * by the address mapper to change an already set address to a new
@@ -531,9 +527,9 @@ class Packet : public Printable
      * an existing address, so it asserts that the current address is
      * valid.
      */
-    void setAddr(Addr _addr) { assert(flags.isSet(VALID_ADDR)); addr = _addr; }
+    void setAddr(Addr _addr) { addr = _addr; }
 
-    unsigned getSize() const  { assert(flags.isSet(VALID_SIZE)); return size; }
+    unsigned getSize() const  { return size; }
 
     Addr getOffset(unsigned int blk_size) const
     {
@@ -545,11 +541,7 @@ class Packet : public Printable
         return getAddr() & ~(Addr(blk_size - 1));
     }
 
-    bool isSecure() const
-    {
-        assert(flags.isSet(VALID_ADDR));
-        return _isSecure;
-    }
+    bool isSecure() const { return _isSecure; }
 
     /**
      * It has been determined that the SC packet should successfully update
@@ -577,43 +569,28 @@ class Packet : public Printable
 
     /**
      * 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 must be supplied.
+     * first, and have a valid physical address and size. The command
+     * must be supplied.
      */
     Packet(const RequestPtr _req, MemCmd _cmd)
-        :  cmd(_cmd), req(_req), data(nullptr), addr(0), _isSecure(false),
-           size(0), headerDelay(0), payloadDelay(0),
+        :  cmd(_cmd), req(_req), data(nullptr), addr(req->getPaddr()),
+           _isSecure(req->isSecure()), size(req->getSize()),
+           headerDelay(0), payloadDelay(0),
            senderState(NULL)
-    {
-        if (req->hasPaddr()) {
-            addr = req->getPaddr();
-            flags.set(VALID_ADDR);
-            _isSecure = req->isSecure();
-        }
-        if (req->hasSize()) {
-            size = req->getSize();
-            flags.set(VALID_SIZE);
-        }
-    }
+    { }
 
     /**
-     * Alternate constructor if you are trying to create a packet with
-     * a request that is for a whole block, not the address from the
-     * req.  this allows for overriding the size/addr of the req.
+     * Alternate constructor when creating a packet that is for a
+     * whole block. This allows for overriding the size and addr of
+     * the request.
      */
-    Packet(const RequestPtr _req, MemCmd _cmd, int _blkSize)
-        :  cmd(_cmd), req(_req), data(nullptr), addr(0), _isSecure(false),
+    Packet(const RequestPtr _req, MemCmd _cmd, unsigned _blkSize)
+        :  cmd(_cmd), req(_req), data(nullptr),
+           addr(_req->getPaddr() & ~Addr(_blkSize - 1)),
+           _isSecure(_req->isSecure()), size(_blkSize),
            headerDelay(0), payloadDelay(0),
            senderState(NULL)
-    {
-        if (req->hasPaddr()) {
-            addr = req->getPaddr() & ~(_blkSize - 1);
-            flags.set(VALID_ADDR);
-            _isSecure = req->isSecure();
-        }
-        size = _blkSize;
-        flags.set(VALID_SIZE);
-    }
+    { }
 
     /**
      * Alternate constructor for copying a packet.  Copy all fields
@@ -634,8 +611,6 @@ class Packet : public Printable
         if (!clear_flags)
             flags.set(pkt->flags & COPY_FLAGS);
 
-        flags.set(pkt->flags & (VALID_ADDR|VALID_SIZE));
-
         // should we allocate space for data, or not, the express
         // snoops do not need to carry any data as they only serve to
         // co-ordinate state changes
@@ -757,16 +732,6 @@ class Packet : public Printable
         }
     }
 
-    void
-    setSize(unsigned size)
-    {
-        assert(!flags.isSet(VALID_SIZE));
-
-        this->size = size;
-        flags.set(VALID_SIZE);
-    }
-
-
   public:
     /**
      * @{
-- 
2.30.2