From 21aa5734a0f2c03263e26c66e5cd95ad64c70697 Mon Sep 17 00:00:00 2001
From: David Hashe <david.hashe@amd.com>
Date: Mon, 20 Jul 2015 09:15:18 -0500
Subject: [PATCH] ruby: fix deadlock bug in banked array resource checks

The Ruby banked array resource checks (initiated from SLICC) did a check and
allocate at the same time. If a transition needs more than one resource, then
it might check/allocate resource #1, then fail to get resource #2. Another
transition might then try to get the same resources, but in reverse order.
Deadlock.

This patch separates resource checking and resource reservation into two
steps to avoid deadlock.
---
 src/mem/protocol/RubySlicc_Types.sm    |  2 +-
 src/mem/ruby/structures/BankedArray.cc | 28 +++++++++++++++++++-------
 src/mem/ruby/structures/BankedArray.hh |  2 ++
 src/mem/ruby/structures/CacheMemory.cc | 12 ++++++++++-
 src/mem/ruby/structures/CacheMemory.hh |  2 +-
 5 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/src/mem/protocol/RubySlicc_Types.sm b/src/mem/protocol/RubySlicc_Types.sm
index 6c3c4168d..11159fd6c 100644
--- a/src/mem/protocol/RubySlicc_Types.sm
+++ b/src/mem/protocol/RubySlicc_Types.sm
@@ -154,7 +154,7 @@ structure (CacheMemory, external = "yes") {
   Cycles getTagLatency();
   Cycles getDataLatency();
   void setMRU(Address);
-  void recordRequestType(CacheRequestType);
+  void recordRequestType(CacheRequestType, Address);
   bool checkResourceAvailable(CacheResourceType, Address);
 
   int getCacheSize();
diff --git a/src/mem/ruby/structures/BankedArray.cc b/src/mem/ruby/structures/BankedArray.cc
index dbde2ab9a..8bc3cf584 100644
--- a/src/mem/ruby/structures/BankedArray.cc
+++ b/src/mem/ruby/structures/BankedArray.cc
@@ -58,13 +58,29 @@ BankedArray::tryAccess(int64 idx)
     assert(bank < banks);
 
     if (busyBanks[bank].endAccess >= curTick()) {
-        if (!(busyBanks[bank].startAccess == curTick() &&
-            busyBanks[bank].idx == idx)) {
             return false;
+    }
+
+    return true;
+}
+
+void
+BankedArray::reserve(int64 idx)
+{
+    if (accessLatency == 0)
+        return;
+
+    unsigned int bank = mapIndexToBank(idx);
+    assert(bank < banks);
+
+    if(busyBanks[bank].endAccess >= curTick()) {
+        if (busyBanks[bank].startAccess == curTick() &&
+             busyBanks[bank].idx == idx) {
+            // this is the same reservation (can happen when
+            // e.g., reserve the same resource for read and write)
+            return; // OK
         } else {
-            // We tried to allocate resources twice
-            // in the same cycle for the same addr
-            return true;
+            panic("BankedArray reservation error");
         }
     }
 
@@ -72,8 +88,6 @@ BankedArray::tryAccess(int64 idx)
     busyBanks[bank].startAccess = curTick();
     busyBanks[bank].endAccess = curTick() +
         (accessLatency-1) * m_ruby_system->clockPeriod();
-
-    return true;
 }
 
 unsigned int
diff --git a/src/mem/ruby/structures/BankedArray.hh b/src/mem/ruby/structures/BankedArray.hh
index dbfee9994..438186944 100644
--- a/src/mem/ruby/structures/BankedArray.hh
+++ b/src/mem/ruby/structures/BankedArray.hh
@@ -70,6 +70,8 @@ class BankedArray
     // This is so we don't get aliasing on blocks being replaced
     bool tryAccess(int64 idx);
 
+    void reserve(int64 idx);
+
     Cycles getLatency() const { return accessLatency; }
 };
 
diff --git a/src/mem/ruby/structures/CacheMemory.cc b/src/mem/ruby/structures/CacheMemory.cc
index c802d8fb6..d08724cff 100644
--- a/src/mem/ruby/structures/CacheMemory.cc
+++ b/src/mem/ruby/structures/CacheMemory.cc
@@ -528,22 +528,32 @@ CacheMemory::regStats()
         ;
 }
 
+// assumption: SLICC generated files will only call this function
+// once **all** resources are granted
 void
-CacheMemory::recordRequestType(CacheRequestType requestType)
+CacheMemory::recordRequestType(CacheRequestType requestType, Address addr)
 {
     DPRINTF(RubyStats, "Recorded statistic: %s\n",
             CacheRequestType_to_string(requestType));
     switch(requestType) {
     case CacheRequestType_DataArrayRead:
+        if (m_resource_stalls)
+            dataArray.reserve(addressToCacheSet(addr));
         numDataArrayReads++;
         return;
     case CacheRequestType_DataArrayWrite:
+        if (m_resource_stalls)
+            dataArray.reserve(addressToCacheSet(addr));
         numDataArrayWrites++;
         return;
     case CacheRequestType_TagArrayRead:
+        if (m_resource_stalls)
+            tagArray.reserve(addressToCacheSet(addr));
         numTagArrayReads++;
         return;
     case CacheRequestType_TagArrayWrite:
+        if (m_resource_stalls)
+            tagArray.reserve(addressToCacheSet(addr));
         numTagArrayWrites++;
         return;
     default:
diff --git a/src/mem/ruby/structures/CacheMemory.hh b/src/mem/ruby/structures/CacheMemory.hh
index c9c20d8b8..af5e680d8 100644
--- a/src/mem/ruby/structures/CacheMemory.hh
+++ b/src/mem/ruby/structures/CacheMemory.hh
@@ -117,7 +117,7 @@ class CacheMemory : public SimObject
 
     void regStats();
     bool checkResourceAvailable(CacheResourceType res, Address addr);
-    void recordRequestType(CacheRequestType requestType);
+    void recordRequestType(CacheRequestType requestType, Address addr);
 
   public:
     Stats::Scalar m_demand_hits;
-- 
2.30.2