Support per-device arguments and device factory reuse

As proposed in #1652, we made the following changes to MMIO device (factory)
plugin API, to mitigate current limitations and facilitate factory reuse.

- removed `sargs` from `device_factory_t`, and introduced a new type alias
  `device_factory_sargs_t` to capture `<device_factory_t *, sargs>` pairs,
  this is used to instantiate sim_t instances;
- changed the signature of `device_factory_t::generate_fdt` and
  `device_factory_t::parse_from_fdt` to take on an extra `sargs` argument,
  for instantiating devices with per-device arguments;
- made `device_factory_t` const and potentially resuable across multiple
  `sim_t` instances.

ci-tests/test-customext.cc

@@ -63,7 +63,7 @@ make_mems(const std::vector<mem_cfg_t> &layout) {
 int main(int argc, char **argv) {
   cfg_t cfg;
   cfg.isa = "RV64IMAFDCV_xdummyslliuw";
-  std::vector<device_factory_t *> plugin_devices;
+  std::vector<device_factory_sargs_t> plugin_devices;
   if (argc != 3) {
     std::cerr << "Error: invalid arguments\n";
     exit(1);

ci-tests/testlib.c

@@ -14,7 +14,7 @@ static std::vector<std::pair<reg_t, abstract_mem_t*>> make_mems(const std::vecto
 
 int main(int argc, char **argv) {
   cfg_t cfg;
-  std::vector<device_factory_t*> plugin_devices;
+  std::vector<device_factory_sargs_t> plugin_devices;
 
   if (argc != 3) {
     std::cerr << "Error: invalid arguments\n";

riscv/abstract_device.h

@@ -24,18 +24,13 @@ class abstract_device_t {
 // parameterized by parsing the DTS
 class device_factory_t {
 public:
-  virtual abstract_device_t* parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base) const = 0;
+  virtual abstract_device_t* parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base, const std::vector<std::string>& sargs) const = 0;
-  virtual std::string generate_dts(const sim_t* sim) const = 0;
+  virtual std::string generate_dts(const sim_t* sim, const std::vector<std::string>& sargs) const = 0;
   virtual ~device_factory_t() {}
-  void set_sargs(std::vector<std::string> sargs) { this->sargs = sargs; }
-  std::vector<std::string> get_sargs() { return sargs; }
-
-protected:
-  std::vector<std::string> sargs;
 };
 
 // Type for holding all registered MMIO plugins by name.
-using mmio_device_map_t = std::map<std::string, device_factory_t*>;
+using mmio_device_map_t = std::map<std::string, const device_factory_t*>;
 
 mmio_device_map_t& mmio_device_map();
 
@@ -46,8 +41,8 @@ mmio_device_map_t& mmio_device_map();
     std::string str(#name); \
     if (!mmio_device_map().emplace(str, this).second) throw std::runtime_error("Plugin \"" + str + "\" already registered"); \
   }; \
-  name##_t* parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base) const override { return parse(fdt, sim, base, sargs); } \
+  name##_t* parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base, const std::vector<std::string>& sargs) const override { return parse(fdt, sim, base, sargs); } \
-  std::string generate_dts(const sim_t* sim) const override { return generate(sim); } \
+  std::string generate_dts(const sim_t* sim, const std::vector<std::string>& sargs) const override { return generate(sim, sargs); } \
   }; device_factory_t *name##_factory = new name##_factory_t();
 
 #endif

riscv/clint.cc

@@ -117,7 +117,7 @@ void clint_t::tick(reg_t rtc_ticks)
 }
 
 clint_t* clint_parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base,
-    const std::vector<std::string>& sargs) {
+    const std::vector<std::string>& UNUSED sargs) {
   if (fdt_parse_clint(fdt, base, "riscv,clint0") == 0 || fdt_parse_clint(fdt, base, "sifive,clint0") == 0)
     return new clint_t(sim,
                        sim->CPU_HZ / sim->INSNS_PER_RTC_TICK,
@@ -126,7 +126,7 @@ clint_t* clint_parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base,
     return nullptr;
 }
 
-std::string clint_generate_dts(const sim_t* sim) {
+std::string clint_generate_dts(const sim_t* sim, const std::vector<std::string>& UNUSED sargs) {
   std::stringstream s;
   s << std::hex
     << "    clint@" << CLINT_BASE << " {\n"

riscv/ns16550.cc

@@ -328,7 +328,7 @@ void ns16550_t::tick(reg_t UNUSED rtc_ticks)
   update_interrupt();
 }
 
-std::string ns16550_generate_dts(const sim_t* sim)
+std::string ns16550_generate_dts(const sim_t* sim, const std::vector<std::string>& UNUSED sargs)
 {
   std::stringstream s;
   s << std::hex
@@ -348,7 +348,7 @@ std::string ns16550_generate_dts(const sim_t* sim)
   return s.str();
 }
 
-ns16550_t* ns16550_parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base, const std::vector<std::string>& sargs)
+ns16550_t* ns16550_parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base, const std::vector<std::string>& UNUSED sargs)
 {
   uint32_t ns16550_shift, ns16550_io_width, ns16550_int_id;
   if (fdt_parse_ns16550(fdt, base,

riscv/plic.cc

@@ -401,7 +401,7 @@ bool plic_t::store(reg_t addr, size_t len, const uint8_t* bytes)
   return ret;
 }
 
-std::string plic_generate_dts(const sim_t* sim)
+std::string plic_generate_dts(const sim_t* sim, const std::vector<std::string>& UNUSED sargs)
 {
   std::stringstream s;
   s << std::hex
@@ -424,7 +424,7 @@ std::string plic_generate_dts(const sim_t* sim)
   return s.str();
 }
 
-plic_t* plic_parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base, const std::vector<std::string>& sargs)
+plic_t* plic_parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base, const std::vector<std::string>& UNUSED sargs)
 {
   uint32_t plic_ndev;
   if (fdt_parse_plic(fdt, base, &plic_ndev, "riscv,plic0") == 0 ||

riscv/sim.cc

@@ -38,7 +38,7 @@ extern device_factory_t* ns16550_factory;
 
 sim_t::sim_t(const cfg_t *cfg, bool halted,
              std::vector<std::pair<reg_t, abstract_mem_t*>> mems,
-             std::vector<device_factory_t*> plugin_device_factories,
+             const std::vector<device_factory_sargs_t>& plugin_device_factories,
              const std::vector<std::string>& args,
              const debug_module_config_t &dm_config,
              const char *log_path,
@@ -115,10 +115,10 @@ sim_t::sim_t(const cfg_t *cfg, bool halted,
   // that's not bus-accessible), but it should handle the normal use cases. In
   // particular, the default device tree configuration that you get without
   // setting the dtb_file argument has one.
-  std::vector<const device_factory_t*> device_factories = {
+  std::vector<device_factory_sargs_t> device_factories = {
-    clint_factory, // clint must be element 0
+    {clint_factory, {}}, // clint must be element 0
-    plic_factory, // plic must be element 1
+    {plic_factory, {}}, // plic must be element 1
-    ns16550_factory};
+    {ns16550_factory, {}}};
   device_factories.insert(device_factories.end(),
                           plugin_device_factories.begin(),
                           plugin_device_factories.end());
@@ -136,8 +136,11 @@ sim_t::sim_t(const cfg_t *cfg, bool halted,
   } else {
     std::pair<reg_t, reg_t> initrd_bounds = cfg->initrd_bounds;
     std::string device_nodes;
-    for (const device_factory_t *factory : device_factories)
+    for (const device_factory_sargs_t& factory_sargs: device_factories) {
-      device_nodes.append(factory->generate_dts(this));
+      const device_factory_t* factory = factory_sargs.first;
+      const std::vector<std::string>& sargs = factory_sargs.second;
+      device_nodes.append(factory->generate_dts(this, sargs));
+    }
     dts = make_dts(INSNS_PER_RTC_TICK, CPU_HZ,
                    initrd_bounds.first, initrd_bounds.second,
                    cfg->bootargs, cfg->pmpregions, cfg->pmpgranularity,
@@ -160,9 +163,10 @@ sim_t::sim_t(const cfg_t *cfg, bool halted,
   void *fdt = (void *)dtb.c_str();
 
   for (size_t i = 0; i < device_factories.size(); i++) {
-    const device_factory_t *factory = device_factories[i];
+    const device_factory_t* factory = device_factories[i].first;
+    const std::vector<std::string>& sargs = device_factories[i].second;
     reg_t device_base = 0;
-    abstract_device_t* device = factory->parse_from_fdt(fdt, this, &device_base);
+    abstract_device_t* device = factory->parse_from_fdt(fdt, this, &device_base, sargs);
     if (device) {
       assert(device_base);
       std::shared_ptr<abstract_device_t> dev_ptr(device);

riscv/sim.h

@@ -21,13 +21,16 @@ class mmu_t;
 class remote_bitbang_t;
 class socketif_t;
 
+// Type for holding a pair of device factory and device specialization arguments.
+using device_factory_sargs_t = std::pair<const device_factory_t*, std::vector<std::string>>;
+
 // this class encapsulates the processors and memory in a RISC-V machine.
 class sim_t : public htif_t, public simif_t
 {
 public:
   sim_t(const cfg_t *cfg, bool halted,
         std::vector<std::pair<reg_t, abstract_mem_t*>> mems,
-        std::vector<device_factory_t*> plugin_device_factories,
+        const std::vector<device_factory_sargs_t>& plugin_device_factories,
         const std::vector<std::string>& args,
         const debug_module_config_t &dm_config, const char *log_path,
         bool dtb_enabled, const char *dtb_file,

spike_main/spike.cc

@@ -335,7 +335,7 @@ int main(int argc, char** argv)
   bool dtb_enabled = true;
   const char* kernel = NULL;
   reg_t kernel_offset, kernel_size;
-  std::vector<device_factory_t*> plugin_device_factories;
+  std::vector<device_factory_sargs_t> plugin_device_factories;
   std::unique_ptr<icache_sim_t> ic;
   std::unique_ptr<dcache_sim_t> dc;
   std::unique_ptr<cache_sim_t> l2;
@@ -372,8 +372,7 @@ int main(int argc, char** argv)
     if (it == mmio_device_map().end()) throw std::runtime_error("Plugin \"" + name + "\" not found in loaded extlibs.");
 
     parsed_args.erase(parsed_args.begin());
-    it->second->set_sargs(parsed_args);
+    plugin_device_factories.push_back(std::make_pair(it->second, parsed_args));
-    plugin_device_factories.push_back(it->second);
   };
 
   option_parser_t parser;