From: Geoffrey Blake <geoffrey.blake@arm.com>
Date: Wed, 3 Sep 2014 11:42:31 +0000 (-0400)
Subject: cache: Fix handling of LL/SC requests under contention
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=b404ffde608ce4f2c2175fdc7936f98da5d0998b;p=gem5.git

cache: Fix handling of LL/SC requests under contention

If a set of LL/SC requests contend on the same cache block we
can get into a situation where CPUs will deadlock if they expect
a failed SC to supply them data.  This case happens where 3 or
more cores are contending for a cache block using LL/SC and the system
is configured where 2 cores are connected to a local bus and the
third is connected to a remote bus.  If a core on the local bus
sends an SCUpgrade and the core on the remote bus sends and SCUpgrade
they will race to see who will win the SC access.  In the meantime
if the other core appends a read to one of the SCUpgrades it will expect
to be supplied data by that SCUpgrade transaction.  If it happens that
the SCUpgrade that was picked to supply the data is failed, it will
drop the appended request for data and never respond, leaving the requesting
core to deadlock.  This patch makes all SC's behave as normal stores to
prevent this case but still makes sure to check whether it can perform
the update.
---

diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index 5ef45ea2f..34dacdf9f 100644
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2013 ARM Limited
+ * Copyright (c) 2010-2014 ARM Limited
  * All rights reserved.
  *
  * The license below extends only to copyright in the software and shall
@@ -166,8 +166,12 @@ Cache<TagStore>::satisfyCpuSideRequest(PacketPtr pkt, BlkType *blk,
     } else if (pkt->isWrite()) {
         if (blk->checkWrite(pkt)) {
             pkt->writeDataToBlock(blk->data, blkSize);
-            blk->status |= BlkDirty;
         }
+        // Always mark the line as dirty even if we are a failed
+        // StoreCond so we supply data to any snoops that have
+        // appended themselves to this cache before knowing the store
+        // will fail.
+        blk->status |= BlkDirty;
     } else if (pkt->isRead()) {
         if (pkt->isLLSC()) {
             blk->trackLoadLocked(pkt);
@@ -658,6 +662,13 @@ Cache<TagStore>::getBusPacket(PacketPtr cpu_pkt, BlkType *blk,
         // (read-only) and we need exclusive
         assert(needsExclusive && !blk->isWritable());
         cmd = cpu_pkt->isLLSC() ? MemCmd::SCUpgradeReq : MemCmd::UpgradeReq;
+    } else if (cpu_pkt->cmd == MemCmd::SCUpgradeFailReq ||
+               cpu_pkt->cmd == MemCmd::StoreCondFailReq) {
+        // Even though this SC will fail, we still need to send out the
+        // request and get the data to supply it to other snoopers in the case
+        // where the determination the StoreCond fails is delayed due to
+        // all caches not being on the same local bus.
+        cmd = MemCmd::SCUpgradeFailReq;
     } else {
         // block is invalid
         cmd = needsExclusive ? MemCmd::ReadExReq : MemCmd::ReadReq;
@@ -1724,18 +1735,7 @@ Cache<TagStore>::getTimingPacket()
     DPRINTF(CachePort, "%s %s for address %x size %d\n", __func__,
             tgt_pkt->cmdString(), tgt_pkt->getAddr(), tgt_pkt->getSize());
 
-    if (tgt_pkt->cmd == MemCmd::SCUpgradeFailReq ||
-        tgt_pkt->cmd == MemCmd::StoreCondFailReq) {
-        // SCUpgradeReq or StoreCondReq saw invalidation while queued
-        // in MSHR, so now that we are getting around to processing
-        // it, just treat it as if we got a failure response
-        pkt = new Packet(tgt_pkt);
-        pkt->cmd = MemCmd::UpgradeFailResp;
-        pkt->senderState = mshr;
-        pkt->busFirstWordDelay = pkt->busLastWordDelay = 0;
-        recvTimingResp(pkt);
-        return NULL;
-    } else if (mshr->isForwardNoResponse()) {
+    if (mshr->isForwardNoResponse()) {
         // no response expected, just forward packet as it is
         assert(tags->findBlock(mshr->addr, mshr->isSecure) == NULL);
         pkt = tgt_pkt;
diff --git a/src/mem/packet.cc b/src/mem/packet.cc
index faa9d9a53..4ff531e80 100644
--- a/src/mem/packet.cc
+++ b/src/mem/packet.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011-2013 ARM Limited
+ * Copyright (c) 2011-2014 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -115,12 +115,13 @@ MemCmd::commandInfo[] =
     /* UpgradeResp */
     { SET3(NeedsExclusive, IsUpgrade, IsResponse),
             InvalidCmd, "UpgradeResp" },
-    /* SCUpgradeFailReq: generates UpgradeFailResp ASAP */
-    { SET5(IsInvalidate, NeedsExclusive, IsLlsc,
-           IsRequest, NeedsResponse),
+    /* SCUpgradeFailReq: generates UpgradeFailResp but still gets the data */
+    { SET6(IsRead, NeedsExclusive, IsInvalidate,
+           IsLlsc, IsRequest, NeedsResponse),
             UpgradeFailResp, "SCUpgradeFailReq" },
-    /* UpgradeFailResp */
-    { SET2(NeedsExclusive, IsResponse),
+    /* UpgradeFailResp - Behaves like a ReadExReq, but notifies an SC
+     * that it has failed, acquires line as Dirty*/
+    { SET4(IsRead, NeedsExclusive, IsResponse, HasData),
             InvalidCmd, "UpgradeFailResp" },
     /* ReadExReq */
     { SET5(IsRead, NeedsExclusive, IsInvalidate, IsRequest, NeedsResponse),
@@ -136,7 +137,7 @@ MemCmd::commandInfo[] =
     { SET6(IsWrite, NeedsExclusive, IsLlsc,
            IsRequest, NeedsResponse, HasData),
             StoreCondResp, "StoreCondReq" },
-    /* StoreCondFailReq: generates failing StoreCondResp ASAP */
+    /* StoreCondFailReq: generates failing StoreCondResp */
     { SET6(IsWrite, NeedsExclusive, IsLlsc,
            IsRequest, NeedsResponse, HasData),
             StoreCondResp, "StoreCondFailReq" },