From 25bfc249998b26403d50587eb66e6ee5e6de5b58 Mon Sep 17 00:00:00 2001 From: Andreas Hansson Date: Tue, 2 Dec 2014 06:07:34 -0500 Subject: [PATCH] mem: Remove null-check bypassing in Packet::getPtr This patch removes the parameter that enables bypassing the null check in the Packet::getPtr method. A number of call sites assume the value to be non-null. The one odd case is the RubyTester, which issues zero-sized prefetches(!), and despite being reads they had no valid data pointer. This is now fixed, but the size oddity remains (unless anyone object or has any good suggestions). Finally, in the Ruby Sequencer, appropriate checks are made for flush packets as they have no valid data pointer. --- src/cpu/testers/rubytest/Check.cc | 5 +++++ src/mem/packet.hh | 4 ++-- src/mem/ruby/slicc_interface/RubyRequest.cc | 2 +- .../ruby/slicc_interface/RubySlicc_Util.hh | 4 ++-- src/mem/ruby/system/DMASequencer.cc | 2 +- src/mem/ruby/system/Sequencer.cc | 20 +++++++++---------- 6 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/cpu/testers/rubytest/Check.cc b/src/cpu/testers/rubytest/Check.cc index 126deba6d..9de766077 100644 --- a/src/cpu/testers/rubytest/Check.cc +++ b/src/cpu/testers/rubytest/Check.cc @@ -110,6 +110,11 @@ Check::initiatePrefetch() req->setThreadContext(index, 0); PacketPtr pkt = new Packet(req, cmd); + // despite the oddity of the 0 size (questionable if this should + // even be allowed), a prefetch is still a read and as such needs + // a place to store the result + uint8_t *data = new uint8_t; + pkt->dataDynamic(data); // push the subblock onto the sender state. The sequencer will // update the subblock on the return diff --git a/src/mem/packet.hh b/src/mem/packet.hh index c7b47c0a7..8d84a7ccb 100644 --- a/src/mem/packet.hh +++ b/src/mem/packet.hh @@ -846,9 +846,9 @@ class Packet : public Printable */ template T* - getPtr(bool null_ok = false) + getPtr() { - assert(null_ok || flags.isSet(STATIC_DATA|DYNAMIC_DATA)); + assert(flags.isSet(STATIC_DATA|DYNAMIC_DATA)); return (T*)data; } diff --git a/src/mem/ruby/slicc_interface/RubyRequest.cc b/src/mem/ruby/slicc_interface/RubyRequest.cc index 56feee59d..ff90e415e 100644 --- a/src/mem/ruby/slicc_interface/RubyRequest.cc +++ b/src/mem/ruby/slicc_interface/RubyRequest.cc @@ -72,7 +72,7 @@ RubyRequest::functionalWrite(Packet *pkt) Addr mBase = m_PhysicalAddress.getAddress(); Addr mTail = mBase + m_Size; - uint8_t * pktData = pkt->getPtr(true); + uint8_t * pktData = pkt->getPtr(); Addr cBase = std::max(wBase, mBase); Addr cTail = std::min(wTail, mTail); diff --git a/src/mem/ruby/slicc_interface/RubySlicc_Util.hh b/src/mem/ruby/slicc_interface/RubySlicc_Util.hh index 5ec34f2dc..8e2a1c5b1 100644 --- a/src/mem/ruby/slicc_interface/RubySlicc_Util.hh +++ b/src/mem/ruby/slicc_interface/RubySlicc_Util.hh @@ -107,7 +107,7 @@ testAndRead(Address addr, DataBlock& blk, Packet *pkt) lineAddr.makeLineAddress(); if (pktLineAddr == lineAddr) { - uint8_t *data = pkt->getPtr(true); + uint8_t *data = pkt->getPtr(); unsigned int size_in_bytes = pkt->getSize(); unsigned startByte = pkt->getAddr() - lineAddr.getAddress(); @@ -135,7 +135,7 @@ testAndWrite(Address addr, DataBlock& blk, Packet *pkt) lineAddr.makeLineAddress(); if (pktLineAddr == lineAddr) { - uint8_t *data = pkt->getPtr(true); + uint8_t *data = pkt->getPtr(); unsigned int size_in_bytes = pkt->getSize(); unsigned startByte = pkt->getAddr() - lineAddr.getAddress(); diff --git a/src/mem/ruby/system/DMASequencer.cc b/src/mem/ruby/system/DMASequencer.cc index eb4ce6123..2c4c024b6 100644 --- a/src/mem/ruby/system/DMASequencer.cc +++ b/src/mem/ruby/system/DMASequencer.cc @@ -235,7 +235,7 @@ DMASequencer::makeRequest(PacketPtr pkt) } uint64_t paddr = pkt->getAddr(); - uint8_t* data = pkt->getPtr(true); + uint8_t* data = pkt->getPtr(); int len = pkt->getSize(); bool write = pkt->isWrite(); diff --git a/src/mem/ruby/system/Sequencer.cc b/src/mem/ruby/system/Sequencer.cc index bd82d9468..281ea22be 100644 --- a/src/mem/ruby/system/Sequencer.cc +++ b/src/mem/ruby/system/Sequencer.cc @@ -524,28 +524,23 @@ Sequencer::hitCallback(SequencerRequest* srequest, DataBlock& data, llscSuccess ? "Done" : "SC_Failed", "", "", request_address, total_latency); - // update the data + // update the data unless it is a non-data-carrying flush if (g_system_ptr->m_warmup_enabled) { - assert(pkt->getPtr(false) != NULL); - data.setData(pkt->getPtr(false), + data.setData(pkt->getPtr(), request_address.getOffset(), pkt->getSize()); - } else if (pkt->getPtr(true) != NULL) { + } else if (!pkt->isFlush()) { if ((type == RubyRequestType_LD) || (type == RubyRequestType_IFETCH) || (type == RubyRequestType_RMW_Read) || (type == RubyRequestType_Locked_RMW_Read) || (type == RubyRequestType_Load_Linked)) { - memcpy(pkt->getPtr(true), + memcpy(pkt->getPtr(), data.getData(request_address.getOffset(), pkt->getSize()), pkt->getSize()); } else { - data.setData(pkt->getPtr(true), + data.setData(pkt->getPtr(), request_address.getOffset(), pkt->getSize()); } - } else { - DPRINTF(MemoryAccess, - "WARNING. Data not transfered from Ruby to M5 for type %s\n", - RubyRequestType_to_string(type)); } // If using the RubyTester, update the RubyTester sender state's @@ -679,9 +674,12 @@ Sequencer::issueRequest(PacketPtr pkt, RubyRequestType secondary_type) pc = pkt->req->getPC(); } + // check if the packet has data as for example prefetch and flush + // requests do not std::shared_ptr msg = std::make_shared(clockEdge(), pkt->getAddr(), - pkt->getPtr(true), + pkt->isFlush() ? + nullptr : pkt->getPtr(), pkt->getSize(), pc, secondary_type, RubyAccessMode_Supervisor, pkt, PrefetchBit_No, proc_id);