mem: Clean up Request initialisation

This patch tidies up how we create and set the fields of a Request. In
essence it tries to use the constructor where possible (as opposed to
setPhys and setVirt), thus avoiding spreading the information across a
number of locations. In fact, setPhys is made private as part of this
patch, and a number of places where we callede setVirt instead uses
the appropriate constructor.

13 files changed, 76 insertions, 94 deletions

diff --git a/src/arch/arm/isa.cc b/src/arch/arm/isa.cc
index 6bbd55195..1198f852f 100644
--- a/src/arch/arm/isa.cc
+++ b/src/arch/arm/isa.cc
@@ -1490,7 +1490,6 @@ ISA::setMiscReg(int misc_reg, const MiscReg &val, ThreadContext *tc)
           case MISCREG_ATS1HR:
           case MISCREG_ATS1HW:
             {
-              RequestPtr req = new Request;
               unsigned flags = 0;
               BaseTLB::Mode mode = BaseTLB::Read;
               TLB::ArmTranslationType tranType = TLB::NormalTran;
@@ -1562,16 +1561,16 @@ ISA::setMiscReg(int misc_reg, const MiscReg &val, ThreadContext *tc)
               // can't be an atomic translation because that causes problems
               // with unexpected atomic snoop requests.
               warn("Translating via MISCREG(%d) in functional mode! Fix Me!\n", misc_reg);
-              req->setVirt(0, val, 1, flags,  Request::funcMasterId,
-                           tc->pcState().pc());
-              req->setThreadContext(tc->contextId(), tc->threadId());
-              fault = tc->getDTBPtr()->translateFunctional(req, tc, mode, tranType);
+              Request req(0, val, 1, flags,  Request::funcMasterId,
+                          tc->pcState().pc(), tc->contextId(),
+                          tc->threadId());
+              fault = tc->getDTBPtr()->translateFunctional(&req, tc, mode, tranType);
               TTBCR ttbcr = readMiscRegNoEffect(MISCREG_TTBCR);
               HCR   hcr   = readMiscRegNoEffect(MISCREG_HCR);
 
               MiscReg newVal;
               if (fault == NoFault) {
-                  Addr paddr = req->getPaddr();
+                  Addr paddr = req.getPaddr();
                   if (haveLPAE && (ttbcr.eae || tranType & TLB::HypMode ||
                      ((tranType & TLB::S1S2NsTran) && hcr.vm) )) {
                       newVal = (paddr & mask(39, 12)) |
@@ -1605,7 +1604,6 @@ ISA::setMiscReg(int misc_reg, const MiscReg &val, ThreadContext *tc)
                           "MISCREG: Translated addr 0x%08x fault fsr %#x: PAR: 0x%08x\n",
                           val, fsr, newVal);
               }
-              delete req;
               setMiscRegNoEffect(MISCREG_PAR, newVal);
               return;
             }
diff --git a/src/cpu/checker/cpu.cc b/src/cpu/checker/cpu.cc
index f91bad294..d6a8bd032 100644
--- a/src/cpu/checker/cpu.cc
+++ b/src/cpu/checker/cpu.cc
@@ -154,8 +154,8 @@ CheckerCPU::readMem(Addr addr, uint8_t *data, unsigned size, unsigned flags)
 
     // Need to account for multiple accesses like the Atomic and TimingSimple
     while (1) {
-        memReq = new Request();
-        memReq->setVirt(0, addr, size, flags, masterId, thread->pcState().instAddr());
+        memReq = new Request(0, addr, size, flags, masterId,
+                             thread->pcState().instAddr(), tc->contextId(), 0);
 
         // translate to physical address
         fault = dtb->translateFunctional(memReq, tc, BaseTLB::Read);
@@ -242,8 +242,8 @@ CheckerCPU::writeMem(uint8_t *data, unsigned size,
 
     // Need to account for a multiple access like Atomic and Timing CPUs
     while (1) {
-        memReq = new Request();
-        memReq->setVirt(0, addr, size, flags, masterId, thread->pcState().instAddr());
+        memReq = new Request(0, addr, size, flags, masterId,
+                             thread->pcState().instAddr(), tc->contextId(), 0);
 
         // translate to physical address
         fault = dtb->translateFunctional(memReq, tc, BaseTLB::Write);
diff --git a/src/cpu/kvm/base.cc b/src/cpu/kvm/base.cc
index 95d91467e..e09c4b7f2 100644
--- a/src/cpu/kvm/base.cc
+++ b/src/cpu/kvm/base.cc
@@ -118,8 +118,6 @@ BaseKvmCPU::init()
     // initialize CPU, including PC
     if (FullSystem && !switchedOut())
         TheISA::initCPU(tc, tc->contextId());
-
-    mmio_req.setThreadContext(tc->contextId(), 0);
 }
 
 void
@@ -995,7 +993,8 @@ BaseKvmCPU::doMMIOAccess(Addr paddr, void *data, int size, bool write)
     ThreadContext *tc(thread->getTC());
     syncThreadContext();
 
-    mmio_req.setPhys(paddr, size, Request::UNCACHEABLE, dataMasterId());
+    Request mmio_req(paddr, size, Request::UNCACHEABLE, dataMasterId());
+    mmio_req.setThreadContext(tc->contextId(), 0);
     // Some architectures do need to massage physical addresses a bit
     // before they are inserted into the memory system. This enables
     // APIC accesses on x86 and m5ops where supported through a MMIO
diff --git a/src/cpu/kvm/base.hh b/src/cpu/kvm/base.hh
index 249398293..dac4934cb 100644
--- a/src/cpu/kvm/base.hh
+++ b/src/cpu/kvm/base.hh
@@ -574,9 +574,6 @@ class BaseKvmCPU : public BaseCPU
     /** Unused dummy port for the instruction interface */
     KVMCpuPort instPort;
 
-    /** Pre-allocated MMIO memory request */
-    Request mmio_req;
-
     /**
      * Is the gem5 context dirty? Set to true to force an update of
      * the KVM vCPU state upon the next call to kvmRun().
diff --git a/src/cpu/kvm/x86_cpu.cc b/src/cpu/kvm/x86_cpu.cc
index 3e736a913..34b51f137 100644
--- a/src/cpu/kvm/x86_cpu.cc
+++ b/src/cpu/kvm/x86_cpu.cc
@@ -554,8 +554,6 @@ X86KvmCPU::startup()
 
     updateCPUID();
 
-    io_req.setThreadContext(tc->contextId(), 0);
-
     // TODO: Do we need to create an identity mapped TSS area? We
     // should call kvm.vm.setTSSAddress() here in that case. It should
     // only be needed for old versions of the virtualization
@@ -1346,8 +1344,9 @@ X86KvmCPU::handleKvmExitIO()
         pAddr = X86ISA::x86IOAddress(port);
     }
 
-    io_req.setPhys(pAddr, kvm_run.io.size, Request::UNCACHEABLE,
+    Request io_req(pAddr, kvm_run.io.size, Request::UNCACHEABLE,
                    dataMasterId());
+    io_req.setThreadContext(tc->contextId(), 0);
 
     const MemCmd cmd(isWrite ? MemCmd::WriteReq : MemCmd::ReadReq);
     // Temporarily lock and migrate to the event queue of the
diff --git a/src/cpu/kvm/x86_cpu.hh b/src/cpu/kvm/x86_cpu.hh
index bfd090ff7..18471040c 100644
--- a/src/cpu/kvm/x86_cpu.hh
+++ b/src/cpu/kvm/x86_cpu.hh
@@ -234,9 +234,6 @@ class X86KvmCPU : public BaseKvmCPU
      */
     void handleIOMiscReg32(int miscreg);
 
-    /** Reusable IO request */
-    Request io_req;
-
     /** Cached intersection of supported MSRs */
     mutable Kvm::MSRIndexVector cachedMsrIntersection;
 
diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple/timing.cc
index c7db5c4f8..8c90d7c4e 100644
--- a/src/cpu/simple/timing.cc
+++ b/src/cpu/simple/timing.cc
@@ -270,8 +270,7 @@ void
 TimingSimpleCPU::sendData(RequestPtr req, uint8_t *data, uint64_t *res,
                           bool read)
 {
-    PacketPtr pkt;
-    buildPacket(pkt, req, read);
+    PacketPtr pkt = buildPacket(req, read);
     pkt->dataDynamic<uint8_t>(data);
     if (req->getFlags().isSet(Request::NO_ACCESS)) {
         assert(!dcache_pkt);
@@ -354,10 +353,10 @@ TimingSimpleCPU::translationFault(const Fault &fault)
     advanceInst(fault);
 }
 
-void
-TimingSimpleCPU::buildPacket(PacketPtr &pkt, RequestPtr req, bool read)
+PacketPtr
+TimingSimpleCPU::buildPacket(RequestPtr req, bool read)
 {
-    pkt = read ? Packet::createRead(req) : Packet::createWrite(req);
+    return read ? Packet::createRead(req) : Packet::createWrite(req);
 }
 
 void
@@ -370,14 +369,13 @@ TimingSimpleCPU::buildSplitPacket(PacketPtr &pkt1, PacketPtr &pkt2,
     assert(!req1->isMmappedIpr() && !req2->isMmappedIpr());
 
     if (req->getFlags().isSet(Request::NO_ACCESS)) {
-        buildPacket(pkt1, req, read);
+        pkt1 = buildPacket(req, read);
         return;
     }
 
-    buildPacket(pkt1, req1, read);
-    buildPacket(pkt2, req2, read);
+    pkt1 = buildPacket(req1, read);
+    pkt2 = buildPacket(req2, read);
 
-    req->setPhys(req1->getPaddr(), req->getSize(), req1->getFlags(), dataMasterId());
     PacketPtr pkt = new Packet(req, pkt1->cmd.responseCommand());
 
     pkt->dataDynamic<uint8_t>(data);
diff --git a/src/cpu/simple/timing.hh b/src/cpu/simple/timing.hh
index 52eb6b1ba..d8460515b 100644
--- a/src/cpu/simple/timing.hh
+++ b/src/cpu/simple/timing.hh
@@ -137,7 +137,7 @@ class TimingSimpleCPU : public BaseSimpleCPU
 
     void translationFault(const Fault &fault);
 
-    void buildPacket(PacketPtr &pkt, RequestPtr req, bool read);
+    PacketPtr buildPacket(RequestPtr req, bool read);
     void buildSplitPacket(PacketPtr &pkt1, PacketPtr &pkt2,
             RequestPtr req1, RequestPtr req2, RequestPtr req,
             uint8_t *data, bool read);
diff --git a/src/cpu/testers/memtest/memtest.cc b/src/cpu/testers/memtest/memtest.cc
index 53c01b7f7..a94f6950d 100644
--- a/src/cpu/testers/memtest/memtest.cc
+++ b/src/cpu/testers/memtest/memtest.cc
@@ -300,16 +300,16 @@ MemTest::tick()
 
     bool do_functional = (random_mt.random(0, 100) < percentFunctional) &&
         !uncacheable;
-    Request *req = new Request();
+    Request *req = nullptr;
     uint8_t *result = new uint8_t[8];
 
     if (issueDmas) {
         paddr &= ~((1 << dma_access_size) - 1);
-        req->setPhys(paddr, 1 << dma_access_size, flags, masterId);
+        req = new Request(paddr, 1 << dma_access_size, flags, masterId);
         req->setThreadContext(id,0);
     } else {
         paddr &= ~((1 << access_size) - 1);
-        req->setPhys(paddr, 1 << access_size, flags, masterId);
+        req = new Request(paddr, 1 << access_size, flags, masterId);
         req->setThreadContext(id,0);
     }
     assert(req->getSize() == 1);
diff --git a/src/cpu/testers/networktest/networktest.cc b/src/cpu/testers/networktest/networktest.cc
index 8ad32d140..4a79d5a17 100644
--- a/src/cpu/testers/networktest/networktest.cc
+++ b/src/cpu/testers/networktest/networktest.cc
@@ -198,9 +198,6 @@ NetworkTest::generatePkt()
         destination = dest_y*networkDimension + dest_x;
     }
 
-    Request *req = new Request();
-    Request::Flags flags;
-
     // The source of the packets is a cache.
     // The destination of the packets is a directory.
     // The destination bits are embedded in the address after byte-offset.
@@ -234,21 +231,24 @@ NetworkTest::generatePkt()
     // 
     MemCmd::Command requestType;
 
+    Request *req = nullptr;
+    Request::Flags flags;
+
     unsigned randomReqType = random_mt.random(0, 2);
     if (randomReqType == 0) {
         // generate packet for virtual network 0
         requestType = MemCmd::ReadReq;
-        req->setPhys(paddr, access_size, flags, masterId);
+        req = new Request(paddr, access_size, flags, masterId);
     } else if (randomReqType == 1) {
         // generate packet for virtual network 1
         requestType = MemCmd::ReadReq;
         flags.set(Request::INST_FETCH);
-        req->setVirt(0, 0x0, access_size, flags, 0x0, masterId);
+        req = new Request(0, 0x0, access_size, flags, masterId, 0x0, 0, 0);
         req->setPaddr(paddr);
     } else {  // if (randomReqType == 2)
         // generate packet for virtual network 2
         requestType = MemCmd::WriteReq;
-        req->setPhys(paddr, access_size, flags, masterId);
+        req = new Request(paddr, access_size, flags, masterId);
     }
 
     req->setThreadContext(id,0);
diff --git a/src/mem/port_proxy.cc b/src/mem/port_proxy.cc
index cce8f6ff4..f0158ec2d 100644
--- a/src/mem/port_proxy.cc
+++ b/src/mem/port_proxy.cc
@@ -43,11 +43,9 @@
 void
 PortProxy::readBlob(Addr addr, uint8_t *p, int size) const
 {
-    Request req;
-
     for (ChunkGenerator gen(addr, size, _cacheLineSize); !gen.done();
          gen.next()) {
-        req.setPhys(gen.addr(), gen.size(), 0, Request::funcMasterId);
+        Request req(gen.addr(), gen.size(), 0, Request::funcMasterId);
         Packet pkt(&req, MemCmd::ReadReq);
         pkt.dataStatic(p);
         _port.sendFunctional(&pkt);
@@ -58,11 +56,9 @@ PortProxy::readBlob(Addr addr, uint8_t *p, int size) const
 void
 PortProxy::writeBlob(Addr addr, const uint8_t *p, int size) const
 {
-    Request req;
-
     for (ChunkGenerator gen(addr, size, _cacheLineSize); !gen.done();
          gen.next()) {
-        req.setPhys(gen.addr(), gen.size(), 0, Request::funcMasterId);
+        Request req(gen.addr(), gen.size(), 0, Request::funcMasterId);
         Packet pkt(&req, MemCmd::WriteReq);
         pkt.dataStaticConst(p);
         _port.sendFunctional(&pkt);
diff --git a/src/mem/request.hh b/src/mem/request.hh
index 15798ecc0..e844a6edb 100644
--- a/src/mem/request.hh
+++ b/src/mem/request.hh
@@ -198,6 +198,28 @@ class Request
         VALID_CONTEXT_ID | VALID_THREAD_ID;
 
   private:
+
+    /**
+     * Set up a physical (e.g. device) request in a previously
+     * allocated Request object.
+     */
+    void
+    setPhys(Addr paddr, unsigned size, Flags flags, MasterID mid, Tick time)
+    {
+        assert(size >= 0);
+        _paddr = paddr;
+        _size = size;
+        _time = time;
+        _masterId = mid;
+        _flags.clear(~STICKY_FLAGS);
+        _flags.set(flags);
+        privateFlags.clear(~STICKY_PRIVATE_FLAGS);
+        privateFlags.set(VALID_PADDR|VALID_SIZE);
+        depth = 0;
+        accessDelta = 0;
+        //translateDelta = 0;
+    }
+
     /**
      * The physical address of the request. Valid only if validPaddr
      * is set.
@@ -209,7 +231,7 @@ class Request
      * paddr is written via setVirt() or setPhys(), so it is always
      * valid as long as one of the address fields is valid.
      */
-    int _size;
+    unsigned _size;
 
     /** The requestor ID which is unique in the system for all ports
      * that are capable of issuing a transaction
@@ -254,9 +276,11 @@ class Request
     Addr _pc;
 
   public:
-    /** Minimal constructor.  No fields are initialized. 
-     *  (Note that _flags and privateFlags are cleared by Flags
-     *  default constructor.)
+
+    /**
+     * Minimal constructor. No fields are initialized. (Note that
+     *  _flags and privateFlags are cleared by Flags default
+     *  constructor.)
      */
     Request()
         : _paddr(0), _size(0), _masterId(invldMasterId), _time(0),
@@ -268,18 +292,18 @@ class Request
     /**
      * Constructor for physical (e.g. device) requests.  Initializes
      * just physical address, size, flags, and timestamp (to curTick()).
-     * These fields are adequate to perform a request. 
+     * These fields are adequate to perform a request.
      */
-    Request(Addr paddr, int size, Flags flags, MasterID mid)
+    Request(Addr paddr, unsigned size, Flags flags, MasterID mid)
         : _paddr(0), _size(0), _masterId(invldMasterId), _time(0),
           _taskId(ContextSwitchTaskId::Unknown), _asid(0), _vaddr(0),
           _extraData(0), _contextId(0), _threadId(0), _pc(0),
           translateDelta(0), accessDelta(0), depth(0)
     {
-        setPhys(paddr, size, flags, mid);
+        setPhys(paddr, size, flags, mid, curTick());
     }
 
-    Request(Addr paddr, int size, Flags flags, MasterID mid, Tick time)
+    Request(Addr paddr, unsigned size, Flags flags, MasterID mid, Tick time)
         : _paddr(0), _size(0), _masterId(invldMasterId), _time(0),
           _taskId(ContextSwitchTaskId::Unknown), _asid(0), _vaddr(0),
           _extraData(0), _contextId(0), _threadId(0), _pc(0),
@@ -288,7 +312,8 @@ class Request
         setPhys(paddr, size, flags, mid, time);
     }
 
-    Request(Addr paddr, int size, Flags flags, MasterID mid, Tick time, Addr pc)
+    Request(Addr paddr, unsigned size, Flags flags, MasterID mid, Tick time,
+            Addr pc)
         : _paddr(0), _size(0), _masterId(invldMasterId), _time(0),
           _taskId(ContextSwitchTaskId::Unknown), _asid(0), _vaddr(0),
           _extraData(0), _contextId(0), _threadId(0), _pc(0),
@@ -299,7 +324,8 @@ class Request
         _pc = pc;
     }
 
-    Request(int asid, Addr vaddr, int size, Flags flags, MasterID mid, Addr pc,
+    Request(int asid, Addr vaddr, unsigned size, Flags flags, MasterID mid,
+            Addr pc,
             int cid, ThreadID tid)
         : _paddr(0), _size(0), _masterId(invldMasterId), _time(0),
           _taskId(ContextSwitchTaskId::Unknown), _asid(0), _vaddr(0),
@@ -324,38 +350,12 @@ class Request
     }
 
     /**
-     * Set up a physical (e.g. device) request in a previously
-     * allocated Request object.
-     */
-    void
-    setPhys(Addr paddr, int size, Flags flags, MasterID mid, Tick time)
-    {
-        assert(size >= 0);
-        _paddr = paddr;
-        _size = size;
-        _time = time;
-        _masterId = mid;
-        _flags.clear(~STICKY_FLAGS);
-        _flags.set(flags);
-        privateFlags.clear(~STICKY_PRIVATE_FLAGS);
-        privateFlags.set(VALID_PADDR|VALID_SIZE);
-        depth = 0;
-        accessDelta = 0;
-        //translateDelta = 0;
-    }
-
-    void
-    setPhys(Addr paddr, int size, Flags flags, MasterID mid)
-    {
-        setPhys(paddr, size, flags, mid, curTick());
-    }
-
-    /**
      * Set up a virtual (e.g., CPU) request in a previously
      * allocated Request object.
      */
     void
-    setVirt(int asid, Addr vaddr, int size, Flags flags, MasterID mid, Addr pc)
+    setVirt(int asid, Addr vaddr, unsigned size, Flags flags, MasterID mid,
+            Addr pc)
     {
         assert(size >= 0);
         _asid = asid;
@@ -397,10 +397,8 @@ class Request
         assert(privateFlags.isSet(VALID_VADDR));
         assert(privateFlags.noneSet(VALID_PADDR));
         assert(split_addr > _vaddr && split_addr < _vaddr + _size);
-        req1 = new Request;
-        *req1 = *this;
-        req2 = new Request;
-        *req2 = *this;
+        req1 = new Request(*this);
+        req2 = new Request(*this);
         req1->_size = split_addr - _vaddr;
         req2->_vaddr = split_addr;
         req2->_size = _size - req1->_size;
diff --git a/src/mem/ruby/system/CacheRecorder.cc b/src/mem/ruby/system/CacheRecorder.cc
index 542d91aef..ab7d1cc91 100644
--- a/src/mem/ruby/system/CacheRecorder.cc
+++ b/src/mem/ruby/system/CacheRecorder.cc
@@ -109,21 +109,21 @@ CacheRecorder::enqueueNextFetchRequest()
 
         for (int rec_bytes_read = 0; rec_bytes_read < m_block_size_bytes;
                 rec_bytes_read += RubySystem::getBlockSizeBytes()) {
-            Request* req = new Request();
+            Request* req = nullptr;
             MemCmd::Command requestType;
 
             if (traceRecord->m_type == RubyRequestType_LD) {
                 requestType = MemCmd::ReadReq;
-                req->setPhys(traceRecord->m_data_address + rec_bytes_read,
+                req = new Request(traceRecord->m_data_address + rec_bytes_read,
                     RubySystem::getBlockSizeBytes(), 0, Request::funcMasterId);
             }   else if (traceRecord->m_type == RubyRequestType_IFETCH) {
                 requestType = MemCmd::ReadReq;
-                req->setPhys(traceRecord->m_data_address + rec_bytes_read,
+                req = new Request(traceRecord->m_data_address + rec_bytes_read,
                         RubySystem::getBlockSizeBytes(),
                         Request::INST_FETCH, Request::funcMasterId);
             }   else {
                 requestType = MemCmd::WriteReq;
-                req->setPhys(traceRecord->m_data_address + rec_bytes_read,
+                req = new Request(traceRecord->m_data_address + rec_bytes_read,
                     RubySystem::getBlockSizeBytes(), 0, Request::funcMasterId);
             }