MEM: Separate snoops and normal memory requests/responses

This patch introduces port access methods that separates snoop
request/responses from normal memory request/responses. The
differentiation is made for functional, atomic and timing accesses and
builds on the introduction of master and slave ports.

Before the introduction of this patch, the packets belonging to the
different phases of the protocol (request -> [forwarded snoop request
-> snoop response]* -> response) all use the same port access
functions, even though the snoop packets flow in the opposite
direction to the normal packet. That is, a coherent master sends
normal request and receives responses, but receives snoop requests and
sends snoop responses (vice versa for the slave). These two distinct
phases now use different access functions, as described below.

Starting with the functional access, a master sends a request to a
slave through sendFunctional, and the request packet is turned into a
response before the call returns. In a system without cache coherence,
this is all that is needed from the functional interface. For the
cache-coherent scenario, a slave also sends snoop requests to coherent
masters through sendFunctionalSnoop, with responses returned within
the same packet pointer. This is currently used by the bus and caches,
and the LSQ of the O3 CPU. The send/recvFunctional and
send/recvFunctionalSnoop are moved from the Port super class to the
appropriate subclass.

Atomic accesses follow the same flow as functional accesses, with
request being sent from master to slave through sendAtomic. In the
case of cache-coherent ports, a slave can send snoop requests to a
master through sendAtomicSnoop. Just as for the functional access
methods, the atomic send and receive member functions are moved to the
appropriate subclasses.

The timing access methods are different from the functional and atomic
in that requests and responses are separated in time and
send/recvTiming are used for both directions. Hence, a master uses
sendTiming to send a request to a slave, and a slave uses sendTiming
to send a response back to a master, at a later point in time. Snoop
requests and responses travel in the opposite direction, similar to
what happens in functional and atomic accesses. With the introduction
of this patch, it is possible to determine the direction of packets in
the bus, and no longer necessary to look for both a master and a slave
port with the requested port id.

In contrast to the normal recvFunctional, recvAtomic and recvTiming
that are pure virtual functions, the recvFunctionalSnoop,
recvAtomicSnoop and recvTimingSnoop have a default implementation that
calls panic. This is to allow non-coherent master and slave ports to
not implement these functions.

19 files changed, 76 insertions, 128 deletions

diff --git a/src/cpu/base.cc b/src/cpu/base.cc
index d01dcbef3..edbec8c80 100644
--- a/src/cpu/base.cc
+++ b/src/cpu/base.cc
@@ -534,27 +534,20 @@ BaseCPU::traceFunctionsInternal(Addr pc)
 bool
 BaseCPU::CpuPort::recvTiming(PacketPtr pkt)
 {
-    panic("BaseCPU doesn't expect recvTiming callback!");
+    panic("BaseCPU doesn't expect recvTiming!\n");
     return true;
 }
 
 void
 BaseCPU::CpuPort::recvRetry()
 {
-    panic("BaseCPU doesn't expect recvRetry callback!");
-}
-
-Tick
-BaseCPU::CpuPort::recvAtomic(PacketPtr pkt)
-{
-    panic("BaseCPU doesn't expect recvAtomic callback!");
-    return curTick();
+    panic("BaseCPU doesn't expect recvRetry!\n");
 }
 
 void
-BaseCPU::CpuPort::recvFunctional(PacketPtr pkt)
+BaseCPU::CpuPort::recvFunctionalSnoop(PacketPtr pkt)
 {
-    // No internal storage to update (in the general case). In the
-    // long term this should never be called, but that assumed a split
-    // into master/slave and request/response.
+    // No internal storage to update (in the general case). A CPU with
+    // internal storage, e.g. an LSQ that should be part of the
+    // coherent memory has to check against stored data.
 }
diff --git a/src/cpu/base.hh b/src/cpu/base.hh
index 3fb0f648b..f94c5e0a4 100644
--- a/src/cpu/base.hh
+++ b/src/cpu/base.hh
@@ -135,11 +135,9 @@ class BaseCPU : public MemObject
 
         virtual bool recvTiming(PacketPtr pkt);
 
-        virtual Tick recvAtomic(PacketPtr pkt);
-
         virtual void recvRetry();
 
-        void recvFunctional(PacketPtr pkt);
+        virtual void recvFunctionalSnoop(PacketPtr pkt);
 
     };
 
diff --git a/src/cpu/inorder/cpu.cc b/src/cpu/inorder/cpu.cc
index 8dab82d71..04176c54f 100644
--- a/src/cpu/inorder/cpu.cc
+++ b/src/cpu/inorder/cpu.cc
@@ -90,16 +90,13 @@ InOrderCPU::CachePort::CachePort(CacheUnit *_cacheUnit) :
 bool
 InOrderCPU::CachePort::recvTiming(Packet *pkt)
 {
+    assert(pkt->isResponse());
+
     if (pkt->isError())
         DPRINTF(InOrderCachePort, "Got error packet back for address: %x\n",
                 pkt->getAddr());
-    else if (pkt->isResponse())
+    else
         cacheUnit->processCacheCompletion(pkt);
-    else {
-        //@note: depending on consistency model, update here
-        DPRINTF(InOrderCachePort, "Received snoop pkt %x,Ignoring\n",
-                pkt->getAddr());
-    }
 
     return true;
 }
diff --git a/src/cpu/inorder/cpu.hh b/src/cpu/inorder/cpu.hh
index d8fe5c057..06d733d85 100644
--- a/src/cpu/inorder/cpu.hh
+++ b/src/cpu/inorder/cpu.hh
@@ -174,6 +174,9 @@ class InOrderCPU : public BaseCPU
 
         /** Handles doing a retry of a failed timing request. */
         void recvRetry();
+
+        /** Ignoring snoops for now. */
+        bool recvTimingSnoop(PacketPtr pkt) { return true; }
     };
 
     /** Define TickEvent for the CPU */
diff --git a/src/cpu/o3/cpu.cc b/src/cpu/o3/cpu.cc
index bfc9438d3..fe70c3fcf 100644
--- a/src/cpu/o3/cpu.cc
+++ b/src/cpu/o3/cpu.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 ARM Limited
+ * Copyright (c) 2011-2012 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -89,14 +89,12 @@ template<class Impl>
 bool
 FullO3CPU<Impl>::IcachePort::recvTiming(PacketPtr pkt)
 {
+    assert(pkt->isResponse());
     DPRINTF(O3CPU, "Fetch unit received timing\n");
-    if (pkt->isResponse()) {
-        // We shouldn't ever get a block in ownership state
-        assert(!(pkt->memInhibitAsserted() && !pkt->sharedAsserted()));
+    // We shouldn't ever get a block in ownership state
+    assert(!(pkt->memInhibitAsserted() && !pkt->sharedAsserted()));
+    fetch->processCacheCompletion(pkt);
 
-        fetch->processCacheCompletion(pkt);
-    }
-    //else Snooped a coherence request, just return
     return true;
 }
 
@@ -111,10 +109,19 @@ template <class Impl>
 bool
 FullO3CPU<Impl>::DcachePort::recvTiming(PacketPtr pkt)
 {
+    assert(pkt->isResponse());
     return lsq->recvTiming(pkt);
 }
 
 template <class Impl>
+bool
+FullO3CPU<Impl>::DcachePort::recvTimingSnoop(PacketPtr pkt)
+{
+    assert(pkt->isRequest());
+    return lsq->recvTimingSnoop(pkt);
+}
+
+template <class Impl>
 void
 FullO3CPU<Impl>::DcachePort::recvRetry()
 {
diff --git a/src/cpu/o3/cpu.hh b/src/cpu/o3/cpu.hh
index 493730458..be51f415f 100644
--- a/src/cpu/o3/cpu.hh
+++ b/src/cpu/o3/cpu.hh
@@ -149,6 +149,7 @@ class FullO3CPU : public BaseO3CPU
         /** Timing version of receive.  Handles setting fetch to the
          * proper status to start fetching. */
         virtual bool recvTiming(PacketPtr pkt);
+        virtual bool recvTimingSnoop(PacketPtr pkt) { return true; }
 
         /** Handles doing a retry of a failed fetch. */
         virtual void recvRetry();
@@ -176,6 +177,7 @@ class FullO3CPU : public BaseO3CPU
          * completing the load or store that has returned from
          * memory. */
         virtual bool recvTiming(PacketPtr pkt);
+        virtual bool recvTimingSnoop(PacketPtr pkt);
 
         /** Handles doing a retry of the previous send. */
         virtual void recvRetry();
diff --git a/src/cpu/o3/lsq.hh b/src/cpu/o3/lsq.hh
index b821dd3f9..dac5fab18 100644
--- a/src/cpu/o3/lsq.hh
+++ b/src/cpu/o3/lsq.hh
@@ -299,6 +299,8 @@ class LSQ {
      */
     bool recvTiming(PacketPtr pkt);
 
+    bool recvTimingSnoop(PacketPtr pkt);
+
     /** The CPU pointer. */
     O3CPU *cpu;
 
diff --git a/src/cpu/o3/lsq_impl.hh b/src/cpu/o3/lsq_impl.hh
index 02758f212..c2f410e37 100644
--- a/src/cpu/o3/lsq_impl.hh
+++ b/src/cpu/o3/lsq_impl.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 ARM Limited
+ * Copyright (c) 2011-2012 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -321,25 +321,32 @@ template <class Impl>
 bool
 LSQ<Impl>::recvTiming(PacketPtr pkt)
 {
+    assert(pkt->isResponse());
     if (pkt->isError())
         DPRINTF(LSQ, "Got error packet back for address: %#X\n",
                 pkt->getAddr());
-    if (pkt->isResponse()) {
-        thread[pkt->req->threadId()].completeDataAccess(pkt);
-    } else {
-        DPRINTF(LSQ, "received pkt for addr:%#x %s\n", pkt->getAddr(),
-                pkt->cmdString());
-
-        // must be a snoop
-        if (pkt->isInvalidate()) {
-            DPRINTF(LSQ, "received invalidation for addr:%#x\n",
-                    pkt->getAddr());
-            for (ThreadID tid = 0; tid < numThreads; tid++) {
-                thread[tid].checkSnoop(pkt);
-            }
+    thread[pkt->req->threadId()].completeDataAccess(pkt);
+    return true;
+}
+
+template <class Impl>
+bool
+LSQ<Impl>::recvTimingSnoop(PacketPtr pkt)
+{
+    assert(pkt->isRequest());
+    DPRINTF(LSQ, "received pkt for addr:%#x %s\n", pkt->getAddr(),
+            pkt->cmdString());
+
+    // must be a snoop
+    if (pkt->isInvalidate()) {
+        DPRINTF(LSQ, "received invalidation for addr:%#x\n",
+                pkt->getAddr());
+        for (ThreadID tid = 0; tid < numThreads; tid++) {
+            thread[tid].checkSnoop(pkt);
         }
-        // to provide stronger consistency model
     }
+
+    // to provide stronger consistency model
     return true;
 }
 
diff --git a/src/cpu/simple/atomic.hh b/src/cpu/simple/atomic.hh
index 3e6238f7d..e88c93cce 100644
--- a/src/cpu/simple/atomic.hh
+++ b/src/cpu/simple/atomic.hh
@@ -91,7 +91,7 @@ class AtomicSimpleCPU : public BaseSimpleCPU
 
       protected:
 
-        virtual Tick recvAtomic(PacketPtr pkt)
+        virtual Tick recvAtomicSnoop(PacketPtr pkt)
         {
             // Snooping a coherence request, just return
             return 0;
diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple/timing.cc
index f661756da..d52003f19 100644
--- a/src/cpu/simple/timing.cc
+++ b/src/cpu/simple/timing.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010 ARM Limited
+ * Copyright (c) 2010-2012 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -718,7 +718,8 @@ TimingSimpleCPU::IcachePort::ITickEvent::process()
 bool
 TimingSimpleCPU::IcachePort::recvTiming(PacketPtr pkt)
 {
-    if (pkt->isResponse() && !pkt->wasNacked()) {
+    assert(pkt->isResponse());
+    if (!pkt->wasNacked()) {
         DPRINTF(SimpleCPU, "Received timing response %#x\n", pkt->getAddr());
         // delay processing of returned data until next CPU clock edge
         Tick next_tick = cpu->nextCycle(curTick());
@@ -729,7 +730,7 @@ TimingSimpleCPU::IcachePort::recvTiming(PacketPtr pkt)
             tickEvent.schedule(pkt, next_tick);
 
         return true;
-    } else if (pkt->wasNacked()) {
+    } else {
         assert(cpu->_status == IcacheWaitResponse);
         pkt->reinitNacked();
         if (!sendTiming(pkt)) {
@@ -737,7 +738,7 @@ TimingSimpleCPU::IcachePort::recvTiming(PacketPtr pkt)
             cpu->ifetch_pkt = pkt;
         }
     }
-    //Snooping a Coherence Request, do nothing
+
     return true;
 }
 
@@ -838,7 +839,8 @@ TimingSimpleCPU::completeDrain()
 bool
 TimingSimpleCPU::DcachePort::recvTiming(PacketPtr pkt)
 {
-    if (pkt->isResponse() && !pkt->wasNacked()) {
+    assert(pkt->isResponse());
+    if (!pkt->wasNacked()) {
         // delay processing of returned data until next CPU clock edge
         Tick next_tick = cpu->nextCycle(curTick());
 
@@ -858,8 +860,7 @@ TimingSimpleCPU::DcachePort::recvTiming(PacketPtr pkt)
         }
 
         return true;
-    }
-    else if (pkt->wasNacked()) {
+    } else  {
         assert(cpu->_status == DcacheWaitResponse);
         pkt->reinitNacked();
         if (!sendTiming(pkt)) {
@@ -867,7 +868,7 @@ TimingSimpleCPU::DcachePort::recvTiming(PacketPtr pkt)
             cpu->dcache_pkt = pkt;
         }
     }
-    //Snooping a Coherence Request, do nothing
+
     return true;
 }
 
diff --git a/src/cpu/simple/timing.hh b/src/cpu/simple/timing.hh
index e0c5c89f7..4c23391d9 100644
--- a/src/cpu/simple/timing.hh
+++ b/src/cpu/simple/timing.hh
@@ -153,6 +153,11 @@ class TimingSimpleCPU : public BaseSimpleCPU
 
       protected:
 
+        /**
+         * Snooping a coherence request, do nothing.
+         */
+        virtual bool recvTimingSnoop(PacketPtr pkt) { return true; }
+
         TimingSimpleCPU* cpu;
 
         struct TickEvent : public Event
diff --git a/src/cpu/testers/directedtest/RubyDirectedTester.cc b/src/cpu/testers/directedtest/RubyDirectedTester.cc
index bfdd28e08..a6dc257d5 100644
--- a/src/cpu/testers/directedtest/RubyDirectedTester.cc
+++ b/src/cpu/testers/directedtest/RubyDirectedTester.cc
@@ -90,13 +90,6 @@ RubyDirectedTester::getMasterPort(const std::string &if_name, int idx)
     }
 }
 
-Tick
-RubyDirectedTester::CpuPort::recvAtomic(PacketPtr pkt)
-{
-    panic("RubyDirectedTester::CpuPort::recvAtomic() not implemented!\n");
-    return 0;
-}
-
 bool
 RubyDirectedTester::CpuPort::recvTiming(PacketPtr pkt)
 {
diff --git a/src/cpu/testers/directedtest/RubyDirectedTester.hh b/src/cpu/testers/directedtest/RubyDirectedTester.hh
index cb207ff80..0965fb786 100644
--- a/src/cpu/testers/directedtest/RubyDirectedTester.hh
+++ b/src/cpu/testers/directedtest/RubyDirectedTester.hh
@@ -64,8 +64,6 @@ class RubyDirectedTester : public MemObject
         virtual bool recvTiming(PacketPtr pkt);
         virtual void recvRetry()
         { panic("%s does not expect a retry\n", name()); }
-        virtual Tick recvAtomic(PacketPtr pkt);
-        virtual void recvFunctional(PacketPtr pkt) { }
     };
 
     typedef RubyDirectedTesterParams Params;
diff --git a/src/cpu/testers/memtest/memtest.cc b/src/cpu/testers/memtest/memtest.cc
index 07cdf73a6..7e34c2833 100644
--- a/src/cpu/testers/memtest/memtest.cc
+++ b/src/cpu/testers/memtest/memtest.cc
@@ -55,35 +55,11 @@ int TESTER_ALLOCATOR=0;
 bool
 MemTest::CpuPort::recvTiming(PacketPtr pkt)
 {
-    if (pkt->isResponse()) {
-        memtest->completeRequest(pkt);
-    } else {
-        // must be snoop upcall
-        assert(pkt->isRequest());
-        assert(pkt->getDest() == Packet::Broadcast);
-    }
+    assert(pkt->isResponse());
+    memtest->completeRequest(pkt);
     return true;
 }
 
-Tick
-MemTest::CpuPort::recvAtomic(PacketPtr pkt)
-{
-    // must be snoop upcall
-    assert(pkt->isRequest());
-    assert(pkt->getDest() == Packet::Broadcast);
-    return curTick();
-}
-
-void
-MemTest::CpuPort::recvFunctional(PacketPtr pkt)
-{
-    //Do nothing if we see one come through
-//    if (curTick() != 0)//Supress warning durring initialization
-//        warn("Functional Writes not implemented in MemTester\n");
-    //Need to find any response values that intersect and update
-    return;
-}
-
 void
 MemTest::CpuPort::recvRetry()
 {
diff --git a/src/cpu/testers/memtest/memtest.hh b/src/cpu/testers/memtest/memtest.hh
index d179fa530..8dccfdc80 100644
--- a/src/cpu/testers/memtest/memtest.hh
+++ b/src/cpu/testers/memtest/memtest.hh
@@ -99,9 +99,11 @@ class MemTest : public MemObject
 
         virtual bool recvTiming(PacketPtr pkt);
 
-        virtual Tick recvAtomic(PacketPtr pkt);
+        virtual bool recvTimingSnoop(PacketPtr pkt) { return true; }
 
-        virtual void recvFunctional(PacketPtr pkt);
+        virtual Tick recvAtomicSnoop(PacketPtr pkt) { return 0; }
+
+        virtual void recvFunctionalSnoop(PacketPtr pkt) { }
 
         virtual void recvRetry();
     };
diff --git a/src/cpu/testers/networktest/networktest.cc b/src/cpu/testers/networktest/networktest.cc
index c4d44b1ab..45a414539 100644
--- a/src/cpu/testers/networktest/networktest.cc
+++ b/src/cpu/testers/networktest/networktest.cc
@@ -53,34 +53,11 @@ int TESTER_NETWORK=0;
 bool
 NetworkTest::CpuPort::recvTiming(PacketPtr pkt)
 {
-    if (pkt->isResponse()) {
-        networktest->completeRequest(pkt);
-    } else {
-        // must be snoop upcall
-        assert(pkt->isRequest());
-        assert(pkt->getDest() == Packet::Broadcast);
-    }
+    assert(pkt->isResponse());
+    networktest->completeRequest(pkt);
     return true;
 }
 
-Tick
-NetworkTest::CpuPort::recvAtomic(PacketPtr pkt)
-{
-    panic("NetworkTest doesn't expect recvAtomic call!");
-    // Will not be used
-    assert(pkt->isRequest());
-    assert(pkt->getDest() == Packet::Broadcast);
-    return curTick();
-}
-
-void
-NetworkTest::CpuPort::recvFunctional(PacketPtr pkt)
-{
-    panic("NetworkTest doesn't expect recvFunctional call!");
-    // Will not be used
-    return;
-}
-
 void
 NetworkTest::CpuPort::recvRetry()
 {
diff --git a/src/cpu/testers/networktest/networktest.hh b/src/cpu/testers/networktest/networktest.hh
index 21984f45d..36d311aa8 100644
--- a/src/cpu/testers/networktest/networktest.hh
+++ b/src/cpu/testers/networktest/networktest.hh
@@ -94,10 +94,6 @@ class NetworkTest : public MemObject
 
         virtual bool recvTiming(PacketPtr pkt);
 
-        virtual Tick recvAtomic(PacketPtr pkt);
-
-        virtual void recvFunctional(PacketPtr pkt);
-
         virtual void recvRetry();
     };
 
diff --git a/src/cpu/testers/rubytest/RubyTester.cc b/src/cpu/testers/rubytest/RubyTester.cc
index 657167394..67f4c372a 100644
--- a/src/cpu/testers/rubytest/RubyTester.cc
+++ b/src/cpu/testers/rubytest/RubyTester.cc
@@ -146,13 +146,6 @@ RubyTester::getMasterPort(const std::string &if_name, int idx)
     }
 }
 
-Tick
-RubyTester::CpuPort::recvAtomic(PacketPtr pkt)
-{
-    panic("RubyTester::CpuPort::recvAtomic() not implemented!\n");
-    return 0;
-}
-
 bool
 RubyTester::CpuPort::recvTiming(PacketPtr pkt)
 {
diff --git a/src/cpu/testers/rubytest/RubyTester.hh b/src/cpu/testers/rubytest/RubyTester.hh
index 82698f201..aaf609e1e 100644
--- a/src/cpu/testers/rubytest/RubyTester.hh
+++ b/src/cpu/testers/rubytest/RubyTester.hh
@@ -78,8 +78,6 @@ class RubyTester : public MemObject
         virtual bool recvTiming(PacketPtr pkt);
         virtual void recvRetry()
         { panic("%s does not expect a retry\n", name()); }
-        virtual Tick recvAtomic(PacketPtr pkt);
-        virtual void recvFunctional(PacketPtr pkt) { }
     };
 
     struct SenderState : public Packet::SenderState