ruby: Removed static members in RubyPort including hitcallback

Removed static members in RubyPort and removed the ruby request unique id.

10 files changed, 118 insertions, 155 deletions

diff --git a/src/mem/protocol/RubySlicc_Exports.sm b/src/mem/protocol/RubySlicc_Exports.sm
index 412fd0de0..8f1e5a6eb 100644
--- a/src/mem/protocol/RubySlicc_Exports.sm
+++ b/src/mem/protocol/RubySlicc_Exports.sm
@@ -338,5 +338,12 @@ enumeration(TransientRequestType, desc="...", default="TransientRequestType_Unde
   LocalTransient, desc="";
 }
 
-
-
+// Request Status
+enumeration(RequestStatus, desc="...", default="RequestStatus_NULL")  {
+  Ready, desc="The sequencer is ready and the request does not alias";
+  Issued, desc="The sequencer successfully issued the request";
+  BufferFull, desc="Can not issue because the sequencer is full";
+  Aliased, desc="This request aliased with a currently outstanding request";
+  LlscFailed, desc="The write failed in the Load-Link Store-Conditional pair";
+  NULL, desc="";
+}
diff --git a/src/mem/ruby/libruby.cc b/src/mem/ruby/libruby.cc
index 8cb03e952..5a164176f 100644
--- a/src/mem/ruby/libruby.cc
+++ b/src/mem/ruby/libruby.cc
@@ -132,7 +132,11 @@ void libruby_init(const char* cfg_filename)
 
 RubyPortHandle libruby_get_port(const char* port_name, void (*hit_callback)(int64_t access_id))
 {
-  return static_cast<RubyPortHandle>(RubySystem::getPort(port_name, hit_callback));
+  //
+  // Fix me: Hit callback is now a non-static member function pointer of 
+  // RubyPort and cannot be set to an arbitrary global function
+  //
+  return NULL;//static_cast<RubyPortHandle>(RubySystem::getPort(port_name, hit_callback));
 }
 
 RubyPortHandle libruby_get_port_by_name(const char* port_name)
diff --git a/src/mem/ruby/libruby.hh b/src/mem/ruby/libruby.hh
index 4c50611c1..60f37da3f 100644
--- a/src/mem/ruby/libruby.hh
+++ b/src/mem/ruby/libruby.hh
@@ -4,6 +4,7 @@
 
 #include <stdint.h>
 #include <ostream>
+#include "mem/packet.hh"
 
 typedef void* RubyPortHandle;
 enum RubyRequestType {
@@ -31,11 +32,26 @@ struct RubyRequest {
   uint64_t pc;
   RubyRequestType type;
   RubyAccessMode access_mode;
+  PacketPtr pkt;
   unsigned proc_id;
 
   RubyRequest() {}
-  RubyRequest(uint64_t _paddr, uint8_t* _data, int _len, uint64_t _pc, RubyRequestType _type, RubyAccessMode _access_mode, unsigned _proc_id = 100)
-    : paddr(_paddr), data(_data), len(_len), pc(_pc), type(_type), access_mode(_access_mode), proc_id(_proc_id)
+  RubyRequest(uint64_t _paddr, 
+              uint8_t* _data, 
+              int _len, 
+              uint64_t _pc, 
+              RubyRequestType _type, 
+              RubyAccessMode _access_mode, 
+              PacketPtr _pkt,
+              unsigned _proc_id = 100)
+    : paddr(_paddr), 
+      data(_data), 
+      len(_len), 
+      pc(_pc), 
+      type(_type), 
+      access_mode(_access_mode), 
+      pkt(_pkt),
+      proc_id(_proc_id)
   {}
 };
 
@@ -71,12 +87,6 @@ RubyPortHandle libruby_get_port(const char* name, void (*hit_callback)(int64_t a
 RubyPortHandle libruby_get_port_by_name(const char* name);
 
 
-/** 
- * libruby_issue_request error return codes 
- */
-#define LIBRUBY_BUFFER_FULL -2
-#define LIBRUBY_ALIASED_REQUEST -3
-
 /**
  * issue_request returns a unique access_id to identify the ruby
  * transaction. This access_id is later returned to the caller via
diff --git a/src/mem/ruby/recorder/TraceRecord.cc b/src/mem/ruby/recorder/TraceRecord.cc
index e700687d6..81d1e17d8 100644
--- a/src/mem/ruby/recorder/TraceRecord.cc
+++ b/src/mem/ruby/recorder/TraceRecord.cc
@@ -85,7 +85,8 @@ void TraceRecord::issueRequest() const
                       RubySystem::getBlockSizeBytes(), 
                       m_pc_address.getAddress(), 
                       m_type, 
-                      RubyAccessMode_User);
+                      RubyAccessMode_User,
+                      NULL);
 
   // Clear out the sequencer
   while (!m_sequencer_ptr->empty()) {
diff --git a/src/mem/ruby/system/DMASequencer.cc b/src/mem/ruby/system/DMASequencer.cc
index 007f92b80..2fc28d6ae 100644
--- a/src/mem/ruby/system/DMASequencer.cc
+++ b/src/mem/ruby/system/DMASequencer.cc
@@ -24,7 +24,7 @@ void DMASequencer::init()
   m_data_block_mask = ~ (~0 << RubySystem::getBlockSizeBits());
 }
 
-int64_t DMASequencer::makeRequest(const RubyRequest & request)
+RequestStatus DMASequencer::makeRequest(const RubyRequest & request)
 {
   uint64_t paddr = request.paddr;
   uint8_t* data = request.data;
@@ -44,7 +44,8 @@ int64_t DMASequencer::makeRequest(const RubyRequest & request)
   case RubyRequestType_RMW_Read:
   case RubyRequestType_RMW_Write:
   case RubyRequestType_NUM:
-    assert(0);
+    panic("DMASequencer::makeRequest does not support the RubyRequestType");
+    return RequestStatus_NULL;
   }
 
   assert(!m_is_busy);  // only support one outstanding DMA request
@@ -56,7 +57,7 @@ int64_t DMASequencer::makeRequest(const RubyRequest & request)
   active_request.len = len;
   active_request.bytes_completed = 0;
   active_request.bytes_issued = 0;
-  active_request.id = makeUniqueRequestID();
+  active_request.pkt = request.pkt;
 
   SequencerMsg msg;
   msg.getPhysicalAddress() = Address(paddr);
@@ -76,7 +77,7 @@ int64_t DMASequencer::makeRequest(const RubyRequest & request)
   m_mandatory_q_ptr->enqueue(msg);
   active_request.bytes_issued += msg.getLen();
 
-  return active_request.id;
+  return RequestStatus_Issued;
 }
 
 void DMASequencer::issueNext()
@@ -84,14 +85,14 @@ void DMASequencer::issueNext()
   assert(m_is_busy == true);
   active_request.bytes_completed = active_request.bytes_issued;
   if (active_request.len == active_request.bytes_completed) {
-    m_hit_callback(active_request.id);
+    ruby_hit_callback(active_request.pkt);
     m_is_busy = false;
     return;
   }
 
   SequencerMsg msg;
   msg.getPhysicalAddress() = Address(active_request.start_paddr + 
-				     active_request.bytes_completed);
+                                     active_request.bytes_completed);
 
   assert((msg.getPhysicalAddress().getAddress() & m_data_block_mask) == 0);
   msg.getLineAddress() = line_address(msg.getPhysicalAddress());
diff --git a/src/mem/ruby/system/DMASequencer.hh b/src/mem/ruby/system/DMASequencer.hh
index 12a5c790f..f42e2600e 100644
--- a/src/mem/ruby/system/DMASequencer.hh
+++ b/src/mem/ruby/system/DMASequencer.hh
@@ -15,7 +15,7 @@ struct DMARequest {
   int bytes_completed;
   int bytes_issued;
   uint8* data;
-  int64_t id;
+  PacketPtr pkt;
 };
 
 class DMASequencer :public RubyPort {
@@ -24,9 +24,7 @@ public:
   DMASequencer(const Params *);
   void init();
   /* external interface */
-  int64_t makeRequest(const RubyRequest & request);
-  bool isReady(const RubyRequest & request, bool dont_set = false) { assert(0); return false;};
-  //  void issueRequest(uint64_t paddr, uint8* data, int len, bool rw);
+  RequestStatus makeRequest(const RubyRequest & request);
   bool busy() { return m_is_busy;}
 
   /* SLICC callback */
diff --git a/src/mem/ruby/system/RubyPort.cc b/src/mem/ruby/system/RubyPort.cc
index e4742dbab..07a06bd07 100644
--- a/src/mem/ruby/system/RubyPort.cc
+++ b/src/mem/ruby/system/RubyPort.cc
@@ -32,10 +32,6 @@
 #include "mem/ruby/slicc_interface/AbstractController.hh"
 #include "cpu/rubytest/RubyTester.hh"
 
-uint16_t RubyPort::m_num_ports = 0;
-
-RubyPort::RequestMap RubyPort::pending_cpu_requests;
-
 RubyPort::RubyPort(const Params *p)
     : MemObject(p)
 {
@@ -47,12 +43,9 @@ RubyPort::RubyPort(const Params *p)
     m_controller = NULL;
     m_mandatory_q_ptr = NULL;
 
-    m_port_id = m_num_ports++;
     m_request_cnt = 0;
-    m_hit_callback = ruby_hit_callback;
     pio_port = NULL;
     physMemPort = NULL;
-    assert(m_num_ports <= 2048); // see below for reason
 }
 
 void RubyPort::init()
@@ -169,8 +162,7 @@ RubyPort::M5Port::recvTiming(PacketPtr pkt)
     //dsm: based on SimpleTimingPort::recvTiming(pkt);
 
     //
-    // After checking for pio responses, the remainder of packets
-    // received by ruby should only be M5 requests, which should never 
+    // The received packets should only be M5 requests, which should never 
     // get nacked.  There used to be code to hanldle nacks here, but 
     // I'm pretty sure it didn't work correctly with the drain code, 
     // so that would need to be fixed if we ever added it back.
@@ -186,17 +178,20 @@ RubyPort::M5Port::recvTiming(PacketPtr pkt)
     }
 
     //
+    // Save the port in the sender state object to be used later to
+    // route the response
+    //
+    pkt->senderState = new SenderState(this, pkt->senderState);
+
+    //
     // Check for pio requests and directly send them to the dedicated
     // pio port.
     //
     if (!isPhysMemAddress(pkt->getAddr())) {
         assert(ruby_port->pio_port != NULL);
-
-        //
-        // Save the port in the sender state object to be used later to
-        // route the response
-        //
-        pkt->senderState = new SenderState(this, pkt->senderState);
+        DPRINTF(MemoryAccess, 
+                "Request for address 0x%#x is assumed to be a pio request\n",
+                pkt->getAddr());
 
         return ruby_port->pio_port->sendTiming(pkt);
     }
@@ -225,41 +220,49 @@ RubyPort::M5Port::recvTiming(PacketPtr pkt)
         type = RubyRequestType_ST;
     } else if (pkt->isReadWrite()) {
         type = RubyRequestType_RMW_Write;
+    } else {
+      panic("Unsupported ruby packet type\n");
     }
 
-    RubyRequest ruby_request(pkt->getAddr(), pkt->getPtr<uint8_t>(),
-                             pkt->getSize(), pc, type,
-                             RubyAccessMode_Supervisor);
+    RubyRequest ruby_request(pkt->getAddr(), 
+                             pkt->getPtr<uint8_t>(),
+                             pkt->getSize(), 
+                             pc, 
+                             type,
+                             RubyAccessMode_Supervisor,
+                             pkt);
 
     // Submit the ruby request
-    int64_t req_id = ruby_port->makeRequest(ruby_request);
-    if (req_id == -1) {
-        return false;
+    RequestStatus requestStatus = ruby_port->makeRequest(ruby_request);
+    if (requestStatus == RequestStatus_Issued) {
+        return true;
     }
-
-    // Save the request for the callback
-    RubyPort::pending_cpu_requests[req_id] = new RequestCookie(pkt, this);
-
-    return true;
+     
+    DPRINTF(MemoryAccess, 
+            "Request for address #x did not issue because %s\n",
+            pkt->getAddr(),
+            RequestStatus_to_string(requestStatus));
+    
+    SenderState* senderState = safe_cast<SenderState*>(pkt->senderState);
+    pkt->senderState = senderState->saved;
+    delete senderState;
+    return false;
 }
 
 void
-RubyPort::ruby_hit_callback(int64_t req_id)
+RubyPort::ruby_hit_callback(PacketPtr pkt)
 {
     //
-    // Note: This single fuction can be called by cpu and dma ports,
-    // as well as the functional port.  
+    // Retrieve the request port from the sender State
     //
-    RequestMap::iterator i = pending_cpu_requests.find(req_id);
-    if (i == pending_cpu_requests.end())
-        panic("could not find pending request %d\n", req_id);
-
-    RequestCookie *cookie = i->second;
-    pending_cpu_requests.erase(i);
-
-    Packet *pkt = cookie->pkt;
-    M5Port *port = cookie->m5Port;
-    delete cookie;
+    RubyPort::SenderState *senderState = 
+        safe_cast<RubyPort::SenderState *>(pkt->senderState);
+    M5Port *port = senderState->port;
+    assert(port != NULL);
+    
+    // pop the sender state from the packet
+    pkt->senderState = senderState->saved;
+    delete senderState;
 
     port->hitCallback(pkt);
 }
diff --git a/src/mem/ruby/system/RubyPort.hh b/src/mem/ruby/system/RubyPort.hh
index 267b557ec..3a862e068 100644
--- a/src/mem/ruby/system/RubyPort.hh
+++ b/src/mem/ruby/system/RubyPort.hh
@@ -37,6 +37,7 @@
 #include "mem/mem_object.hh"
 #include "mem/tport.hh"
 #include "mem/physical.hh"
+#include "mem/protocol/RequestStatus.hh"
 
 #include "params/RubyPort.hh"
 
@@ -105,16 +106,7 @@ public:
 
     Port *getPort(const std::string &if_name, int idx);
 
-    virtual int64_t makeRequest(const RubyRequest & request) = 0;
-
-    void registerHitCallback(void (*hit_callback)(int64_t request_id)) {
-        //
-        // Can't assign hit_callback twice and by default it is set to the
-        // RubyPort's default callback function.
-        //
-        assert(m_hit_callback == ruby_hit_callback); 
-        m_hit_callback = hit_callback;
-    }
+    virtual RequestStatus makeRequest(const RubyRequest & request) = 0;
 
     //
     // Called by the controller to give the sequencer a pointer.
@@ -124,55 +116,17 @@ public:
 
 protected:
   const string m_name;
-  void (*m_hit_callback)(int64_t);
-
-  int64_t makeUniqueRequestID() {
-    // The request ID is generated by combining the port ID with a request count
-    // so that request IDs can be formed concurrently by multiple threads.
-    // IDs are formed as follows:
-    //
-    //
-    //   0        PortID                         Request Count
-    // +----+---------------+-----------------------------------------------------+
-    // | 63 |     62-48     |                         47-0                        |
-    // +----+---------------+-----------------------------------------------------+
-    //
-    //
-    //  This limits the system to a maximum of 2^11 == 2048 components
-    //  and 2^48 ~= 3x10^14 requests per component
-
-    int64_t id = (static_cast<uint64_t>(m_port_id) << 48) | m_request_cnt;
-    m_request_cnt++;
-    // assert((m_request_cnt & (1<<48)) == 0);
-    return id;
-  }
+  void ruby_hit_callback(PacketPtr pkt);
+  void hit(PacketPtr pkt);
 
   int m_version;
   AbstractController* m_controller;
   MessageBuffer* m_mandatory_q_ptr;
     PioPort* pio_port;
 
-    //
-    // The pending request map is protected so that the Sequencer can access it.
-    // This is a temporary fix until the libruby inteface is cleaned
-    //
-    struct RequestCookie {
-        Packet *pkt;
-        M5Port *m5Port;
-        RequestCookie(Packet *p, M5Port *m5p)
-            : pkt(p), m5Port(m5p)
-        {}
-    };
-
-    typedef std::map<int64_t, RequestCookie*> RequestMap;
-    static RequestMap pending_cpu_requests;
-
 private:
-  static uint16_t m_num_ports;
-  uint16_t m_port_id;
-  uint64_t m_request_cnt;
-
-    static void ruby_hit_callback(int64_t req_id);
+    uint16_t m_port_id;
+    uint64_t m_request_cnt;
 
     M5Port* physMemPort;
 
diff --git a/src/mem/ruby/system/Sequencer.cc b/src/mem/ruby/system/Sequencer.cc
index 578dcbf58..b471a4b8b 100644
--- a/src/mem/ruby/system/Sequencer.cc
+++ b/src/mem/ruby/system/Sequencer.cc
@@ -45,11 +45,6 @@
 
 #include "params/RubySequencer.hh"
 
-//Sequencer::Sequencer(int core_id, MessageBuffer* mandatory_q)
-
-#define LLSC_FAIL -2
-long int already = 0;
-
 Sequencer *
 RubySequencerParams::create()
 {
@@ -352,30 +347,18 @@ void Sequencer::hitCallback(SequencerRequest* srequest, DataBlock& data) {
   // Note: RubyPort will access it's sender state before the RubyTester.
   //
   if (m_usingRubyTester) {
-      //
-      // Since the hit callback func only takes a request id, we must iterate
-      // through the requests and update the packet's subBlock here.
-      // All this would be fixed if we could attach a M5 pkt pointer to the
-      // ruby request, however that change will break the libruby interface so
-      // we'll hold off on that for now.
-      //
-      RequestMap::iterator i = pending_cpu_requests.find(srequest->id);
-      if (i == pending_cpu_requests.end())
-          panic("could not find pending request %d\n", srequest->id);
-      RequestCookie *cookie = i->second;
-      Packet *pkt = cookie->pkt;
-
       RubyTester::SenderState* testerSenderState;
-      testerSenderState = safe_cast<RubyTester::SenderState*>(pkt->senderState);
+      testerSenderState = safe_cast<RubyTester::SenderState*>( \
+          safe_cast<RubyPort::SenderState*>(ruby_request.pkt->senderState)->saved);
       testerSenderState->subBlock->mergeFrom(data);
   }
 
-  m_hit_callback(srequest->id);
+  ruby_hit_callback(ruby_request.pkt);
   delete srequest;
 }
 
 // Returns true if the sequencer already has a load or store outstanding
-int Sequencer::isReady(const RubyRequest& request) {
+RequestStatus Sequencer::getRequestStatus(const RubyRequest& request) {
   bool is_outstanding_store = m_writeRequestTable.exist(line_address(Address(request.paddr)));
   bool is_outstanding_load = m_readRequestTable.exist(line_address(Address(request.paddr)));
   if ( is_outstanding_store ) {
@@ -386,7 +369,7 @@ int Sequencer::isReady(const RubyRequest& request) {
     } else {
       m_store_waiting_on_store_cycles++;
     }
-    return LIBRUBY_ALIASED_REQUEST;
+    return RequestStatus_Aliased;
   } else if ( is_outstanding_load ) {
     if ((request.type == RubyRequestType_ST) ||
         (request.type == RubyRequestType_RMW_Write) ) {
@@ -394,14 +377,14 @@ int Sequencer::isReady(const RubyRequest& request) {
     } else {
       m_load_waiting_on_load_cycles++;
     }
-    return LIBRUBY_ALIASED_REQUEST;
+    return RequestStatus_Aliased;
   }
 
   if (m_outstanding_count >= m_max_outstanding_requests) {
-    return LIBRUBY_BUFFER_FULL;
+    return RequestStatus_BufferFull;
   }
   
-  return 1;
+  return RequestStatus_Ready;
 }
 
 bool Sequencer::empty() const {
@@ -409,20 +392,21 @@ bool Sequencer::empty() const {
 }
 
 
-int64_t Sequencer::makeRequest(const RubyRequest & request)
+RequestStatus Sequencer::makeRequest(const RubyRequest & request)
 {
-  assert(Address(request.paddr).getOffset() + request.len <= RubySystem::getBlockSizeBytes());
-  int ready = isReady(request);
-  if (ready > 0) {
-    int64_t id = makeUniqueRequestID();
-    SequencerRequest *srequest = new SequencerRequest(request, id, g_eventQueue_ptr->getTime());
+  assert(Address(request.paddr).getOffset() + request.len <= 
+         RubySystem::getBlockSizeBytes());
+  RequestStatus status = getRequestStatus(request);
+  if (status == RequestStatus_Ready) {
+    SequencerRequest *srequest = new SequencerRequest(request, 
+                                                  g_eventQueue_ptr->getTime());
     bool found = insertRequest(srequest);
     if (!found) {
       if (request.type == RubyRequestType_Locked_Write) {
         // NOTE: it is OK to check the locked flag here as the mandatory queue will be checked first
         // ensuring that nothing comes between checking the flag and servicing the store
         if (!m_dataCache_ptr->isLocked(line_address(Address(request.paddr)), m_version)) {
-          return LLSC_FAIL;
+          return RequestStatus_LlscFailed;
         }
         else {
           m_dataCache_ptr->clearLocked(line_address(Address(request.paddr)));
@@ -431,14 +415,15 @@ int64_t Sequencer::makeRequest(const RubyRequest & request)
       issueRequest(request);
 
       // TODO: issue hardware prefetches here
-      return id;
+      return RequestStatus_Issued;
     }
     else {
-      assert(0);
-      return 0;
+        panic("Sequencer::makeRequest should never be called if the request"\
+              "is already outstanding\n");
+        return RequestStatus_NULL;
     }
   } else {
-    return ready;
+    return status;
   }
 }
 
diff --git a/src/mem/ruby/system/Sequencer.hh b/src/mem/ruby/system/Sequencer.hh
index a571092f7..333ca5f24 100644
--- a/src/mem/ruby/system/Sequencer.hh
+++ b/src/mem/ruby/system/Sequencer.hh
@@ -56,11 +56,12 @@ class RubySequencerParams;
 
 struct SequencerRequest {
   RubyRequest ruby_request;
-  int64_t id;
   Time issue_time;
 
-  SequencerRequest(const RubyRequest & _ruby_request, int64_t _id, Time _issue_time)
-    : ruby_request(_ruby_request), id(_id), issue_time(_issue_time)
+  SequencerRequest(const RubyRequest & _ruby_request, 
+                   Time _issue_time)
+    : ruby_request(_ruby_request), 
+      issue_time(_issue_time)
   {}
 };
 
@@ -85,9 +86,8 @@ public:
   void writeCallback(const Address& address, DataBlock& data);
   void readCallback(const Address& address, DataBlock& data);
 
-  // called by Tester or Simics
-  int64_t makeRequest(const RubyRequest & request);
-  int isReady(const RubyRequest& request);
+  RequestStatus makeRequest(const RubyRequest & request);
+  RequestStatus getRequestStatus(const RubyRequest& request);
   bool empty() const;
 
   void print(ostream& out) const;