From 603f1378c0459f58593683782eaafb69500f0693 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Tiago=20M=C3=BCck?= Date: Mon, 29 Jul 2019 13:45:31 -0500 Subject: [PATCH] mem-cache: mark block as dirty when handling SW prefetch MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This addresses the issue described in 64687ee mem-cache: Mark block as dirty after a SWPrefetchEXResp. Previous patch misses cases when the prefetch response is ReadExResp or UpgradeResp. Also, marking the block as dirty in serviceMSHRTargets instead of in handleFill covers cases when the prefetch is coalesced with other requests. Change-Id: I2b377fdd240eb0f09e720b6bb284dee6545925ce Signed-off-by: Tiago Mück Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/19688 Reviewed-by: Nikos Nikoleris Reviewed-by: Daniel Carvalho Maintainer: Nikos Nikoleris Tested-by: kokoro --- src/mem/cache/base.cc | 24 ------------------------ src/mem/cache/cache.cc | 26 ++++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc index 082650c09..0de7f2150 100644 --- a/src/mem/cache/base.cc +++ b/src/mem/cache/base.cc @@ -1409,30 +1409,6 @@ BaseCache::handleFill(PacketPtr pkt, CacheBlk *blk, PacketList &writebacks, chatty_assert(!isReadOnly, "Should never see dirty snoop response " "in read-only cache %s\n", name()); - } else if (pkt->cmd.isSWPrefetch() && pkt->needsWritable()) { - // All other copies of the block were invalidated and we - // have an exclusive copy. - - // The coherence protocol assumes that if we fetched an - // exclusive copy of the block, we have the intention to - // modify it. Therefore the MSHR for the PrefetchExReq has - // been the point of ordering and this cache has commited - // to respond to snoops for the block. - // - // In most cases this is true anyway - a PrefetchExReq - // will be followed by a WriteReq. However, if that - // doesn't happen, the block is not marked as dirty and - // the cache doesn't respond to snoops that has committed - // to do so. - // - // To avoid deadlocks in cases where there is a snoop - // between the PrefetchExReq and the expected WriteReq, we - // proactively mark the block as Dirty. - - blk->status |= BlkDirty; - - panic_if(!isReadOnly, "Prefetch exclusive requests from read-only " - "cache %s\n", name()); } } diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc index bded74643..b054cd43d 100644 --- a/src/mem/cache/cache.cc +++ b/src/mem/cache/cache.cc @@ -710,6 +710,32 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk) // Software prefetch handling for cache closest to core if (tgt_pkt->cmd.isSWPrefetch()) { + if (tgt_pkt->needsWritable()) { + // All other copies of the block were invalidated and we + // have an exclusive copy. + + // The coherence protocol assumes that if we fetched an + // exclusive copy of the block, we have the intention to + // modify it. Therefore the MSHR for the PrefetchExReq has + // been the point of ordering and this cache has commited + // to respond to snoops for the block. + // + // In most cases this is true anyway - a PrefetchExReq + // will be followed by a WriteReq. However, if that + // doesn't happen, the block is not marked as dirty and + // the cache doesn't respond to snoops that has committed + // to do so. + // + // To avoid deadlocks in cases where there is a snoop + // between the PrefetchExReq and the expected WriteReq, we + // proactively mark the block as Dirty. + assert(blk); + blk->status |= BlkDirty; + + panic_if(isReadOnly, "Prefetch exclusive requests from " + "read-only cache %s\n", name()); + } + // a software prefetch would have already been ack'd // immediately with dummy data so the core would be able to // retire it. This request completes right here, so we -- 2.30.2