Hello Andreas Sandberg,
I'd like you to do a code review. Please visit
https://gem5-review.googlesource.com/9241
to review the following change.
Change subject: mem-cache: Fix snoop handling for uncacheable write requests
......................................................................
mem-cache: Fix snoop handling for uncacheable write requests
The cache with:
* a dirty copy of the block, or
* an in-service MSHR for the block, or
* a dirty writeback for the block
is typically the ordering point and the cache that responds to
subsequent requests.
Uncacheable write requests bypass the ordering point and software
needs to enforce coherence between cacheable and uncacheable requests.
Change-Id: I73ddd2dc043de81eb67a769e8311a855bea12c9e
Reviewed-by: Andreas Sandberg <[email protected]>
---
M src/mem/cache/cache.cc
M src/mem/cache/mshr.cc
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index 85c9677..b25c786 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -2156,10 +2156,13 @@
// we respond in atomic mode), so just figure out what to do now
// and then do it later. We respond to all snoops that need
// responses provided we have the block in dirty state. The
- // invalidation itself is taken care of below. We don't respond to
- // cache maintenance operations as this is done by the destination
- // xbar.
- respond = blk->isDirty() && pkt->needsResponse();
+ // invalidation itself is taken care of below. We don't respond to:
+ // * cache maintenance operations as this is done by the
+ // destination xbar
+ // * uncacheable write requests, we don't need to maintain
+ // coherence between cacheable and uncacheable writes
+ respond = blk->isDirty() && pkt->needsResponse() &&
+ !(pkt->isWrite() && pkt->req->isUncacheable());
chatty_assert(!(isReadOnly && blk->isDirty()), "Should never have "
"a dirty block in a read-only cache %s\n", name());
@@ -2340,7 +2343,8 @@
// state to determine if it is dirty and writable, we use the
// command and fields of the writeback packet
bool respond = wb_pkt->cmd == MemCmd::WritebackDirty &&
- pkt->needsResponse();
+ pkt->needsResponse() &&
+ !(pkt->isWrite() && pkt->req->isUncacheable());
bool have_writable = !wb_pkt->hasSharers();
bool invalidate = pkt->isInvalidate();
diff --git a/src/mem/cache/mshr.cc b/src/mem/cache/mshr.cc
index 493b7f0..b701035 100644
--- a/src/mem/cache/mshr.cc
+++ b/src/mem/cache/mshr.cc
@@ -354,6 +354,12 @@
{
DPRINTF(Cache, "%s for %s\n", __func__, pkt->print());
+ if (pkt->isWrite() && pkt->req->isUncacheable()) {
+ // we do not need to maintain coherence between cacheable and
+ // uncacheable write requests, software has to deal with it
+ return true;
+ }
+
// when we snoop packets the needsWritable and isInvalidate flags
// should always be the same, however, this assumes that we never
// snoop writes as they are currently not marked as invalidations
--
To view, visit https://gem5-review.googlesource.com/9241
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: I73ddd2dc043de81eb67a769e8311a855bea12c9e
Gerrit-Change-Number: 9241
Gerrit-PatchSet: 1
Gerrit-Owner: Nikos Nikoleris <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev