mem: Align all MSHR entries to block boundaries

This patch aligns all MSHR queue entries to block boundaries to
simplify checks for matches. Previously there were corner cases that
could lead to existing entries not being identified as matches.

There are, rather alarmingly, a few regressions that change with this
patch.

6 files changed, 81 insertions, 66 deletions

diff --git a/src/mem/cache/base.hh b/src/mem/cache/base.hh
index cd2f55246..16160dba3 100644
--- a/src/mem/cache/base.hh
+++ b/src/mem/cache/base.hh
@@ -217,6 +217,11 @@ class BaseCache : public MemObject
     MSHR *allocateBufferInternal(MSHRQueue *mq, Addr addr, int size,
                                  PacketPtr pkt, Tick time, bool requestBus)
     {
+        // check that the address is block aligned since we rely on
+        // this in a number of places when checking for matches and
+        // overlap
+        assert(addr == blockAlign(addr));
+
         MSHR *mshr = mq->allocate(addr, size, pkt, time, order++);
 
         if (mq->isFull()) {
@@ -506,7 +511,7 @@ class BaseCache : public MemObject
     {
         assert(pkt->isWrite() && !pkt->isRead());
         return allocateBufferInternal(&writeBuffer,
-                                      pkt->getAddr(), pkt->getSize(),
+                                      blockAlign(pkt->getAddr()), blkSize,
                                       pkt, time, requestBus);
     }
 
@@ -515,7 +520,7 @@ class BaseCache : public MemObject
         assert(pkt->req->isUncacheable());
         assert(pkt->isRead());
         return allocateBufferInternal(&mshrQueue,
-                                      pkt->getAddr(), pkt->getSize(),
+                                      blockAlign(pkt->getAddr()), blkSize,
                                       pkt, time, requestBus);
     }
 
diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index 538f1632b..93ef34bb2 100644
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -836,6 +836,9 @@ Cache<TagStore>::getBusPacket(PacketPtr cpu_pkt, BlkType *blk,
     }
     PacketPtr pkt = new Packet(cpu_pkt->req, cmd, blkSize);
 
+    // the packet should be block aligned
+    assert(pkt->getAddr() == blockAlign(pkt->getAddr()));
+
     pkt->allocate();
     DPRINTF(Cache, "%s created %s addr %#llx size %d\n",
             __func__, pkt->cmdString(), pkt->getAddr(), pkt->getSize());
@@ -1209,6 +1212,10 @@ Cache<TagStore>::recvTimingResp(PacketPtr pkt)
                 completion_time += clockEdge(responseLatency) +
                     pkt->payloadDelay;
                 if (pkt->isRead() && !is_error) {
+                    // sanity check
+                    assert(pkt->getAddr() == tgt_pkt->getAddr());
+                    assert(pkt->getSize() >= tgt_pkt->getSize());
+
                     tgt_pkt->setData(pkt->getConstPtr<uint8_t>());
                 }
             }
@@ -1543,7 +1550,10 @@ Cache<TagStore>::handleFill(PacketPtr pkt, BlkType *blk,
     // if we got new data, copy it in (checking for a read response
     // and a response that has data is the same in the end)
     if (pkt->isRead()) {
+        // sanity checks
         assert(pkt->hasData());
+        assert(pkt->getSize() == blkSize);
+
         std::memcpy(blk->data, pkt->getConstPtr<uint8_t>(), blkSize);
     }
     // We pay for fillLatency here.
@@ -1899,7 +1909,7 @@ Cache<TagStore>::getNextMSHR()
          !miss_mshr)) {
         // need to search MSHR queue for conflicting earlier miss.
         MSHR *conflict_mshr =
-            mshrQueue.findPending(write_mshr->addr, write_mshr->size,
+            mshrQueue.findPending(write_mshr->blkAddr,
                                   write_mshr->isSecure);
 
         if (conflict_mshr && conflict_mshr->order < write_mshr->order) {
@@ -1914,7 +1924,7 @@ Cache<TagStore>::getNextMSHR()
     } else if (miss_mshr) {
         // need to check for conflicting earlier writeback
         MSHR *conflict_mshr =
-            writeBuffer.findPending(miss_mshr->addr, miss_mshr->size,
+            writeBuffer.findPending(miss_mshr->blkAddr,
                                     miss_mshr->isSecure);
         if (conflict_mshr) {
             // not sure why we don't check order here... it was in the
@@ -1985,10 +1995,10 @@ Cache<TagStore>::getTimingPacket()
 
     if (mshr->isForwardNoResponse()) {
         // no response expected, just forward packet as it is
-        assert(tags->findBlock(mshr->addr, mshr->isSecure) == NULL);
+        assert(tags->findBlock(mshr->blkAddr, mshr->isSecure) == NULL);
         pkt = tgt_pkt;
     } else {
-        BlkType *blk = tags->findBlock(mshr->addr, mshr->isSecure);
+        BlkType *blk = tags->findBlock(mshr->blkAddr, mshr->isSecure);
 
         if (tgt_pkt->cmd == MemCmd::HardPFReq) {
             // We need to check the caches above us to verify that
@@ -2025,7 +2035,8 @@ Cache<TagStore>::getTimingPacket()
 
             if (snoop_pkt.isBlockCached() || blk != NULL) {
                 DPRINTF(Cache, "Block present, prefetch squashed by cache.  "
-                               "Deallocating mshr target %#x.\n", mshr->addr);
+                               "Deallocating mshr target %#x.\n",
+                        mshr->blkAddr);
 
                 // Deallocate the mshr target
                 if (mshr->queue->forceDeallocateTarget(mshr)) {
diff --git a/src/mem/cache/mshr.cc b/src/mem/cache/mshr.cc
index 530ea7797..8eb5e4752 100644
--- a/src/mem/cache/mshr.cc
+++ b/src/mem/cache/mshr.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2013 ARM Limited
+ * Copyright (c) 2012-2013, 2015 ARM Limited
  * All rights reserved.
  *
  * The license below extends only to copyright in the software and shall
@@ -64,8 +64,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), isSecure(false), inService(false),
+               queue(NULL), order(0), blkAddr(0),
+               blkSize(0), isSecure(false), inService(false),
                isForward(false), threadNum(InvalidThreadID), data(NULL)
 {
 }
@@ -202,13 +202,13 @@ print(std::ostream &os, int verbosity, const std::string &prefix) const
 
 
 void
-MSHR::allocate(Addr _addr, int _size, PacketPtr target, Tick whenReady,
-               Counter _order)
+MSHR::allocate(Addr blk_addr, unsigned blk_size, PacketPtr target,
+               Tick when_ready, Counter _order)
 {
-    addr = _addr;
-    size = _size;
+    blkAddr = blk_addr;
+    blkSize = blk_size;
     isSecure = target->isSecure();
-    readyTime = whenReady;
+    readyTime = when_ready;
     order = _order;
     assert(target);
     isForward = false;
@@ -221,7 +221,7 @@ MSHR::allocate(Addr _addr, int _size, PacketPtr target, Tick whenReady,
     // snoop (mem-side request), so set source according to request here
     Target::Source source = (target->cmd == MemCmd::HardPFReq) ?
         Target::FromPrefetcher : Target::FromCPU;
-    targets.add(target, whenReady, _order, source, true);
+    targets.add(target, when_ready, _order, source, true);
     assert(deferredTargets.isReset());
     data = NULL;
 }
@@ -446,7 +446,7 @@ MSHR::checkFunctional(PacketPtr pkt)
     // For other requests, we iterate over the individual targets
     // since that's where the actual data lies.
     if (pkt->isPrint()) {
-        pkt->checkFunctional(this, addr, isSecure, size, NULL);
+        pkt->checkFunctional(this, blkAddr, isSecure, blkSize, NULL);
         return false;
     } else {
         return (targets.checkFunctional(pkt) ||
@@ -459,7 +459,7 @@ void
 MSHR::print(std::ostream &os, int verbosity, const std::string &prefix) const
 {
     ccprintf(os, "%s[%#llx:%#llx](%s) %s %s %s state: %s %s %s %s %s\n",
-             prefix, addr, addr+size-1,
+             prefix, blkAddr, blkAddr + blkSize - 1,
              isSecure ? "s" : "ns",
              isForward ? "Forward" : "",
              isForwardNoResponse() ? "ForwNoResp" : "",
diff --git a/src/mem/cache/mshr.hh b/src/mem/cache/mshr.hh
index c8967d2ea..86b8dee4d 100644
--- a/src/mem/cache/mshr.hh
+++ b/src/mem/cache/mshr.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2013 ARM Limited
+ * Copyright (c) 2012-2013, 2015 ARM Limited
  * All rights reserved.
  *
  * The license below extends only to copyright in the software and shall
@@ -45,8 +45,8 @@
  * Miss Status and Handling Register (MSHR) declaration.
  */
 
-#ifndef __MSHR_HH__
-#define __MSHR_HH__
+#ifndef __MEM_CACHE_MSHR_HH__
+#define __MEM_CACHE_MSHR_HH__
 
 #include <list>
 
@@ -149,11 +149,11 @@ class MSHR : public Packet::SenderState, public Printable
     /** Order number assigned by the miss queue. */
     Counter order;
 
-    /** Address of the request. */
-    Addr addr;
+    /** Block aligned address of the MSHR. */
+    Addr blkAddr;
 
-    /** Size of the request. */
-    int size;
+    /** Block size of the cache. */
+    unsigned blkSize;
 
     /** True if the request targets the secure memory space. */
     bool isSecure;
@@ -216,14 +216,14 @@ class MSHR : public Packet::SenderState, public Printable
 
     /**
      * Allocate a miss to this MSHR.
-     * @param cmd The requesting command.
-     * @param addr The address of the miss.
-     * @param asid The address space id of the miss.
-     * @param size The number of bytes to request.
-     * @param pkt  The original miss.
+     * @param blk_addr The address of the block.
+     * @param blk_size The number of bytes to request.
+     * @param pkt The original miss.
+     * @param when_ready When should the MSHR be ready to act upon.
+     * @param _order The logical order of this MSHR
      */
-    void allocate(Addr addr, int size, PacketPtr pkt,
-                  Tick when, Counter _order);
+    void allocate(Addr blk_addr, unsigned blk_size, PacketPtr pkt,
+                  Tick when_ready, Counter _order);
 
     bool markInService(bool pending_dirty_resp);
 
@@ -304,4 +304,4 @@ class MSHR : public Packet::SenderState, public Printable
     std::string print() const;
 };
 
-#endif //__MSHR_HH__
+#endif // __MEM_CACHE_MSHR_HH__
diff --git a/src/mem/cache/mshr_queue.cc b/src/mem/cache/mshr_queue.cc
index 72e9b2ec3..788793aff 100644
--- a/src/mem/cache/mshr_queue.cc
+++ b/src/mem/cache/mshr_queue.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2013 ARM Limited
+ * Copyright (c) 2012-2013, 2015 ARM Limited
  * All rights reserved.
  *
  * The license below extends only to copyright in the software and shall
@@ -66,13 +66,13 @@ MSHRQueue::MSHRQueue(const std::string &_label,
 }
 
 MSHR *
-MSHRQueue::findMatch(Addr addr, bool is_secure) const
+MSHRQueue::findMatch(Addr blk_addr, bool is_secure) const
 {
     MSHR::ConstIterator i = allocatedList.begin();
     MSHR::ConstIterator end = allocatedList.end();
     for (; i != end; ++i) {
         MSHR *mshr = *i;
-        if (mshr->addr == addr && mshr->isSecure == is_secure) {
+        if (mshr->blkAddr == blk_addr && mshr->isSecure == is_secure) {
             return mshr;
         }
     }
@@ -80,7 +80,8 @@ MSHRQueue::findMatch(Addr addr, bool is_secure) const
 }
 
 bool
-MSHRQueue::findMatches(Addr addr, bool is_secure, vector<MSHR*>& matches) const
+MSHRQueue::findMatches(Addr blk_addr, bool is_secure,
+                       vector<MSHR*>& matches) const
 {
     // Need an empty vector
     assert(matches.empty());
@@ -89,7 +90,7 @@ MSHRQueue::findMatches(Addr addr, bool is_secure, vector<MSHR*>& matches) const
     MSHR::ConstIterator end = allocatedList.end();
     for (; i != end; ++i) {
         MSHR *mshr = *i;
-        if (mshr->addr == addr && mshr->isSecure == is_secure) {
+        if (mshr->blkAddr == blk_addr && mshr->isSecure == is_secure) {
             retval = true;
             matches.push_back(mshr);
         }
@@ -106,7 +107,7 @@ MSHRQueue::checkFunctional(PacketPtr pkt, Addr blk_addr)
     MSHR::ConstIterator end = allocatedList.end();
     for (; i != end; ++i) {
         MSHR *mshr = *i;
-        if (mshr->addr == blk_addr && mshr->checkFunctional(pkt)) {
+        if (mshr->blkAddr == blk_addr && mshr->checkFunctional(pkt)) {
             pkt->popLabel();
             return true;
         }
@@ -117,20 +118,14 @@ MSHRQueue::checkFunctional(PacketPtr pkt, Addr blk_addr)
 
 
 MSHR *
-MSHRQueue::findPending(Addr addr, int size, bool is_secure) const
+MSHRQueue::findPending(Addr blk_addr, bool is_secure) const
 {
     MSHR::ConstIterator i = readyList.begin();
     MSHR::ConstIterator end = readyList.end();
     for (; i != end; ++i) {
         MSHR *mshr = *i;
-        if (mshr->isSecure == is_secure) {
-            if (mshr->addr < addr) {
-                if (mshr->addr + mshr->size > addr)
-                    return mshr;
-            } else {
-                if (addr + size > mshr->addr)
-                    return mshr;
-            }
+        if (mshr->blkAddr == blk_addr && mshr->isSecure == is_secure) {
+            return mshr;
         }
     }
     return NULL;
@@ -157,15 +152,15 @@ MSHRQueue::addToReadyList(MSHR *mshr)
 
 
 MSHR *
-MSHRQueue::allocate(Addr addr, int size, PacketPtr &pkt,
-                    Tick when, Counter order)
+MSHRQueue::allocate(Addr blk_addr, unsigned blk_size, PacketPtr pkt,
+                    Tick when_ready, Counter order)
 {
     assert(!freeList.empty());
     MSHR *mshr = freeList.front();
     assert(mshr->getNumTargets() == 0);
     freeList.pop_front();
 
-    mshr->allocate(addr, size, pkt, when, order);
+    mshr->allocate(blk_addr, blk_size, pkt, when_ready, order);
     mshr->allocIter = allocatedList.insert(allocatedList.end(), mshr);
     mshr->readyIter = addToReadyList(mshr);
 
diff --git a/src/mem/cache/mshr_queue.hh b/src/mem/cache/mshr_queue.hh
index 1e1218782..4043bc565 100644
--- a/src/mem/cache/mshr_queue.hh
+++ b/src/mem/cache/mshr_queue.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2013 ARM Limited
+ * Copyright (c) 2012-2013, 2015 ARM Limited
  * All rights reserved.
  *
  * The license below extends only to copyright in the software and shall
@@ -45,8 +45,8 @@
  * Declaration of a structure to manage MSHRs.
  */
 
-#ifndef __MEM__CACHE__MISS__MSHR_QUEUE_HH__
-#define __MEM__CACHE__MISS__MSHR_QUEUE_HH__
+#ifndef __MEM_CACHE_MSHR_QUEUE_HH__
+#define __MEM_CACHE_MSHR_QUEUE_HH__
 
 #include <vector>
 
@@ -120,44 +120,48 @@ class MSHRQueue : public Drainable
 
     /**
      * Find the first MSHR that matches the provided address.
-     * @param addr The address to find.
+     * @param blk_addr The block address to find.
      * @param is_secure True if the target memory space is secure.
      * @return Pointer to the matching MSHR, null if not found.
      */
-    MSHR *findMatch(Addr addr, bool is_secure) const;
+    MSHR *findMatch(Addr blk_addr, bool is_secure) const;
 
     /**
      * Find and return all the matching entries in the provided vector.
-     * @param addr The address to find.
+     * @param blk_addr The block  address to find.
      * @param is_secure True if the target memory space is secure.
      * @param matches The vector to return pointers to the matching entries.
      * @return True if any matches are found, false otherwise.
-     * @todo Typedef the vector??
      */
-    bool findMatches(Addr addr, bool is_secure,
+    bool findMatches(Addr blk_addr, bool is_secure,
                      std::vector<MSHR*>& matches) const;
 
     /**
      * Find any pending requests that overlap the given request.
-     * @param pkt The request to find.
+     * @param blk_addr Block address.
      * @param is_secure True if the target memory space is secure.
      * @return A pointer to the earliest matching MSHR.
      */
-    MSHR *findPending(Addr addr, int size, bool is_secure) const;
+    MSHR *findPending(Addr blk_addr, bool is_secure) const;
 
     bool checkFunctional(PacketPtr pkt, Addr blk_addr);
 
     /**
      * Allocates a new MSHR for the request and size. This places the request
      * as the first target in the MSHR.
-     * @param pkt The request to handle.
-     * @param size The number in bytes to fetch from memory.
+     *
+     * @param blk_addr The address of the block.
+     * @param blk_size The number of bytes to request.
+     * @param pkt The original miss.
+     * @param when_ready When should the MSHR be ready to act upon.
+     * @param order The logical order of this MSHR
+     *
      * @return The a pointer to the MSHR allocated.
      *
      * @pre There are free entries.
      */
-    MSHR *allocate(Addr addr, int size, PacketPtr &pkt,
-                   Tick when, Counter order);
+    MSHR *allocate(Addr blk_addr, unsigned blk_size, PacketPtr pkt,
+                   Tick when_ready, Counter order);
 
     /**
      * Removes the given MSHR from the queue. This places the MSHR on the
@@ -257,4 +261,4 @@ class MSHRQueue : public Drainable
     unsigned int drain(DrainManager *dm);
 };
 
-#endif //__MEM__CACHE__MISS__MSHR_QUEUE_HH__
+#endif //__MEM_CACHE_MSHR_QUEUE_HH__