From: Nikos Nikoleris Date: Thu, 7 Mar 2019 11:48:01 +0000 (+0000) Subject: mem-cache: Mark block as dirty after a SWPrefetchEXResp X-Git-Tag: v19.0.0.0~887 X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=64687eee01c63d72cc4d38bfb2ec3b529d81c38f;p=gem5.git mem-cache: Mark block as dirty after a SWPrefetchEXResp This is a workaround for a bug introduced from the change: 59e3585a8 arch-arm: We add PRFM PST instruction for arm which can cause deadlocks in the memory system. The design of the classic memory system in gem5 makes the folloing two assumptions: * A cache that fetches a block with an intention to modify it, becomes the point of ordering and therefore commits to respond to any snoop requests [1]. * A cache that fetches an exclusive copy of the block, does so with the intention to modify it [2]. Immediately after it receives the block, it will write to it and mark it as dirty. As the point of ordering, it responds to any outstanding snoops. The current implementation of prefetch exclusive request breaks the second assumption. A cache can fetch an exclusive block without an immediate intention to modify it. If the block is not modified, it will not be marked as dirty. However, the cache has committed to respond to outstanding snoops, and if the block is clean it won't. This can result in deadlocks where a snoop gets stuck waiting for responses. One solution (implemented by this patch) is to unconditionally mark the block dirty when filling due to a prefetch exclusive request. This makes the PrefetchExReq behave like a WriteReq. However, as it may mark as dirty a clean block, it creates the requirement for an uncessary WritebackDirty in the future. In practice, this shouldn't be a big problem unless the application is unnecessarily using prefetch exclusive instructions. Other solutions, would require deeper changes to the design of the memory system to handle this properly. [1]: When a cache commits to respond, it "informs" the xbar/PoC (point of coherence) and the other caches of its intention to respond. As a result the request will not be send to the main memory. [2]: In fact the assumption is that in the needsWritable MSHR there is at least one WriteReq before any snoops from other caches. Change-Id: I378d3c0dadf25fc52e430b67102347b44d2f18ea Signed-off-by: Nikos Nikoleris Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/17729 Reviewed-by: Daniel Carvalho Tested-by: kokoro --- diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc index f087618c7..f31fbaf03 100644 --- a/src/mem/cache/base.cc +++ b/src/mem/cache/base.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2013, 2018 ARM Limited + * Copyright (c) 2012-2013, 2018-2019 ARM Limited * All rights reserved. * * The license below extends only to copyright in the software and shall @@ -1263,6 +1263,31 @@ 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()); } }