devices: Switch plugin device interface to use device_factory_t

Plugins should now implement and register a device_factory_t to
configure how that device should be parsed from a FDT, and an optional
default DTS string.

This drops support for command-line flag-based device configuration

9 files changed, 32 insertions, 203 deletions

diff --git a/ci-tests/testlib.c b/ci-tests/testlib.c
index 33eaede..6342f9d 100644
--- a/ci-tests/testlib.c
+++ b/ci-tests/testlib.c
@@ -28,7 +28,7 @@ int main()
             hartids,
             false,
             4);
-  std::vector<std::pair<reg_t, std::shared_ptr<abstract_device_t>>> plugin_devices;
+  std::vector<const device_factory_t*> plugin_devices;
   std::vector<std::string> htif_args {"pk", "hello"};
   debug_module_config_t dm_config = {
     .progbufsize = 2,
diff --git a/riscv/abstract_device.h b/riscv/abstract_device.h
index 90e2b24..c5c6415 100644
--- a/riscv/abstract_device.h
+++ b/riscv/abstract_device.h
@@ -6,6 +6,8 @@
 #include <cstdint>
 #include <cstddef>
 #include <string>
+#include <map>
+#include <stdexcept>
 
 class sim_t;
 
@@ -26,9 +28,18 @@ public:
   virtual ~device_factory_t() {}
 };
 
+// Type for holding all registered MMIO plugins by name.
+using mmio_device_map_t = std::map<std::string, const device_factory_t*>;
+
+mmio_device_map_t& mmio_device_map();
+
 #define REGISTER_DEVICE(name, parse, generate) \
   class name##_factory_t : public device_factory_t { \
   public: \
+  name##_factory_t() { \
+    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); } \
   std::string generate_dts(const sim_t* sim) const override { return generate(sim); } \
   }; const device_factory_t *name##_factory = new name##_factory_t();
diff --git a/riscv/devices.cc b/riscv/devices.cc
index 81b232d..2c06f78 100644
--- a/riscv/devices.cc
+++ b/riscv/devices.cc
@@ -2,6 +2,12 @@
 #include "mmu.h"
 #include <stdexcept>
 
+mmio_device_map_t& mmio_device_map()
+{
+  static mmio_device_map_t device_map;
+  return device_map;
+}
+
 void bus_t::add_device(reg_t addr, abstract_device_t* dev)
 {
   // Searching devices via lower_bound/upper_bound
@@ -51,46 +57,6 @@ std::pair<reg_t, abstract_device_t*> bus_t::find_device(reg_t addr)
   return std::make_pair(it->first, it->second);
 }
 
-// Type for holding all registered MMIO plugins by name.
-using mmio_plugin_map_t = std::map<std::string, mmio_plugin_t>;
-
-// Simple singleton instance of an mmio_plugin_map_t.
-static mmio_plugin_map_t& mmio_plugin_map()
-{
-  static mmio_plugin_map_t instance;
-  return instance;
-}
-
-void register_mmio_plugin(const char* name_cstr,
-                          const mmio_plugin_t* mmio_plugin)
-{
-  std::string name(name_cstr);
-  if (!mmio_plugin_map().emplace(name, *mmio_plugin).second) {
-    throw std::runtime_error("Plugin \"" + name + "\" already registered!");
-  }
-}
-
-mmio_plugin_device_t::mmio_plugin_device_t(const std::string& name,
-                                           const std::string& args)
-  : plugin(mmio_plugin_map().at(name)), user_data((*plugin.alloc)(args.c_str()))
-{
-}
-
-mmio_plugin_device_t::~mmio_plugin_device_t()
-{
-  (*plugin.dealloc)(user_data);
-}
-
-bool mmio_plugin_device_t::load(reg_t addr, size_t len, uint8_t* bytes)
-{
-  return (*plugin.load)(user_data, addr, len, bytes);
-}
-
-bool mmio_plugin_device_t::store(reg_t addr, size_t len, const uint8_t* bytes)
-{
-  return (*plugin.store)(user_data, addr, len, bytes);
-}
-
 mem_t::mem_t(reg_t size)
   : sz(size)
 {
diff --git a/riscv/devices.h b/riscv/devices.h
index 11cc347..b752a21 100644
--- a/riscv/devices.h
+++ b/riscv/devices.h
@@ -2,7 +2,6 @@
 #define _RISCV_DEVICES_H
 
 #include "decode.h"
-#include "mmio_plugin.h"
 #include "abstract_device.h"
 #include "abstract_interrupt_controller.h"
 #include "platform.h"
@@ -157,19 +156,6 @@ class ns16550_t : public abstract_device_t {
   static const int MAX_BACKOFF = 16;
 };
 
-class mmio_plugin_device_t : public abstract_device_t {
- public:
-  mmio_plugin_device_t(const std::string& name, const std::string& args);
-  virtual ~mmio_plugin_device_t() override;
-
-  virtual bool load(reg_t addr, size_t len, uint8_t* bytes) override;
-  virtual bool store(reg_t addr, size_t len, const uint8_t* bytes) override;
-
- private:
-  mmio_plugin_t plugin;
-  void* user_data;
-};
-
 template<typename T>
 void write_little_endian_reg(T* word, reg_t addr, size_t len, const uint8_t* bytes)
 {
diff --git a/riscv/mmio_plugin.h b/riscv/mmio_plugin.h
deleted file mode 100644
index f14470b..0000000
--- a/riscv/mmio_plugin.h
+++ /dev/null
@@ -1,91 +0,0 @@
-#ifndef _RISCV_MMIO_PLUGIN_H
-#define _RISCV_MMIO_PLUGIN_H
-
-#include <stdbool.h>
-#include <stddef.h>
-#include <stdint.h>
-
-#ifdef __cplusplus
-extern "C"
-{
-#endif
-
-typedef uint64_t reg_t;
-
-typedef struct {
-  // Allocate user data for an instance of the plugin. The parameter is a simple
-  // c-string containing arguments used to construct the plugin. It returns a
-  // void* to the allocated data.
-  void* (*alloc)(const char*);
-
-  // Load a memory address of the MMIO plugin. The parameters are the user_data
-  // (void*), memory offset (reg_t), number of bytes to load (size_t), and the
-  // buffer into which the loaded data should be written (uint8_t*). Return true
-  // if the load is successful and false otherwise.
-  bool (*load)(void*, reg_t, size_t, uint8_t*);
-
-  // Store some bytes to a memory address of the MMIO plugin. The parameters are
-  // the user_data (void*), memory offset (reg_t), number of bytes to store
-  // (size_t), and the buffer containing the data to be stored (const uint8_t*).
-  // Return true if the store is successful and false otherwise.
-  bool (*store)(void*, reg_t, size_t, const uint8_t*);
-
-  // Deallocate the data allocated during the call to alloc. The parameter is a
-  // pointer to the user data allocated during the call to alloc.
-  void (*dealloc)(void*);
-} mmio_plugin_t;
-
-// Register an mmio plugin with the application. This should be called by
-// plugins as part of their loading process.
-extern void register_mmio_plugin(const char* name_cstr,
-                                 const mmio_plugin_t* mmio_plugin);
-
-#ifdef __cplusplus
-}
-
-#include <string>
-
-// Wrapper around the C plugin API that makes registering a C++ class with
-// correctly formed constructor, load, and store functions easier. The template
-// type should be the type that implements the MMIO plugin interface. Simply
-// make a global mmio_plugin_registration_t and your plugin should register
-// itself with the application when it is loaded because the
-// mmio_plugin_registration_t constructor will be called.
-template <typename T>
-struct mmio_plugin_registration_t
-{
-  static void* alloc(const char* args)
-  {
-    return reinterpret_cast<void*>(new T(std::string(args)));
-  }
-
-  static bool load(void* self, reg_t addr, size_t len, uint8_t* bytes)
-  {
-    return reinterpret_cast<T*>(self)->load(addr, len, bytes);
-  }
-
-  static bool store(void* self, reg_t addr, size_t len, const uint8_t* bytes)
-  {
-    return reinterpret_cast<T*>(self)->store(addr, len, bytes);
-  }
-
-  static void dealloc(void* self)
-  {
-    delete reinterpret_cast<T*>(self);
-  }
-
-  mmio_plugin_registration_t(const std::string& name)
-  {
-    mmio_plugin_t plugin = {
-      mmio_plugin_registration_t<T>::alloc,
-      mmio_plugin_registration_t<T>::load,
-      mmio_plugin_registration_t<T>::store,
-      mmio_plugin_registration_t<T>::dealloc,
-    };
-
-    register_mmio_plugin(name.c_str(), &plugin);
-  }
-};
-#endif // __cplusplus
-
-#endif
diff --git a/riscv/riscv.mk.in b/riscv/riscv.mk.in
index d82df45..1ad8b23 100644
--- a/riscv/riscv.mk.in
+++ b/riscv/riscv.mk.in
@@ -35,7 +35,6 @@ riscv_install_hdrs = \
 	isa_parser.h \
 	log_file.h \
 	memtracer.h \
-	mmio_plugin.h \
 	mmu.h \
 	platform.h \
 	processor.h \
diff --git a/riscv/sim.cc b/riscv/sim.cc
index 50dc4f6..0c5a7fb 100644
--- a/riscv/sim.cc
+++ b/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, mem_t*>> mems,
-             std::vector<std::pair<reg_t, std::shared_ptr<abstract_device_t>>> plugin_devices,
+             std::vector<const device_factory_t*> plugin_device_factories,
              const std::vector<std::string>& args,
              const debug_module_config_t &dm_config,
              const char *log_path,
@@ -69,11 +69,6 @@ sim_t::sim_t(const cfg_t *cfg, bool halted,
   for (auto& x : mems)
     bus.add_device(x.first, x.second);
 
-  for (auto& x : plugin_devices) {
-    bus.add_device(x.first, x.second.get());
-    devices.push_back(x.second);
-  }
-
   debug_module.add_device(&bus);
 
   socketif = NULL;
@@ -124,6 +119,9 @@ sim_t::sim_t(const cfg_t *cfg, bool halted,
     clint_factory, // clint must be element 0
     plic_factory, // plic must be element 1
     ns16550_factory};
+  device_factories.insert(device_factories.end(),
+                          plugin_device_factories.begin(),
+                          plugin_device_factories.end());
 
   // Load dtb_file if provided, otherwise self-generate a dts/dtb
   if (dtb_file) {
diff --git a/riscv/sim.h b/riscv/sim.h
index 1cb0658..a3445db 100644
--- a/riscv/sim.h
+++ b/riscv/sim.h
@@ -27,7 +27,7 @@ class sim_t : public htif_t, public simif_t
 public:
   sim_t(const cfg_t *cfg, bool halted,
         std::vector<std::pair<reg_t, mem_t*>> mems,
-        std::vector<std::pair<reg_t, std::shared_ptr<abstract_device_t>>> plugin_devices,
+        std::vector<const device_factory_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,
diff --git a/spike_main/spike.cc b/spike_main/spike.cc
index f257582..4766f6d 100644
--- a/spike_main/spike.cc
+++ b/spike_main/spike.cc
@@ -50,12 +50,7 @@ static void help(int exit_code = 1)
   fprintf(stderr, "  --l2=<S>:<W>:<B>        B both powers of 2).\n");
   fprintf(stderr, "  --big-endian          Use a big-endian memory system.\n");
   fprintf(stderr, "  --misaligned          Support misaligned memory accesses\n");
-  fprintf(stderr, "  --device=<P,B,A>      Attach MMIO plugin device from an --extlib library\n");
-  fprintf(stderr, "                          P -- Name of the MMIO plugin\n");
-  fprintf(stderr, "                          B -- Base memory address of the device\n");
-  fprintf(stderr, "                          A -- String arguments to pass to the plugin\n");
-  fprintf(stderr, "                          This flag can be used multiple times.\n");
-  fprintf(stderr, "                          The extlib flag for the library must come first.\n");
+  fprintf(stderr, "  --device=<name>       Attach MMIO plugin device from an --extlib library\n");
   fprintf(stderr, "  --log-cache-miss      Generate a log of cache miss\n");
   fprintf(stderr, "  --log-commits         Generate a log of commits info\n");
   fprintf(stderr, "  --extension=<name>    Specify RoCC Extension\n");
@@ -336,7 +331,7 @@ int main(int argc, char** argv)
   bool dtb_enabled = true;
   const char* kernel = NULL;
   reg_t kernel_offset, kernel_size;
-  std::vector<std::pair<reg_t, std::shared_ptr<abstract_device_t>>> plugin_devices;
+  std::vector<const device_factory_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;
@@ -376,47 +371,12 @@ int main(int argc, char** argv)
             /*default_real_time_clint=*/false,
             /*default_trigger_count=*/4);
 
-  auto const device_parser = [&plugin_devices](const char *s) {
-    const std::string str(s);
-    std::istringstream stream(str);
-
-    // We are parsing a string like name,base,args.
-
-    // Parse the name, which is simply all of the characters leading up to the
-    // first comma. The validity of the plugin name will be checked later.
-    std::string name;
-    std::getline(stream, name, ',');
-    if (name.empty()) {
-      throw std::runtime_error("Plugin name is empty.");
-    }
-
-    // Parse the base address. First, get all of the characters up to the next
-    // comma (or up to the end of the string if there is no comma). Then try to
-    // parse that string as an integer according to the rules of strtoull. It
-    // could be in decimal, hex, or octal. Fail if we were able to parse a
-    // number but there were garbage characters after the valid number. We must
-    // consume the entire string between the commas.
-    std::string base_str;
-    std::getline(stream, base_str, ',');
-    if (base_str.empty()) {
-      throw std::runtime_error("Device base address is empty.");
-    }
-    char* end;
-    reg_t base = static_cast<reg_t>(strtoull(base_str.c_str(), &end, 0));
-    if (end != &*base_str.cend()) {
-      throw std::runtime_error("Error parsing device base address.");
-    }
-
-    // The remainder of the string is the arguments. We could use getline, but
-    // that could ignore newline characters in the arguments. That should be
-    // rare and discouraged, but handle it here anyway with this weird in_avail
-    // technique. The arguments are optional, so if there were no arguments
-    // specified we could end up with an empty string here. That's okay.
-    auto avail = stream.rdbuf()->in_avail();
-    std::string args(avail, '\0');
-    stream.readsome(&args[0], avail);
-
-    plugin_devices.emplace_back(base, std::make_shared<mmio_plugin_device_t>(name, args));
+  auto const device_parser = [&plugin_device_factories](const char *s) {
+    const std::string name(s);
+    if (name.empty()) throw std::runtime_error("Plugin name is empty.");
+    auto it = mmio_device_map().find(name);
+    if (it == mmio_device_map().end()) throw std::runtime_error("Plugin \"" + name + "\" not found in loaded extlibs.");
+    plugin_device_factories.push_back(it->second);
   };
 
   option_parser_t parser;
@@ -564,7 +524,7 @@ int main(int argc, char** argv)
   }
 
   sim_t s(&cfg, halted,
-      mems, plugin_devices, htif_args, dm_config, log_path, dtb_enabled, dtb_file,
+      mems, plugin_device_factories, htif_args, dm_config, log_path, dtb_enabled, dtb_file,
       socket,
       cmd_file);
   std::unique_ptr<remote_bitbang_t> remote_bitbang((remote_bitbang_t *) NULL);