mem: Remove null-check bypassing in Packet::getPtr
authorAndreas Hansson <andreas.hansson@arm.com>
Tue, 2 Dec 2014 11:07:34 +0000 (06:07 -0500)
committerAndreas Hansson <andreas.hansson@arm.com>
Tue, 2 Dec 2014 11:07:34 +0000 (06:07 -0500)
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
src/mem/packet.hh
src/mem/ruby/slicc_interface/RubyRequest.cc
src/mem/ruby/slicc_interface/RubySlicc_Util.hh
src/mem/ruby/system/DMASequencer.cc
src/mem/ruby/system/Sequencer.cc

index 126deba6daff8399158fe2e4eff829f838363823..9de766077caabb665f415148e60532965d0b7940 100644 (file)
@@ -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
index c7b47c0a72acf064263a9dee59194226bfe27e9f..8d84a7ccbfea119c135ef8ff4b04522f67de7f2e 100644 (file)
@@ -846,9 +846,9 @@ class Packet : public Printable
      */
     template <typename T>
     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;
     }
 
index 56feee59d85168cd3a33e056174f986f97464722..ff90e415e2a3334e4925ab1ba6fa08f682556331 100644 (file)
@@ -72,7 +72,7 @@ RubyRequest::functionalWrite(Packet *pkt)
     Addr mBase = m_PhysicalAddress.getAddress();
     Addr mTail = mBase + m_Size;
 
-    uint8_t * pktData = pkt->getPtr<uint8_t>(true);
+    uint8_t * pktData = pkt->getPtr<uint8_t>();
 
     Addr cBase = std::max(wBase, mBase);
     Addr cTail = std::min(wTail, mTail);
index 5ec34f2dc631c741cb209699dd7fc493371f7367..8e2a1c5b16cce227394326950b7da12c9968623a 100644 (file)
@@ -107,7 +107,7 @@ testAndRead(Address addr, DataBlock& blk, Packet *pkt)
     lineAddr.makeLineAddress();
 
     if (pktLineAddr == lineAddr) {
-        uint8_t *data = pkt->getPtr<uint8_t>(true);
+        uint8_t *data = pkt->getPtr<uint8_t>();
         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<uint8_t>(true);
+        uint8_t *data = pkt->getPtr<uint8_t>();
         unsigned int size_in_bytes = pkt->getSize();
         unsigned startByte = pkt->getAddr() - lineAddr.getAddress();
 
index eb4ce61234ee4691be21133ad33fde5aed19a0d5..2c4c024b61e2e898de4de8da1ffa2daf8233f885 100644 (file)
@@ -235,7 +235,7 @@ DMASequencer::makeRequest(PacketPtr pkt)
     }
 
     uint64_t paddr = pkt->getAddr();
-    uint8_t* data =  pkt->getPtr<uint8_t>(true);
+    uint8_t* data =  pkt->getPtr<uint8_t>();
     int len = pkt->getSize();
     bool write = pkt->isWrite();
 
index bd82d946889777e1efa9aa65d239d2d9bba349b6..281ea22bef29e53ee77d1a3458c6000e32694e91 100644 (file)
@@ -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<uint8_t>(false) != NULL);
-        data.setData(pkt->getPtr<uint8_t>(false),
+        data.setData(pkt->getPtr<uint8_t>(),
                      request_address.getOffset(), pkt->getSize());
-    } else if (pkt->getPtr<uint8_t>(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<uint8_t>(true),
+            memcpy(pkt->getPtr<uint8_t>(),
                    data.getData(request_address.getOffset(), pkt->getSize()),
                    pkt->getSize());
         } else {
-            data.setData(pkt->getPtr<uint8_t>(true),
+            data.setData(pkt->getPtr<uint8_t>(),
                          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<RubyRequest> msg =
         std::make_shared<RubyRequest>(clockEdge(), pkt->getAddr(),
-                                      pkt->getPtr<uint8_t>(true),
+                                      pkt->isFlush() ?
+                                      nullptr : pkt->getPtr<uint8_t>(),
                                       pkt->getSize(), pc, secondary_type,
                                       RubyAccessMode_Supervisor, pkt,
                                       PrefetchBit_No, proc_id);