From e7e039dece6bfb30c9f0d85856777aa3dace4b81 Mon Sep 17 00:00:00 2001 From: LIU Yu Date: Tue, 30 Apr 2024 09:50:09 +0800 Subject: 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 `` 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 | 2 +- ci-tests/testlib.c | 2 +- riscv/abstract_device.h | 15 +++++---------- riscv/clint.cc | 4 ++-- riscv/ns16550.cc | 4 ++-- riscv/plic.cc | 4 ++-- riscv/sim.cc | 22 +++++++++++++--------- riscv/sim.h | 5 ++++- spike_main/spike.cc | 5 ++--- 9 files changed, 32 insertions(+), 31 deletions(-) diff --git a/ci-tests/test-customext.cc b/ci-tests/test-customext.cc index 5ef6c4f..e493811 100644 --- a/ci-tests/test-customext.cc +++ b/ci-tests/test-customext.cc @@ -63,7 +63,7 @@ make_mems(const std::vector &layout) { int main(int argc, char **argv) { cfg_t cfg; cfg.isa = "RV64IMAFDCV_xdummyslliuw"; - std::vector plugin_devices; + std::vector plugin_devices; if (argc != 3) { std::cerr << "Error: invalid arguments\n"; exit(1); diff --git a/ci-tests/testlib.c b/ci-tests/testlib.c index 9cdbe07..6bc0886 100644 --- a/ci-tests/testlib.c +++ b/ci-tests/testlib.c @@ -14,7 +14,7 @@ static std::vector> make_mems(const std::vecto int main(int argc, char **argv) { cfg_t cfg; - std::vector plugin_devices; + std::vector plugin_devices; if (argc != 3) { std::cerr << "Error: invalid arguments\n"; diff --git a/riscv/abstract_device.h b/riscv/abstract_device.h index d6097c1..0726cd7 100644 --- a/riscv/abstract_device.h +++ b/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 std::string generate_dts(const sim_t* sim) const = 0; + virtual abstract_device_t* parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base, const std::vector& sargs) const = 0; + virtual std::string generate_dts(const sim_t* sim, const std::vector& sargs) const = 0; virtual ~device_factory_t() {} - void set_sargs(std::vector sargs) { this->sargs = sargs; } - std::vector get_sargs() { return sargs; } - -protected: - std::vector sargs; }; // Type for holding all registered MMIO plugins by name. -using mmio_device_map_t = std::map; +using mmio_device_map_t = std::map; 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); } \ - std::string generate_dts(const sim_t* sim) const override { return generate(sim); } \ + name##_t* parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base, const std::vector& sargs) const override { return parse(fdt, sim, base, sargs); } \ + std::string generate_dts(const sim_t* sim, const std::vector& sargs) const override { return generate(sim, sargs); } \ }; device_factory_t *name##_factory = new name##_factory_t(); #endif diff --git a/riscv/clint.cc b/riscv/clint.cc index 4423467..208ea0e 100644 --- a/riscv/clint.cc +++ b/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& sargs) { + const std::vector& 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& UNUSED sargs) { std::stringstream s; s << std::hex << " clint@" << CLINT_BASE << " {\n" diff --git a/riscv/ns16550.cc b/riscv/ns16550.cc index e0b3251..2805fd8 100644 --- a/riscv/ns16550.cc +++ b/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& 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& sargs) +ns16550_t* ns16550_parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base, const std::vector& UNUSED sargs) { uint32_t ns16550_shift, ns16550_io_width, ns16550_int_id; if (fdt_parse_ns16550(fdt, base, diff --git a/riscv/plic.cc b/riscv/plic.cc index 01def70..78eb1d2 100644 --- a/riscv/plic.cc +++ b/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& 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& sargs) +plic_t* plic_parse_from_fdt(const void* fdt, const sim_t* sim, reg_t* base, const std::vector& UNUSED sargs) { uint32_t plic_ndev; if (fdt_parse_plic(fdt, base, &plic_ndev, "riscv,plic0") == 0 || diff --git a/riscv/sim.cc b/riscv/sim.cc index f4919c9..d08e274 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> mems, - std::vector plugin_device_factories, + const std::vector& plugin_device_factories, const std::vector& 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 device_factories = { - clint_factory, // clint must be element 0 - plic_factory, // plic must be element 1 - ns16550_factory}; + std::vector device_factories = { + {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()); @@ -136,8 +136,11 @@ sim_t::sim_t(const cfg_t *cfg, bool halted, } else { std::pair initrd_bounds = cfg->initrd_bounds; std::string device_nodes; - for (const device_factory_t *factory : device_factories) - device_nodes.append(factory->generate_dts(this)); + for (const device_factory_sargs_t& factory_sargs: device_factories) { + const device_factory_t* factory = factory_sargs.first; + const std::vector& 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& 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 dev_ptr(device); diff --git a/riscv/sim.h b/riscv/sim.h index efe5f83..726de7d 100644 --- a/riscv/sim.h +++ b/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>; + // 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> mems, - std::vector plugin_device_factories, + const std::vector& plugin_device_factories, const std::vector& 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 09a1fe8..6596bc1 100644 --- a/spike_main/spike.cc +++ b/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 plugin_device_factories; + std::vector plugin_device_factories; std::unique_ptr ic; std::unique_ptr dc; std::unique_ptr 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(it->second); + plugin_device_factories.push_back(std::make_pair(it->second, parsed_args)); }; option_parser_t parser; -- cgit v1.1