changeset f2f1dbfd505e in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=f2f1dbfd505e
description:
mem: have WriteInvalidate obsolete MSHRs
Since WriteInvalidate directly writes into the cache, it can
create tricky timing interleavings with reads and writes to the
same cache line that haven't yet completed. This patch ensures
that these requests, when completed, don't overwrite the newer
data from the WriteInvalidate.
diffstat:
src/mem/cache/cache_impl.hh | 35 +++++++++++++++++++++++++++++++----
src/mem/cache/mshr.cc | 5 +++--
src/mem/cache/mshr.hh | 11 +++++++++++
3 files changed, 45 insertions(+), 6 deletions(-)
diffs (116 lines):
diff -r e278fa3086b5 -r f2f1dbfd505e src/mem/cache/cache_impl.hh
--- a/src/mem/cache/cache_impl.hh Tue Sep 02 11:26:32 2014 +0100
+++ b/src/mem/cache/cache_impl.hh Wed Oct 29 23:18:24 2014 -0500
@@ -636,7 +636,20 @@
pkt = pf;
}
- if (mshr) {
+ if (pkt && (pkt->cmd == MemCmd::WriteInvalidateReq)) {
+ // WriteInvalidates cannot coalesce with other requests, so
+ // we cannot use an existing MSHR. If one exists, we mark it
+ // as 'obsolete' so they don't modify the cache.
+ if (mshr) {
+ // Everything up to this point is obsolete, meaning
+ // they should not modify the cache.
+ DPRINTF(Cache, "%s: marking MSHR obsolete in %s of %x\n",
+ __func__, pkt->cmdString(), pkt->getAddr());
+
+ mshr->markObsolete();
+ }
+ allocateMissBuffer(pkt, time, true);
+ } else if (mshr) {
/// MSHR hit
/// @note writebacks will be checked in getNextMSHR()
/// for any conflicting requests to the same block
@@ -1077,7 +1090,10 @@
bool is_fill = !mshr->isForward &&
(pkt->isRead() || pkt->cmd == MemCmd::UpgradeResp);
- if (is_fill && !is_error) {
+ if (mshr->isObsolete()) {
+ DPRINTF(Cache, "%s: skipping cache fills; data for %s of %x "
+ "is obsolete\n", __func__, pkt->cmdString(), pkt->getAddr());
+ } else if (is_fill && !is_error) {
DPRINTF(Cache, "Block for addr %x being updated in Cache\n",
pkt->getAddr());
@@ -1113,8 +1129,19 @@
}
if (is_fill) {
- satisfyCpuSideRequest(target->pkt, blk,
- true, mshr->hasPostDowngrade());
+ // Presently the only situation leading to 'obsolete'
+ // data is when a WriteInvalidate blows away an already
+ // pending/in-progress read. We don't want to overwrite
+ // cache data in that case.
+ if (mshr->isObsolete()) {
+ DPRINTF(Cache, "%s: skipping satisfyCpuSideRequest; "
+ "data for %s of %x is obsolete\n",
+ __func__, target->pkt->cmdString(),
+ target->pkt->getAddr());
+ } else {
+ satisfyCpuSideRequest(target->pkt, blk,
+ true, mshr->hasPostDowngrade());
+ }
// How many bytes past the first request is this one
int transfer_offset =
target->pkt->getOffset(blkSize) - initial_offset;
diff -r e278fa3086b5 -r f2f1dbfd505e src/mem/cache/mshr.cc
--- a/src/mem/cache/mshr.cc Tue Sep 02 11:26:32 2014 +0100
+++ b/src/mem/cache/mshr.cc Wed Oct 29 23:18:24 2014 -0500
@@ -62,8 +62,8 @@
using namespace std;
MSHR::MSHR() : readyTime(0), _isUncacheable(false), downstreamPending(false),
- pendingDirty(false), postInvalidate(false),
- postDowngrade(false), queue(NULL), order(0), addr(0), size(0),
+ pendingDirty(false), postInvalidate(false),
postDowngrade(false),
+ _isObsolete(false), queue(NULL), order(0), addr(0), size(0),
isSecure(false), inService(false), isForward(false),
threadNum(InvalidThreadID), data(NULL)
{
@@ -214,6 +214,7 @@
_isUncacheable = target->req->isUncacheable();
inService = false;
downstreamPending = false;
+ _isObsolete = false;
threadNum = 0;
assert(targets.isReset());
// Don't know of a case where we would allocate a new MSHR for a
diff -r e278fa3086b5 -r f2f1dbfd505e src/mem/cache/mshr.hh
--- a/src/mem/cache/mshr.hh Tue Sep 02 11:26:32 2014 +0100
+++ b/src/mem/cache/mshr.hh Wed Oct 29 23:18:24 2014 -0500
@@ -89,6 +89,9 @@
/** Did we snoop a read while waiting for data? */
bool postDowngrade;
+ /** Did we get WriteInvalidate'd (and therefore obsoleted)? */
+ bool _isObsolete;
+
public:
class Target {
@@ -214,6 +217,8 @@
bool isUncacheable() const { return _isUncacheable; }
+ bool isObsolete() const { return _isObsolete; }
+
/**
* Allocate a miss to this MSHR.
* @param cmd The requesting command.
@@ -289,6 +294,12 @@
bool checkFunctional(PacketPtr pkt);
+ /** Mark this MSHR as tracking a transaction with obsoleted data. It still
+ * needs to complete its lifecycle, but should not modify the cache. */
+ void markObsolete() {
+ _isObsolete = true;
+ }
+
/**
* Prints the contents of this MSHR for debugging.
*/
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
