From f49830ce0ba79c54c65c9c4b25bc3c6184aaf2a9 Mon Sep 17 00:00:00 2001 From: Andreas Hansson Date: Thu, 22 Jan 2015 05:00:53 -0500 Subject: 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. --- src/arch/arm/isa.cc | 12 ++--- src/cpu/checker/cpu.cc | 8 +-- src/cpu/kvm/base.cc | 5 +- src/cpu/kvm/base.hh | 3 -- src/cpu/kvm/x86_cpu.cc | 5 +- src/cpu/kvm/x86_cpu.hh | 3 -- src/cpu/simple/timing.cc | 16 +++--- src/cpu/simple/timing.hh | 2 +- src/cpu/testers/memtest/memtest.cc | 6 +-- src/cpu/testers/networktest/networktest.cc | 12 ++--- src/mem/port_proxy.cc | 8 +-- src/mem/request.hh | 82 +++++++++++++++--------------- src/mem/ruby/system/CacheRecorder.cc | 8 +-- 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(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(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), @@ -323,39 +349,13 @@ class Request privateFlags.set(VALID_CONTEXT_ID|VALID_THREAD_ID); } - /** - * 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); } -- cgit v1.2.3