From 96e8f2ed3c3e1cfa32677dcc32f2d0c263dd6be6 Mon Sep 17 00:00:00 2001 From: Nikos Nikoleris Date: Thu, 7 Nov 2019 10:50:36 +0000 Subject: [PATCH] mem-cache: Ensure that responses get data from the right source This CL makes sure that we use the right source for data for responses after a response from the cache below. Change-Id: I7329f3e6bcb7ce2054e912eb9dea48c9d169d45a Signed-off-by: Nikos Nikoleris Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23667 Reviewed-by: Daniel Carvalho Tested-by: kokoro --- src/mem/cache/cache.cc | 45 +++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc index e7dd5efc9..a4f2baeb6 100644 --- a/src/mem/cache/cache.cc +++ b/src/mem/cache/cache.cc @@ -756,7 +756,14 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk) assert(blk->isWritable()); } - if (blk && blk->isValid() && !mshr->isForward) { + // Here we decide whether we will satisfy the target using + // data from the block or from the response. We use the + // block data to satisfy the request when the block is + // present and valid and in addition the response in not + // forwarding data to the cache above (we didn't fill + // either); otherwise we use the packet data. + if (blk && blk->isValid() && + (!mshr->isForward || !pkt->hasData())) { satisfyRequest(tgt_pkt, blk, true, mshr->hasPostDowngrade()); // How many bytes past the first request is this one @@ -791,15 +798,15 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk) pkt->payloadDelay; tgt_pkt->req->setExtraData(0); } else { - // We are about to send a response to a cache above - // that asked for an invalidation; we need to - // invalidate our copy immediately as the most - // up-to-date copy of the block will now be in the - // cache above. It will also prevent this cache from - // responding (if the block was previously dirty) to - // snoops as they should snoop the caches above where - // they will get the response from. if (is_invalidate && blk && blk->isValid()) { + // We are about to send a response to a cache above + // that asked for an invalidation; we need to + // invalidate our copy immediately as the most + // up-to-date copy of the block will now be in the + // cache above. It will also prevent this cache from + // responding (if the block was previously dirty) to + // snoops as they should snoop the caches above where + // they will get the response from. invalidateBlock(blk); } // not a cache fill, just forwarding response @@ -807,12 +814,22 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk) // from lower level cahces/memory to the core. completion_time += clockEdge(responseLatency) + pkt->payloadDelay; - if (pkt->isRead() && !is_error) { - // sanity check - assert(pkt->matchAddr(tgt_pkt)); - assert(pkt->getSize() >= tgt_pkt->getSize()); + if (!is_error) { + if (pkt->isRead()) { + // sanity check + assert(pkt->matchAddr(tgt_pkt)); + assert(pkt->getSize() >= tgt_pkt->getSize()); - tgt_pkt->setData(pkt->getConstPtr()); + tgt_pkt->setData(pkt->getConstPtr()); + } else { + // MSHR targets can read data either from the + // block or the response pkt. If we can't get data + // from the block (i.e., invalid or has old data) + // or the response (did not bring in any data) + // then make sure that the target didn't expect + // any. + assert(!tgt_pkt->hasRespData()); + } } // this response did not allocate here and therefore -- 2.30.2