From adde4c91f99fa99abbdd23cf97c45de020429d2d Mon Sep 17 00:00:00 2001 From: "Daniel R. Carvalho" Date: Mon, 15 Oct 2018 11:48:32 +0200 Subject: 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 Reviewed-on: https://gem5-review.googlesource.com/c/13695 Reviewed-by: Nikos Nikoleris Maintainer: Nikos Nikoleris --- 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(), 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(), - pkt->getSize()); - std::memcpy(pkt->getPtr(), 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(), 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(), 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(), hostAddr, pkt->getSize()); + if (pmemAddr) { + pkt->setData(hostAddr); + } TRACE_PACKET("Read"); pkt->makeResponse(); } else if (pkt->isWrite()) { - if (pmemAddr) - memcpy(hostAddr, pkt->getConstPtr(), 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(), 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(), 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(), size); + pkt->writeData(pkt_data.get()); } if (is_read || is_write) { -- cgit v1.2.3