mem-cache: Add setters to validate and secure block

In order to allow polymorphism of the block these two
functions have been added, and all direct status
assignments to these bits have been substituted.

We also assert that the block has been invalidated
before insertion. Then the block is validated in
the insertion.

Change-Id: Ie7be42408721ad4c2c9dc880f82a62cb594f8668
Signed-off-by: Daniel R. Carvalho <odanrc@yahoo.com.br>
Reviewed-on: https://gem5-review.googlesource.com/c/14362
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Maintainer: Nikos Nikoleris <nikos.nikoleris@arm.com>

diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc
index 497f75c8848afeb9a45c0a910fc07e0dc16f34a8..244d7ce4e0e1d5c8eab4d92626fdc9e3f4261bf8 100644 (file)
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -1004,7 +1004,7 @@ BaseCache::access(PacketPtr pkt, CacheBlk *&blk, Cycles &lat,
                 return false;
             }
 
-            blk->status |= (BlkValid | BlkReadable);
+            blk->status |= BlkReadable;
         }
         // only mark the block dirty if we got a writeback command,
         // and leave it as is for a clean writeback
@@ -1062,7 +1062,7 @@ BaseCache::access(PacketPtr pkt, CacheBlk *&blk, Cycles &lat,
                     return false;
                 }
 
-                blk->status |= (BlkValid | BlkReadable);
+                blk->status |= BlkReadable;
             }
         }
 
@@ -1149,26 +1149,23 @@ BaseCache::handleFill(PacketPtr pkt, CacheBlk *blk, PacketList &writebacks,
             // No replaceable block or a mostly exclusive
             // cache... just use temporary storage to complete the
             // current request and then get rid of it
-            assert(!tempBlock->isValid());
             blk = tempBlock;
             tempBlock->insert(addr, is_secure);
             DPRINTF(Cache, "using temp block for %#llx (%s)\n", addr,
                     is_secure ? "s" : "ns");
         }
-
-        // we should never be overwriting a valid block
-        assert(!blk->isValid());
     } else {
         // existing block... probably an upgrade
-        assert(regenerateBlkAddr(blk) == addr);
-        assert(blk->isSecure() == is_secure);
-        // either we're getting new data or the block should already be valid
-        assert(pkt->hasData() || blk->isValid());
         // don't clear block status... if block is already dirty we
         // don't want to lose that
     }
 
-    blk->status |= BlkValid | BlkReadable;
+    // Block is guaranteed to be valid at this point
+    assert(blk->isValid());
+    assert(blk->isSecure() == is_secure);
+    assert(regenerateBlkAddr(blk) == addr);
+
+    blk->status |= BlkReadable;
 
     // sanity check for whole-line writes, which should always be
     // marked as writable as part of the fill, and then later marked

diff --git a/src/mem/cache/cache_blk.cc b/src/mem/cache/cache_blk.cc
index a77fc630af385c3df73c2fb871f1a3ea877a6751..c4730caebc718d29714e9b76729dbf559743c474 100644 (file)
--- a/src/mem/cache/cache_blk.cc
+++ b/src/mem/cache/cache_blk.cc
@@ -46,6 +46,9 @@ void
 CacheBlk::insert(const Addr tag, const bool is_secure,
                  const int src_master_ID, const uint32_t task_ID)
 {
+    // Make sure that the block has been properly invalidated
+    assert(status == 0);
+
     // Set block tag
     this->tag = tag;
 
@@ -63,10 +66,11 @@ CacheBlk::insert(const Addr tag, const bool is_secure,
 
     // Set secure state
     if (is_secure) {
-        status = BlkSecure;
-    } else {
-        status = 0;
+        setSecure();
     }
+
+    // Validate block
+    setValid();
 }
 
 void

diff --git a/src/mem/cache/cache_blk.hh b/src/mem/cache/cache_blk.hh
index 80983a6372af895a6f80267cd06298c588878a75..d7530d6561c433b11e9ecd42c3611217b1f35c01 100644 (file)
--- a/src/mem/cache/cache_blk.hh
+++ b/src/mem/cache/cache_blk.hh
@@ -243,11 +243,28 @@ class CacheBlk : public ReplaceableEntry
         return (status & BlkSecure) != 0;
     }
 
+    /**
+     * Set valid bit.
+     */
+    virtual void setValid()
+    {
+        assert(!isValid());
+        status |= BlkValid;
+    }
+
+    /**
+     * Set secure bit.
+     */
+    virtual void setSecure()
+    {
+        status |= BlkSecure;
+    }
+
     /**
      * Set member variables when a block insertion occurs. Resets reference
      * count to 1 (the insertion counts as a reference), and touch block if
      * it hadn't been touched previously. Sets the insertion tick to the
-     * current tick. Does not make block valid.
+     * current tick. Marks the block valid.
      *
      * @param tag Block address tag.
      * @param is_secure Whether the block is in secure space or not.
@@ -425,15 +442,19 @@ class TempCacheBlk final : public CacheBlk
     void insert(const Addr addr, const bool is_secure,
                 const int src_master_ID=0, const uint32_t task_ID=0) override
     {
+        // Make sure that the block has been properly invalidated
+        assert(status == 0);
+
         // Set block address
         _addr = addr;
 
         // Set secure state
         if (is_secure) {
-            status = BlkSecure;
-        } else {
-            status = 0;
+            setSecure();
         }
+
+        // Validate block
+        setValid();
     }
 
     /**

diff --git a/src/mem/cache/tags/sector_blk.hh b/src/mem/cache/tags/sector_blk.hh
index 6eed04464f1b7e6b38cc000ee73b272291a8b9bb..12649fce2b7cbb94299f1ee74334099ee9973e5f 100644 (file)
--- a/src/mem/cache/tags/sector_blk.hh
+++ b/src/mem/cache/tags/sector_blk.hh
@@ -105,7 +105,7 @@ class SectorSubBlk : public CacheBlk
      * Set member variables when a block insertion occurs. Resets reference
      * count to 1 (the insertion counts as a reference), and touch block if
      * it hadn't been touched previously. Sets the insertion tick to the
-     * current tick. Does not make block valid.
+     * current tick. Marks the block valid.
      *
      * @param tag Block address tag.
      * @param is_secure Whether the block is in secure space or not.

diff --git a/src/mem/cache/tags/sector_tags.cc b/src/mem/cache/tags/sector_tags.cc
index 68440c2f2a8c01707665cd8c53341e47f4527d0c..ad374e5eeb1f3c83967b9470f9729c4c976be6c3 100644 (file)
--- a/src/mem/cache/tags/sector_tags.cc
+++ b/src/mem/cache/tags/sector_tags.cc
@@ -171,16 +171,12 @@ SectorTags::insertBlock(const Addr addr, const bool is_secure,
                         const int src_master_ID, const uint32_t task_ID,
                         CacheBlk *blk)
 {
-    // Do common block insertion functionality
-    BaseTags::insertBlock(addr, is_secure, src_master_ID, task_ID, blk);
-
     // Get block's sector
     SectorSubBlk* sub_blk = static_cast<SectorSubBlk*>(blk);
     const SectorBlk* sector_blk = sub_blk->getSectorBlock();
 
     // When a block is inserted, the tag is only a newly used tag if the
     // sector was not previously present in the cache.
-    // This assumes BaseTags::insertBlock does not set the valid bit.
     if (sector_blk->isValid()) {
         // An existing entry's replacement data is just updated
         replacementPolicy->touch(sector_blk->replacementData);
@@ -191,6 +187,9 @@ SectorTags::insertBlock(const Addr addr, const bool is_secure,
         // A new entry resets the replacement data
         replacementPolicy->reset(sector_blk->replacementData);
     }
+
+    // Do common block insertion functionality
+    BaseTags::insertBlock(addr, is_secure, src_master_ID, task_ID, blk);
 }
 
 CacheBlk*