Matthew Poremba has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/29653 )

Change subject: mem-ruby: Support device memories
......................................................................

mem-ruby: Support device memories

Adds support for device memories in the system and RubySystem classes.
Devices may register memory ranges with the system class and packets
which originate from the device MasterID will update the device memory
in Ruby. In RubySystem functional access is updated to keep the packets
within the Ruby network they originated from.

Change-Id: I47850df1dc1994485d471ccd9da89e8d88eb0d20
JIRA: https://gem5.atlassian.net/browse/GEM5-470
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/29653
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Maintainer: Jason Lowe-Power <power...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/mem/ruby/network/Network.cc
M src/mem/ruby/slicc_interface/AbstractController.cc
M src/mem/ruby/slicc_interface/AbstractController.hh
M src/mem/ruby/system/RubyPort.cc
M src/mem/ruby/system/RubyPort.hh
M src/mem/ruby/system/RubySystem.cc
M src/mem/ruby/system/RubySystem.hh
M src/mem/ruby/system/RubySystem.py
M src/mem/slicc/symbols/StateMachine.py
M src/sim/system.cc
M src/sim/system.hh
11 files changed, 170 insertions(+), 29 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/mem/ruby/network/Network.cc b/src/mem/ruby/network/Network.cc
index 982b57e..ba847e5 100644
--- a/src/mem/ruby/network/Network.cc
+++ b/src/mem/ruby/network/Network.cc
@@ -55,6 +55,8 @@
     m_virtual_networks = p->number_of_virtual_networks;
     m_control_msg_size = p->control_msg_size;

+    params()->ruby_system->registerNetwork(this);
+
     // Populate localNodeVersions with the version of each MachineType in
     // this network. This will be used to compute a global to local ID.
     // Do this by looking at the ext_node for each ext_link. There is one
@@ -64,6 +66,7 @@
     for (auto &it : params()->ext_links) {
         AbstractController *cntrl = it->params()->ext_node;
         localNodeVersions[cntrl->getType()].push_back(cntrl->getVersion());
+ params()->ruby_system->registerMachineID(cntrl->getMachineID(), this);
     }

// Compute a local ID for each MachineType using the same order as SLICC
@@ -104,8 +107,6 @@
         m_ordered[i] = false;
     }

-    params()->ruby_system->registerNetwork(this);
-
     // Initialize the controller's network pointers
for (std::vector<BasicExtLink*>::const_iterator i = p->ext_links.begin();
          i != p->ext_links.end(); ++i) {
diff --git a/src/mem/ruby/slicc_interface/AbstractController.cc b/src/mem/ruby/slicc_interface/AbstractController.cc
index bdc88b9..9da8727 100644
--- a/src/mem/ruby/slicc_interface/AbstractController.cc
+++ b/src/mem/ruby/slicc_interface/AbstractController.cc
@@ -68,7 +68,6 @@
 void
 AbstractController::init()
 {
-    params()->ruby_system->registerAbstractController(this);
     m_delayHistogram.init(10);
     uint32_t size = Network::getNumberOfVirtualNetworks();
     for (uint32_t i = 0; i < size; i++) {
diff --git a/src/mem/ruby/slicc_interface/AbstractController.hh b/src/mem/ruby/slicc_interface/AbstractController.hh
index 1577cfa..750a620 100644
--- a/src/mem/ruby/slicc_interface/AbstractController.hh
+++ b/src/mem/ruby/slicc_interface/AbstractController.hh
@@ -147,6 +147,7 @@

   public:
     MachineID getMachineID() const { return m_machineID; }
+    MasterID getMasterId() const { return m_masterId; }

     Stats::Histogram& getDelayHist() { return m_delayHistogram; }
     Stats::Histogram& getDelayVCHist(uint32_t index)
diff --git a/src/mem/ruby/system/RubyPort.cc b/src/mem/ruby/system/RubyPort.cc
index 92fed81..0526e65 100644
--- a/src/mem/ruby/system/RubyPort.cc
+++ b/src/mem/ruby/system/RubyPort.cc
@@ -252,7 +252,7 @@
     // Check for pio requests and directly send them to the dedicated
     // pio port.
     if (pkt->cmd != MemCmd::MemFenceReq) {
-        if (!isPhysMemAddress(pkt->getAddr())) {
+        if (!isPhysMemAddress(pkt)) {
             assert(ruby_port->memMasterPort.isConnected());
             DPRINTF(RubyPort, "Request address %#x assumed to be a "
                     "pio address\n", pkt->getAddr());
@@ -313,7 +313,7 @@
     // Check for pio requests and directly send them to the dedicated
     // pio port.
     if (pkt->cmd != MemCmd::MemFenceReq) {
-        if (!isPhysMemAddress(pkt->getAddr())) {
+        if (!isPhysMemAddress(pkt)) {
             assert(ruby_port->memMasterPort.isConnected());
             DPRINTF(RubyPort, "Request address %#x assumed to be a "
                     "pio address\n", pkt->getAddr());
@@ -371,7 +371,7 @@

     // Check for pio requests and directly send them to the dedicated
     // pio port.
-    if (!isPhysMemAddress(pkt->getAddr())) {
+    if (!isPhysMemAddress(pkt)) {
DPRINTF(RubyPort, "Pio Request for address: 0x%#x\n", pkt->getAddr());
         assert(rp->pioMasterPort.isConnected());
         rp->pioMasterPort.sendFunctional(pkt);
@@ -431,7 +431,7 @@

     // The packet was destined for memory and has not yet been turned
     // into a response
-    assert(system->isMemAddr(pkt->getAddr()));
+ assert(system->isMemAddr(pkt->getAddr()) || system->isDeviceMemAddr(pkt));
     assert(pkt->isRequest());

     // First we must retrieve the request port from the sender State
@@ -553,7 +553,16 @@
     RubyPort *ruby_port = static_cast<RubyPort *>(&owner);
     RubySystem *rs = ruby_port->m_ruby_system;
     if (accessPhysMem) {
-        rs->getPhysMem()->access(pkt);
+        // We must check device memory first in case it overlaps with the
+        // system memory range.
+        if (ruby_port->system->isDeviceMemAddr(pkt)) {
+ auto dmem = ruby_port->system->getDeviceMemory(pkt->masterId());
+            dmem->access(pkt);
+        } else if (ruby_port->system->isMemAddr(pkt->getAddr())) {
+            rs->getPhysMem()->access(pkt);
+        } else {
+            panic("Packet is in neither device nor system memory!");
+        }
     } else if (needsResponse) {
         pkt->makeResponse();
     }
@@ -589,10 +598,11 @@
 }

 bool
-RubyPort::MemSlavePort::isPhysMemAddress(Addr addr) const
+RubyPort::MemSlavePort::isPhysMemAddress(PacketPtr pkt) const
 {
     RubyPort *ruby_port = static_cast<RubyPort *>(&owner);
-    return ruby_port->system->isMemAddr(addr);
+    return ruby_port->system->isMemAddr(pkt->getAddr())
+        || ruby_port->system->isDeviceMemAddr(pkt);
 }

 void
diff --git a/src/mem/ruby/system/RubyPort.hh b/src/mem/ruby/system/RubyPort.hh
index b14e707..1e21090 100644
--- a/src/mem/ruby/system/RubyPort.hh
+++ b/src/mem/ruby/system/RubyPort.hh
@@ -99,7 +99,7 @@
         void addToRetryList();

       private:
-        bool isPhysMemAddress(Addr addr) const;
+        bool isPhysMemAddress(PacketPtr pkt) const;
     };

     class PioMasterPort : public QueuedMasterPort
diff --git a/src/mem/ruby/system/RubySystem.cc b/src/mem/ruby/system/RubySystem.cc
index 9fa4736..5babf2d 100644
--- a/src/mem/ruby/system/RubySystem.cc
+++ b/src/mem/ruby/system/RubySystem.cc
@@ -57,6 +57,7 @@
 #include "mem/simple_mem.hh"
 #include "sim/eventq.hh"
 #include "sim/simulate.hh"
+#include "sim/system.hh"

 using namespace std;

@@ -106,6 +107,61 @@
     m_abstract_controls[id.getType()][id.getNum()] = cntrl;
 }

+void
+RubySystem::registerMachineID(const MachineID& mach_id, Network* network)
+{
+    int network_id = -1;
+    for (int idx = 0; idx < m_networks.size(); ++idx) {
+        if (m_networks[idx].get() == network) {
+            network_id = idx;
+        }
+    }
+
+ fatal_if(network_id < 0, "Could not add MachineID %s. Network not found",
+             MachineIDToString(mach_id).c_str());
+
+    machineToNetwork.insert(std::make_pair(mach_id, network_id));
+}
+
+// This registers all master IDs in the system for functional reads. This
+// should be called in init() since master IDs are obtained in a SimObject's
+// constructor and there are functional reads/writes between init() and
+// startup().
+void
+RubySystem::registerMasterIDs()
+{
+    // Create the map for MasterID to network node. This is done in init()
+    // because all MasterIDs must be obtained in the constructor and
+ // AbstractControllers are registered in their constructor. This is done
+    // in two steps: (1) Add all of the AbstractControllers. Since we don't
+    // have a mapping of MasterID to MachineID this is the easiest way to
+    // filter out AbstractControllers from non-Ruby masters. (2) Go through
+ // the system's list of MasterIDs and add missing MasterIDs to network 0
+    // (the default).
+    for (auto& cntrl : m_abs_cntrl_vec) {
+        MasterID mid = cntrl->getMasterId();
+        MachineID mach_id = cntrl->getMachineID();
+
+        // These are setup in Network constructor and should exist
+        fatal_if(!machineToNetwork.count(mach_id),
+                 "No machineID %s. Does not belong to a Ruby network?",
+                 MachineIDToString(mach_id).c_str());
+
+        auto network_id = machineToNetwork[mach_id];
+        masterToNetwork.insert(std::make_pair(mid, network_id));
+
+        // Create helper vectors for each network to iterate over.
+        netCntrls[network_id].push_back(cntrl);
+    }
+
+    // Default all other master IDs to network 0
+    for (auto mid = 0; mid < params()->system->maxMasters(); ++mid) {
+        if (!masterToNetwork.count(mid)) {
+            masterToNetwork.insert(std::make_pair(mid, 0));
+        }
+    }
+}
+
 RubySystem::~RubySystem()
 {
     delete m_profiler;
@@ -342,6 +398,12 @@
 }

 void
+RubySystem::init()
+{
+    registerMasterIDs();
+}
+
+void
 RubySystem::startup()
 {

@@ -418,7 +480,6 @@
     Addr line_address = makeLineAddress(address);

     AccessPermission access_perm = AccessPermission_NotPresent;
-    int num_controllers = m_abs_cntrl_vec.size();

     DPRINTF(RubySystem, "Functional Read request for %#x\n", address);

@@ -429,21 +490,26 @@
     unsigned int num_backing_store = 0;
     unsigned int num_invalid = 0;

+    // Only send functional requests within the same network.
+    assert(masterToNetwork.count(pkt->masterId()));
+    int master_net_id = masterToNetwork[pkt->masterId()];
+    assert(netCntrls.count(master_net_id));
+
     AbstractController *ctrl_ro = nullptr;
     AbstractController *ctrl_rw = nullptr;
     AbstractController *ctrl_backing_store = nullptr;

     // In this loop we count the number of controllers that have the given
     // address in read only, read write and busy states.
-    for (unsigned int i = 0; i < num_controllers; ++i) {
- access_perm = m_abs_cntrl_vec[i]-> getAccessPermission(line_address);
+    for (auto& cntrl : netCntrls[master_net_id]) {
+        access_perm = cntrl-> getAccessPermission(line_address);
         if (access_perm == AccessPermission_Read_Only){
             num_ro++;
-            if (ctrl_ro == nullptr) ctrl_ro = m_abs_cntrl_vec[i];
+            if (ctrl_ro == nullptr) ctrl_ro = cntrl;
         }
         else if (access_perm == AccessPermission_Read_Write){
             num_rw++;
-            if (ctrl_rw == nullptr) ctrl_rw = m_abs_cntrl_vec[i];
+            if (ctrl_rw == nullptr) ctrl_rw = cntrl;
         }
         else if (access_perm == AccessPermission_Busy)
             num_busy++;
@@ -456,7 +522,7 @@
             // or not.
             num_backing_store++;
             if (ctrl_backing_store == nullptr)
-                ctrl_backing_store = m_abs_cntrl_vec[i];
+                ctrl_backing_store = cntrl;
         }
         else if (access_perm == AccessPermission_Invalid ||
                  access_perm == AccessPermission_NotPresent)
@@ -471,6 +537,7 @@
// The reason is because the Backing_Store memory could easily be stale, if // there are copies floating around the cache hierarchy, so you want to read
     // it only if it's not in the cache hierarchy at all.
+    int num_controllers = netCntrls[master_net_id].size();
     if (num_invalid == (num_controllers - 1) && num_backing_store == 1) {
DPRINTF(RubySystem, "only copy in Backing_Store memory, read from it\n");
         ctrl_backing_store->functionalRead(line_address, pkt);
@@ -506,8 +573,8 @@
         DPRINTF(RubySystem, "Controllers functionalRead lookup "
                             "(num_maybe_stale=%d, num_busy = %d)\n",
                 num_maybe_stale, num_busy);
-        for (unsigned int i = 0; i < num_controllers;++i) {
-            if (m_abs_cntrl_vec[i]->functionalReadBuffers(pkt))
+        for (auto& cntrl : netCntrls[master_net_id]) {
+            if (cntrl->functionalReadBuffers(pkt))
                 return true;
         }
         DPRINTF(RubySystem, "Network functionalRead lookup "
@@ -532,32 +599,35 @@
     Addr addr(pkt->getAddr());
     Addr line_addr = makeLineAddress(addr);
     AccessPermission access_perm = AccessPermission_NotPresent;
-    int num_controllers = m_abs_cntrl_vec.size();

     DPRINTF(RubySystem, "Functional Write request for %#x\n", addr);

     uint32_t M5_VAR_USED num_functional_writes = 0;

-    for (unsigned int i = 0; i < num_controllers;++i) {
-        num_functional_writes +=
-            m_abs_cntrl_vec[i]->functionalWriteBuffers(pkt);
+    // Only send functional requests within the same network.
+    assert(masterToNetwork.count(pkt->masterId()));
+    int master_net_id = masterToNetwork[pkt->masterId()];
+    assert(netCntrls.count(master_net_id));

-        access_perm = m_abs_cntrl_vec[i]->getAccessPermission(line_addr);
+    for (auto& cntrl : netCntrls[master_net_id]) {
+        num_functional_writes += cntrl->functionalWriteBuffers(pkt);
+
+        access_perm = cntrl->getAccessPermission(line_addr);
         if (access_perm != AccessPermission_Invalid &&
             access_perm != AccessPermission_NotPresent) {
             num_functional_writes +=
-                m_abs_cntrl_vec[i]->functionalWrite(line_addr, pkt);
+                cntrl->functionalWrite(line_addr, pkt);
         }

         // Also updates requests pending in any sequencer associated
         // with the controller
-        if (m_abs_cntrl_vec[i]->getCPUSequencer()) {
+        if (cntrl->getCPUSequencer()) {
             num_functional_writes +=
- m_abs_cntrl_vec[i]->getCPUSequencer()->functionalWrite(pkt);
+                cntrl->getCPUSequencer()->functionalWrite(pkt);
         }
-        if (m_abs_cntrl_vec[i]->getDMASequencer()) {
+        if (cntrl->getDMASequencer()) {
             num_functional_writes +=
- m_abs_cntrl_vec[i]->getDMASequencer()->functionalWrite(pkt);
+                cntrl->getDMASequencer()->functionalWrite(pkt);
         }
     }

diff --git a/src/mem/ruby/system/RubySystem.hh b/src/mem/ruby/system/RubySystem.hh
index 2407072..7ce5ce0 100644
--- a/src/mem/ruby/system/RubySystem.hh
+++ b/src/mem/ruby/system/RubySystem.hh
@@ -35,6 +35,8 @@
 #ifndef __MEM_RUBY_SYSTEM_RUBYSYSTEM_HH__
 #define __MEM_RUBY_SYSTEM_RUBYSYSTEM_HH__

+#include <unordered_map>
+
 #include "base/callback.hh"
 #include "base/output.hh"
 #include "mem/packet.hh"
@@ -53,6 +55,7 @@
     typedef RubySystemParams Params;
     RubySystem(const Params *p);
     ~RubySystem();
+    const Params *params() const { return (const Params *)_params; }

     // config accessors
     static int getRandomization() { return m_randomization; }
@@ -86,12 +89,15 @@
     void unserialize(CheckpointIn &cp) override;
     void drainResume() override;
     void process();
+    void init() override;
     void startup() override;
     bool functionalRead(Packet *ptr);
     bool functionalWrite(Packet *ptr);

     void registerNetwork(Network*);
     void registerAbstractController(AbstractController*);
+    void registerMachineID(const MachineID& mach_id, Network* network);
+    void registerMasterIDs();

     bool eventQueueEmpty() { return eventq->empty(); }
     void enqueueRubyEvent(Tick tick)
@@ -135,6 +141,10 @@
     std::vector<AbstractController *> m_abs_cntrl_vec;
     Cycles m_start_cycle;

+    std::unordered_map<MachineID, unsigned> machineToNetwork;
+    std::unordered_map<MasterID, unsigned> masterToNetwork;
+ std::unordered_map<unsigned, std::vector<AbstractController*>> netCntrls;
+
   public:
     Profiler* m_profiler;
     CacheRecorder* m_cache_recorder;
diff --git a/src/mem/ruby/system/RubySystem.py b/src/mem/ruby/system/RubySystem.py
index 1ce4871..347896f 100644
--- a/src/mem/ruby/system/RubySystem.py
+++ b/src/mem/ruby/system/RubySystem.py
@@ -25,6 +25,7 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

 from m5.params import *
+from m5.proxy import *
 from m5.objects.ClockedObject import ClockedObject
 from m5.objects.SimpleMemory import *

@@ -41,6 +42,7 @@
         "number of bits that a memory address requires");

     phys_mem = Param.SimpleMemory(NULL, "")
+    system = Param.System(Parent.any, "system object")

access_backing_store = Param.Bool(False, "Use phys_mem as the functional \
         store and only use ruby for timing.")
diff --git a/src/mem/slicc/symbols/StateMachine.py b/src/mem/slicc/symbols/StateMachine.py
index 2e9700f..987f3b5 100644
--- a/src/mem/slicc/symbols/StateMachine.py
+++ b/src/mem/slicc/symbols/StateMachine.py
@@ -558,6 +558,7 @@
     m_machineID.type = MachineType_${ident};
     m_machineID.num = m_version;
     m_num_controllers++;
+    p->ruby_system->registerAbstractController(this);

     m_in_ports = $num_in_ports;
 ''')
diff --git a/src/sim/system.cc b/src/sim/system.cc
index 6e5975e..7057a97 100644
--- a/src/sim/system.cc
+++ b/src/sim/system.cc
@@ -418,6 +418,31 @@
 }

 void
+System::addDeviceMemory(MasterID masterId, AbstractMemory *deviceMemory)
+{
+    if (!deviceMemMap.count(masterId)) {
+        deviceMemMap.insert(std::make_pair(masterId, deviceMemory));
+    }
+}
+
+bool
+System::isDeviceMemAddr(PacketPtr pkt) const
+{
+    const MasterID& mid = pkt->masterId();
+
+    return (deviceMemMap.count(mid) &&
+            deviceMemMap.at(mid)->getAddrRange().contains(pkt->getAddr()));
+}
+
+AbstractMemory *
+System::getDeviceMemory(MasterID mid) const
+{
+    panic_if(!deviceMemMap.count(mid),
+             "No device memory found for MasterID %d\n", mid);
+    return deviceMemMap.at(mid);
+}
+
+void
 System::drainResume()
 {
     totalNumInsts = 0;
diff --git a/src/sim/system.hh b/src/sim/system.hh
index 72734f8..9480821 100644
--- a/src/sim/system.hh
+++ b/src/sim/system.hh
@@ -98,6 +98,9 @@
     std::list<PCEvent *> liveEvents;
     SystemPort _systemPort;

+ // Map of memory address ranges for devices with their own backing stores
+    std::unordered_map<MasterID, AbstractMemory *> deviceMemMap;
+
   public:

     class Threads
@@ -354,6 +357,25 @@
     bool isMemAddr(Addr addr) const;

     /**
+     * Add a physical memory range for a device. The ranges added here will
+     * be considered a non-PIO memory address if the masterId of the packet
+     * and range match something in the device memory map.
+     */
+    void addDeviceMemory(MasterID masterID, AbstractMemory *deviceMemory);
+
+    /**
+     * Similar to isMemAddr but for devices. Checks if a physical address
+ * of the packet match an address range of a device corresponding to the
+     * MasterId of the request.
+     */
+    bool isDeviceMemAddr(PacketPtr pkt) const;
+
+    /**
+     * Return a pointer to the device memory.
+     */
+    AbstractMemory *getDeviceMemory(MasterID masterID) const;
+
+    /**
      * Get the architecture.
      */
     Arch getArch() const { return Arch::TheISA; }

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/29653
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I47850df1dc1994485d471ccd9da89e8d88eb0d20
Gerrit-Change-Number: 29653
Gerrit-PatchSet: 4
Gerrit-Owner: Matthew Poremba <matthew.pore...@amd.com>
Gerrit-Reviewer: Bradford Beckmann <brad.beckm...@amd.com>
Gerrit-Reviewer: Gabe Black <gabebl...@google.com>
Gerrit-Reviewer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: Matthew Poremba <matthew.pore...@amd.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to