Rahul Thakur has uploaded this change for review. (
https://gem5-review.googlesource.com/8681
Change subject: mem: Fix uncacheable write snoop flow
......................................................................
mem: Fix uncacheable write snoop flow
Earlier patches fixed handling of uncacheable IO WriteReq snoops hit in
caches by allowing in-place modification of cache block.
This patch proposes a few fixes in that:
1. Update upstream snoop packet with current snoop pkt data
2. Update mshr and cache block for WriteReq and additionally
WriteLineReq which was previously left out.
Change-Id: Ia6fbdf53e92427f8e6ec74028b73769accc27562
---
M src/mem/cache/cache.cc
M src/mem/cache/mshr.cc
M src/mem/packet.hh
3 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index 1821f18..9cdc490 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -2069,6 +2069,15 @@
// there is a snoop hit in upper levels
Packet snoopPkt(pkt, true, true);
snoopPkt.setExpressSnoop();
+
+ // Set data for the upstream snoop pkt from the current pkt
+ // which is write transaction and carries modified data
+ if ((pkt->cmd == MemCmd::WriteReq ||
+ pkt->cmd == MemCmd::WriteLineReq) && !alreadyResponded) {
+ assert(pkt->req->isUncacheable());
+ snoopPkt.setData(pkt->getConstPtr<uint8_t>());
+ }
+
// the snoop packet does not need to wait any additional
// time
snoopPkt.headerDelay = snoopPkt.payloadDelay = 0;
@@ -2211,6 +2220,18 @@
// we already called setHasSharers above
}
+ // For uncacheable write, update the dirty data
+ if (pkt->cmd == MemCmd::WriteReq || pkt->cmd ==
MemCmd::WriteLineReq) {
+ assert(pkt->req->isUncacheable());
+ // write the partial line to the right part of the block
+ pkt->writeDataToBlock(blk->data, blkSize);
+
+ // the block is not going to end up with a cache
+ // allocation, but set the flag to indicate that there are
+ // sharers, also to avoid the panic below
+ pkt->setHasSharers();
+ }
+
// if we are returning a writable and dirty (Modified) line,
// we should be invalidating the line
panic_if(!invalidate && !pkt->hasSharers(),
@@ -2356,6 +2377,19 @@
pkt->setResponderHadWritable();
}
+ // partial line uncacheable write, update the dirty data
+ if (pkt->cmd == MemCmd::WriteReq ||
+ pkt->cmd == MemCmd::WriteLineReq) {
+ assert(pkt->req->isUncacheable());
+ // write the partial line to the right part of the
+ // writeback packet
+ pkt->writeDataToBlock(wb_pkt->getPtr<uint8_t>(), blkSize);
+
+ // not strictly necessary, but set the flag indicating
+ // that the packet has sharers
+ pkt->setHasSharers();
+ }
+
doTimingSupplyResponse(pkt, wb_pkt->getConstPtr<uint8_t>(),
false, false);
}
diff --git a/src/mem/cache/mshr.cc b/src/mem/cache/mshr.cc
index 493b7f0..5af4f6a 100644
--- a/src/mem/cache/mshr.cc
+++ b/src/mem/cache/mshr.cc
@@ -425,8 +425,19 @@
// the latter case the cache is responsible for deleting both
// the packet and the request as part of handling the deferred
// snoop.
- PacketPtr cp_pkt = will_respond ? new Packet(pkt, true, true) :
- new Packet(new Request(*pkt->req), pkt->cmd, blkSize, pkt->id);
+ PacketPtr cp_pkt = nullptr;
+ if (will_respond) {
+ cp_pkt = new Packet(pkt, true, true);
+ // Update the data if request is write carrying updated data
+ if (pkt->cmd == MemCmd::WriteReq ||
+ pkt->cmd == MemCmd::WriteLineReq) {
+ assert(pkt->req->isUncacheable());
+ cp_pkt->allocate();
+ cp_pkt->setData(pkt->getConstPtr<uint8_t>());
+ }
+ } else {
+ cp_pkt = new Packet(new Request(*pkt->req), pkt->cmd, blkSize);
+ }
if (will_respond) {
// we are the ordering point, and will consequently
diff --git a/src/mem/packet.hh b/src/mem/packet.hh
index 66625b3..1f481db 100644
--- a/src/mem/packet.hh
+++ b/src/mem/packet.hh
@@ -530,7 +530,8 @@
// look at the hasSharers flag (if not set, the response is to
// be considered writable)
assert(isRequest());
- return cmd.needsWritable();
+ // ignore the needsWritable flag if the packet is uncacheable
+ return cmd.needsWritable() && !req->isUncacheable();
}
bool needsResponse() const { return cmd.needsResponse(); }
bool isInvalidate() const { return cmd.isInvalidate(); }
--
To view, visit https://gem5-review.googlesource.com/8681
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: Ia6fbdf53e92427f8e6ec74028b73769accc27562
Gerrit-Change-Number: 8681
Gerrit-PatchSet: 1
Gerrit-Owner: Rahul Thakur <rjtha...@google.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev
