From adde4c91f99fa99abbdd23cf97c45de020429d2d Mon Sep 17 00:00:00 2001
From: "Daniel R. Carvalho" <odanrc@yahoo.com.br>
Date: Mon, 15 Oct 2018 11:48:32 +0200
Subject: [PATCH] mem: Use Packet writing functions instead of memcpy

Classes were using memcpy instead of the Packet functions
created for writing to/from the packet. This allows these
writes to be better checked and tracked.

This also fixes a bug in MemCheckerMonitor, which was using
the incorrect type for the packet pointer.

Change-Id: I5bbc8a24e59464e8219bb6d54af8209e6d4ee1af
Signed-off-by: Daniel R. Carvalho <odanrc@yahoo.com.br>
Reviewed-on: https://gem5-review.googlesource.com/c/13695
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Maintainer: Nikos Nikoleris <nikos.nikoleris@arm.com>
---
 src/mem/abstract_mem.cc        | 28 +++++++++++++++-------------
 src/mem/cache/base.cc          |  3 +--
 src/mem/mem_checker_monitor.cc |  4 ++--
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/src/mem/abstract_mem.cc b/src/mem/abstract_mem.cc
index 084ee26ad..b05a99db5 100644
--- a/src/mem/abstract_mem.cc
+++ b/src/mem/abstract_mem.cc
@@ -337,7 +337,7 @@ AbstractMemory::access(PacketPtr pkt)
     if (pkt->cmd == MemCmd::SwapReq) {
         if (pkt->isAtomicOp()) {
             if (pmemAddr) {
-                memcpy(pkt->getPtr<uint8_t>(), hostAddr, pkt->getSize());
+                pkt->setData(hostAddr);
                 (*(pkt->getAtomicOp()))(hostAddr);
             }
         } else {
@@ -345,15 +345,14 @@ AbstractMemory::access(PacketPtr pkt)
             uint64_t condition_val64;
             uint32_t condition_val32;
 
-            if (!pmemAddr)
-                panic("Swap only works if there is real memory (i.e. null=False)");
+            panic_if(!pmemAddr, "Swap only works if there is real memory " \
+                     "(i.e. null=False)");
 
             bool overwrite_mem = true;
             // keep a copy of our possible write value, and copy what is at the
             // memory address into the packet
-            std::memcpy(&overwrite_val[0], pkt->getConstPtr<uint8_t>(),
-                        pkt->getSize());
-            std::memcpy(pkt->getPtr<uint8_t>(), hostAddr, pkt->getSize());
+            pkt->writeData(&overwrite_val[0]);
+            pkt->setData(hostAddr);
 
             if (pkt->req->isCondSwap()) {
                 if (pkt->getSize() == sizeof(uint64_t)) {
@@ -383,8 +382,9 @@ AbstractMemory::access(PacketPtr pkt)
             // to do the LL/SC tracking here
             trackLoadLocked(pkt);
         }
-        if (pmemAddr)
-            memcpy(pkt->getPtr<uint8_t>(), hostAddr, pkt->getSize());
+        if (pmemAddr) {
+            pkt->setData(hostAddr);
+        }
         TRACE_PACKET(pkt->req->isInstFetch() ? "IFetch" : "Read");
         numReads[pkt->req->masterId()]++;
         bytesRead[pkt->req->masterId()] += pkt->getSize();
@@ -399,7 +399,7 @@ AbstractMemory::access(PacketPtr pkt)
     } else if (pkt->isWrite()) {
         if (writeOK(pkt)) {
             if (pmemAddr) {
-                memcpy(hostAddr, pkt->getConstPtr<uint8_t>(), pkt->getSize());
+                pkt->writeData(hostAddr);
                 DPRINTF(MemoryAccess, "%s wrote %i bytes to address %x\n",
                         __func__, pkt->getSize(), pkt->getAddr());
             }
@@ -426,13 +426,15 @@ AbstractMemory::functionalAccess(PacketPtr pkt)
     uint8_t *hostAddr = pmemAddr + pkt->getAddr() - range.start();
 
     if (pkt->isRead()) {
-        if (pmemAddr)
-            memcpy(pkt->getPtr<uint8_t>(), hostAddr, pkt->getSize());
+        if (pmemAddr) {
+            pkt->setData(hostAddr);
+        }
         TRACE_PACKET("Read");
         pkt->makeResponse();
     } else if (pkt->isWrite()) {
-        if (pmemAddr)
-            memcpy(hostAddr, pkt->getConstPtr<uint8_t>(), pkt->getSize());
+        if (pmemAddr) {
+            pkt->writeData(hostAddr);
+        }
         TRACE_PACKET("Write");
         pkt->makeResponse();
     } else if (pkt->isPrint()) {
diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc
index 6f3914ccd..a7c210697 100644
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -861,10 +861,9 @@ BaseCache::satisfyRequest(PacketPtr pkt, CacheBlk *blk, bool, bool)
         if (pkt->isAtomicOp()) {
             // extract data from cache and save it into the data field in
             // the packet as a return value from this atomic op
-
             int offset = tags->extractBlkOffset(pkt->getAddr());
             uint8_t *blk_data = blk->data + offset;
-            std::memcpy(pkt->getPtr<uint8_t>(), blk_data, pkt->getSize());
+            pkt->setData(blk_data);
 
             // execute AMO operation
             (*(pkt->getAtomicOp()))(blk_data);
diff --git a/src/mem/mem_checker_monitor.cc b/src/mem/mem_checker_monitor.cc
index c58ac54fa..ee7eb3fcc 100644
--- a/src/mem/mem_checker_monitor.cc
+++ b/src/mem/mem_checker_monitor.cc
@@ -164,7 +164,7 @@ MemCheckerMonitor::recvTimingReq(PacketPtr pkt)
         // write. For reads, we have no data yet, so it doesn't make sense to
         // allocate.
         pkt_data.reset(new uint8_t[size]);
-        memcpy(pkt_data.get(), pkt->getConstPtr<uint8_t*>(), size);
+        pkt->writeData(pkt_data.get());
     }
 
     // If a cache miss is served by a cache, a monitor near the memory
@@ -253,7 +253,7 @@ MemCheckerMonitor::recvTimingResp(PacketPtr pkt)
         // a read. For writes, we have already given the MemChecker the data on
         // the request, so it doesn't make sense to allocate on write.
         pkt_data.reset(new uint8_t[size]);
-        memcpy(pkt_data.get(), pkt->getConstPtr<uint8_t*>(), size);
+        pkt->writeData(pkt_data.get());
     }
 
     if (is_read || is_write) {
-- 
2.30.2