changeset 842f56345a42 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=842f56345a42
description:
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.
diffstat:
src/cpu/minor/lsq.cc | 6 ++++
src/mem/packet.hh | 71 +++++++++++++--------------------------------------
2 files changed, 24 insertions(+), 53 deletions(-)
diffs (148 lines):
diff -r 54071fd5c397 -r 842f56345a42 src/cpu/minor/lsq.cc
--- a/src/cpu/minor/lsq.cc Fri Aug 21 07:03:25 2015 -0400
+++ b/src/cpu/minor/lsq.cc Fri Aug 21 07:03:27 2015 -0400
@@ -1578,6 +1578,12 @@
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 -r 54071fd5c397 -r 842f56345a42 src/mem/packet.hh
--- a/src/mem/packet.hh Fri Aug 21 07:03:25 2015 -0400
+++ b/src/mem/packet.hh Fri Aug 21 07:03:27 2015 -0400
@@ -255,10 +255,6 @@
// 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 @@
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 @@
* 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 @@
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 @@
/**
* 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 @@
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 @@
}
}
- void
- setSize(unsigned size)
- {
- assert(!flags.isSet(VALID_SIZE));
-
- this->size = size;
- flags.set(VALID_SIZE);
- }
-
-
public:
/**
* @{
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
