From: Nikos Nikoleris <nikos.nikoleris@arm.com>
Date: Thu, 14 Dec 2017 17:36:09 +0000 (+0000)
Subject: mem-cache: Cleaned blocks should be marked as not writable
X-Git-Tag: v19.0.0.0~2319
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=a1fc8b7ecdbadc3e9116594ed0803d2ed2612670;p=gem5.git

mem-cache: Cleaned blocks should be marked as not writable

A writeclean packet writes a dirty block to the memory below and
therefore sets the dirty flag for the block when the memory below is a
cache. If the block was also marked as writable it can satisfy future
write requests without further requests/snoops. This can lead to
multiple copies of the same block marked as dirty which is not
allowed. This changeset clears the writable flag from the cleaned
block to prevent the cache from satisfying future write requests
without sending a downstream request.

Change-Id: I14d3c62fd33f81b1a8ba62374c8565ccab00a6fe
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/7821
Maintainer: Jason Lowe-Power <jason@lowepower.com>
Reviewed-by: Jason Lowe-Power <jason@lowepower.com>
---

diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index 421fa5b72..1821f1873 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2017 ARM Limited
+ * Copyright (c) 2010-2018 ARM Limited
  * All rights reserved.
  *
  * The license below extends only to copyright in the software and shall
@@ -405,6 +405,7 @@ Cache::access(PacketPtr pkt, CacheBlk *&blk, Cycles &lat,
         // only mark the block dirty if we got a writeback command,
         // and leave it as is for a clean writeback
         if (pkt->cmd == MemCmd::WritebackDirty) {
+            assert(!blk->isDirty());
             blk->status |= BlkDirty;
         }
         // if the packet does not have sharers, it is passing
@@ -467,6 +468,7 @@ Cache::access(PacketPtr pkt, CacheBlk *&blk, Cycles &lat,
         // write clean operation and the block is already in this
         // cache, we need to update the data and the block flags
         assert(blk);
+        assert(!blk->isDirty());
         if (!pkt->writeThrough()) {
             blk->status |= BlkDirty;
         }
@@ -1697,21 +1699,32 @@ Cache::writecleanBlk(CacheBlk *blk, Request::Flags dest, PacketId id)
         req->setFlags(Request::SECURE);
     }
     req->taskId(blk->task_id);
-    blk->task_id = ContextSwitchTaskId::Unknown;
+
     PacketPtr pkt = new Packet(req, MemCmd::WriteClean, blkSize, id);
+
+    if (dest) {
+        req->setFlags(dest);
+        pkt->setWriteThrough();
+    }
+
     DPRINTF(Cache, "Create %s writable: %d, dirty: %d\n", pkt->print(),
             blk->isWritable(), blk->isDirty());
+
+    if (blk->isWritable()) {
+        // not asserting shared means we pass the block in modified
+        // state, mark our own block non-writeable
+        blk->status &= ~BlkWritable;
+    } else {
+        // we are in the Owned state, tell the receiver
+        pkt->setHasSharers();
+    }
+
     // make sure the block is not marked dirty
     blk->status &= ~BlkDirty;
+
     pkt->allocate();
-    // We inform the cache below that the block has sharers in the
-    // system as we retain our copy.
-    pkt->setHasSharers();
-    if (dest) {
-        req->setFlags(dest);
-        pkt->setWriteThrough();
-    }
     std::memcpy(pkt->getPtr<uint8_t>(), blk->data, blkSize);
+
     return pkt;
 }