From ec891adca9c9430b178562ad498c1699026e5906 Mon Sep 17 00:00:00 2001 From: Giacomo Travaglini Date: Sun, 12 Dec 2021 14:59:27 +0000 Subject: [PATCH 01/18] arch-arm: Do not squash table walks if translation is partial As partial translations have been introduced we cannot just rely on checking if there is a valid translation when looking for translations to squash. The translation has to be complete as well. This is fixing realview-o3-checker regression Change-Id: I1ad42bd6172207a72f53b7a843c323c0eea88f06 Signed-off-by: Giacomo Travaglini Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54043 Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power Tested-by: kokoro --- src/arch/arm/table_walker.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/arch/arm/table_walker.cc b/src/arch/arm/table_walker.cc index f94e7652ef..c7b8c3c0c3 100644 --- a/src/arch/arm/table_walker.cc +++ b/src/arch/arm/table_walker.cc @@ -533,7 +533,8 @@ TableWalker::processWalkWrapper() unsigned num_squashed = 0; ThreadContext *tc = currState->tc; while ((num_squashed < numSquashable) && currState && - (currState->transState->squashed() || te)) { + (currState->transState->squashed() || + (te && !te->partial))) { pendingQueue.pop_front(); num_squashed++; stats.squashedBefore++; From 2a5f2ef55a5b3a2417d16c9f022eab17c92ad691 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Mon, 13 Dec 2021 21:43:07 -0800 Subject: [PATCH 02/18] scons: Make the sim_objects parameter of SimObject mandantory. If there really are no c++ sim_objects in the file, then sim_objects can be set to [] which it used to default to. This way, if someone hasn't remembered to update their SConscript files for the new sim_objects and enums parameters, this will give them some indication what's wrong, rather than the build just failing later. Change-Id: Ic1933f7b9dfff7dd7e403c6c84f1f510c8ee8c72 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54203 Maintainer: Gabe Black Tested-by: kokoro Reviewed-by: Andreas Sandberg --- src/SConscript | 12 ++++++++++-- src/cpu/o3/SConscript | 2 +- src/dev/SConscript | 2 +- src/systemc/SConscript | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/SConscript b/src/SConscript index 7eb810a9c2..e40262a75d 100644 --- a/src/SConscript +++ b/src/SConscript @@ -149,13 +149,21 @@ class SimObject(PySource): enums = dict() tags = dict() - def __init__(self, source, *, sim_objects=[], enums=[], + def __init__(self, source, *, sim_objects=None, enums=None, tags=None, add_tags=None): '''Specify the source file and any tags (automatically in the m5.objects package)''' + if sim_objects is None: + if enums is None: + error(f"SimObject({source}...) must list c++ sim_objects or " + "enums (set either to [] if there are none).") + sim_objects = [] + if enums is None: + enums = [] + super().__init__('m5.objects', source, tags, add_tags) if self.fixed: - raise AttributeError("Too late to call SimObject now.") + error("Too late to call SimObject now.") SimObject.sim_objects[self.modpath] = sim_objects SimObject.enums[self.modpath] = enums diff --git a/src/cpu/o3/SConscript b/src/cpu/o3/SConscript index a20e30d42d..ba021a892a 100755 --- a/src/cpu/o3/SConscript +++ b/src/cpu/o3/SConscript @@ -32,7 +32,7 @@ Import('*') if 'O3CPU' in env['CPU_MODELS']: SimObject('FUPool.py', sim_objects=['FUPool']) - SimObject('FuncUnitConfig.py') + SimObject('FuncUnitConfig.py', sim_objects=[]) SimObject('O3CPU.py', sim_objects=['O3CPU'], enums=[ 'SMTFetchPolicy', 'SMTQueuePolicy', 'CommitPolicy']) diff --git a/src/dev/SConscript b/src/dev/SConscript index db3163e76f..755ddb541a 100644 --- a/src/dev/SConscript +++ b/src/dev/SConscript @@ -35,7 +35,7 @@ Source('isa_fake.cc') Source('dma_device.cc') Source('dma_virt_device.cc') -SimObject('IntPin.py') +SimObject('IntPin.py', sim_objects=[]) Source('intpin.cc') DebugFlag('IsaFake') diff --git a/src/systemc/SConscript b/src/systemc/SConscript index ef7e79b69d..57cb1d947d 100644 --- a/src/systemc/SConscript +++ b/src/systemc/SConscript @@ -32,4 +32,4 @@ env.UseSystemcCheck(warn=True) env.Append(CPPPATH=Dir('ext')) -SimObject('Tlm.py') +SimObject('Tlm.py', sim_objects=[]) From 4f987ff52d5c3e35b4fd9d9707ca80653154b2df Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Sat, 11 Dec 2021 05:44:56 -0800 Subject: [PATCH 03/18] python: Add a mechanism for installing pybind modules. pybind provides a mechanism for this already, but it assumes that because it works through static initializers, it must run before the python interpreter has started. That is not true when gem5 is built as a library, since its static intitializers won't run until that library is loaded. Change-Id: I6f36c5f3831dda893039b4923902e9ce46a91808 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54003 Maintainer: Bobby Bruce Tested-by: kokoro Reviewed-by: Bobby Bruce Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54263 --- src/python/pybind_init.hh | 74 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 src/python/pybind_init.hh diff --git a/src/python/pybind_init.hh b/src/python/pybind_init.hh new file mode 100644 index 0000000000..102f3f18f7 --- /dev/null +++ b/src/python/pybind_init.hh @@ -0,0 +1,74 @@ +/* + * Copyright 2021 Google, Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer; + * redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution; + * neither the name of the copyright holders nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef __PYTHON_PYBIND_INIT_HH__ +#define __PYTHON_PYBIND_INIT_HH__ + +#include + +#include "pybind11/pybind11.h" + +namespace gem5 +{ + +struct PybindModuleInit +{ + PybindModuleInit(const char *name, PyObject *(*func)()) + { + if (Py_IsInitialized()) { + // If python is running, initialize and inject the module. + PyImport_AddModule(name); + PyObject *sys_modules = PyImport_GetModuleDict(); + PyDict_SetItemString(sys_modules, name, func()); + } else { + // If not, tell python how to build importer itself later. + PyImport_AppendInittab(name, func); + } + } +}; + +} // namespace gem5 + +#define GEM5_PYBIND_MODULE_INIT(name, func) \ +namespace { \ + \ +::PyObject * \ +initializer() \ +{ \ + static ::pybind11::module_::module_def mod_def; \ + static auto m = ::pybind11::module_::create_extension_module( \ + #name, nullptr, &mod_def); \ + func(m); \ + m.inc_ref(); \ + return m.ptr(); \ +} \ + \ +::gem5::PybindModuleInit modInit(#name, initializer); \ + \ +} // anonymous namespace + +#endif // __PYTHON_PYBIND_INIT_HH__ From ebbfe1d2812486d743f1cc5a4b8d823d63f7a9b2 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Sat, 11 Dec 2021 05:51:11 -0800 Subject: [PATCH 04/18] python: Replace PYBIND11_EMBEDDED_MODULE with GEM5_PYBIND_MODULE_INIT. That will make it possible for gem5's static intializers to run even if the python interpreter has started. Change-Id: Ic3574c32244e5ac475222f6d305ddc70dd6298d6 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54004 Maintainer: Bobby Bruce Tested-by: kokoro Reviewed-by: Bobby Bruce Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54264 --- src/python/importer.cc | 15 ++++++++++++--- src/sim/init.cc | 6 ++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/python/importer.cc b/src/python/importer.cc index 2d10e962b4..dde86a8896 100644 --- a/src/python/importer.cc +++ b/src/python/importer.cc @@ -25,15 +25,20 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "pybind11/embed.h" +#include "pybind11/eval.h" #include "pybind11/pybind11.h" -#include "python/m5ImporterCode.hh" #include "python/embedded.hh" +#include "python/m5ImporterCode.hh" +#include "python/pybind_init.hh" namespace py = pybind11; -PYBIND11_EMBEDDED_MODULE(importer, m) +namespace +{ + +void +importerInit(py::module_ &m) { m.def("_init_all_embedded", gem5::EmbeddedPython::initAll); py::str importer_code( @@ -41,3 +46,7 @@ PYBIND11_EMBEDDED_MODULE(importer, m) gem5::Blobs::m5ImporterCode_len); py::exec(std::move(importer_code), m.attr("__dict__")); } + +GEM5_PYBIND_MODULE_INIT(importer, importerInit) + +} // anonymous namespace diff --git a/src/sim/init.cc b/src/sim/init.cc index 8c0d1aa271..d594b08933 100644 --- a/src/sim/init.cc +++ b/src/sim/init.cc @@ -48,6 +48,7 @@ #include "base/cprintf.hh" #include "python/pybind11/pybind.hh" +#include "python/pybind_init.hh" namespace py = pybind11; @@ -126,9 +127,6 @@ EmbeddedPyBind::initAll(py::module_ &_m5) } } -PYBIND11_EMBEDDED_MODULE(_m5, _m5) -{ - EmbeddedPyBind::initAll(_m5); -} +GEM5_PYBIND_MODULE_INIT(_m5, EmbeddedPyBind::initAll) } // namespace gem5 From 7f4a485fcfede99527e31e321f8cbf901252a14d Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Sat, 11 Dec 2021 07:04:05 -0800 Subject: [PATCH 05/18] sim: Make the EmbeddedPyBind::initAll method work correctly. This method depended on all of the EmbeddedPyBind objects having all been constructed already so that it would have a complete list. This would only be true if it was called after static intialization was complete, which is not true if python is ready to go as soon as gem5 (in library form) is loaded. This change makes EmbeddedPyBind able to defer initialization of a module more generically than before, so that they can wait for either another module to be initiailized, or the _m5 package itself. Change-Id: I6b636524f969bd9882d8c1a7594dc36eb4e78037 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54005 Maintainer: Bobby Bruce Tested-by: kokoro Reviewed-by: Bobby Bruce Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54265 --- src/sim/init.cc | 97 ++++++++++++++++++++++++++++--------------------- src/sim/init.hh | 17 +++++++-- 2 files changed, 68 insertions(+), 46 deletions(-) diff --git a/src/sim/init.cc b/src/sim/init.cc index d594b08933..1c3d3aa958 100644 --- a/src/sim/init.cc +++ b/src/sim/init.cc @@ -55,76 +55,89 @@ namespace py = pybind11; namespace gem5 { +pybind11::module_ *EmbeddedPyBind::mod = nullptr; + EmbeddedPyBind::EmbeddedPyBind(const char *_name, void (*init_func)(py::module_ &), - const char *_base) - : initFunc(init_func), registered(false), name(_name), base(_base) + const char *_base) : + initFunc(init_func), name(_name), base(_base) { - getMap()[_name] = this; + init(); } EmbeddedPyBind::EmbeddedPyBind(const char *_name, - void (*init_func)(py::module_ &)) - : initFunc(init_func), registered(false), name(_name), base("") + void (*init_func)(py::module_ &)) : + EmbeddedPyBind(_name, init_func, "") +{} + +void +EmbeddedPyBind::init() { - getMap()[_name] = this; + // If this module is already registered, complain and stop. + if (registered) { + cprintf("Warning: %s already registered.\n", name); + return; + } + + auto &ready = getReady(); + auto &pending = getPending(); + + // If we're not ready for this module yet, defer intialization. + if (!mod || (!base.empty() && ready.find(base) == ready.end())) { + pending.insert({std::string(base), this}); + return; + } + + // We must be ready, so set this module up. + initFunc(*mod); + ready[name] = this; + registered = true; + + // Find any other modules that were waiting for this one and init them. + initPending(name); } void -EmbeddedPyBind::init(py::module_ &m) +EmbeddedPyBind::initPending(const std::string &finished) { - if (!registered) { - initFunc(m); - registered = true; - } else { - cprintf("Warning: %s already registered.\n", name); - } -} + auto &pending = getPending(); -bool -EmbeddedPyBind::depsReady() const -{ - return base.empty() || getMap()[base]->registered; + auto range = pending.equal_range(finished); + std::list> todo( + range.first, range.second); + pending.erase(range.first, range.second); + + for (auto &entry: todo) + entry.second->init(); } std::map & -EmbeddedPyBind::getMap() +EmbeddedPyBind::getReady() { - static std::map objs; - return objs; + static std::map ready; + return ready; +} + +std::multimap & +EmbeddedPyBind::getPending() +{ + static std::multimap pending; + return pending; } void EmbeddedPyBind::initAll(py::module_ &_m5) { - std::list pending; - pybind_init_core(_m5); pybind_init_debug(_m5); pybind_init_event(_m5); pybind_init_stats(_m5); - for (auto &kv : EmbeddedPyBind::getMap()) { - auto &obj = kv.second; - if (obj->base.empty()) { - obj->init(_m5); - } else { - pending.push_back(obj); - } - } + mod = &_m5; - while (!pending.empty()) { - for (auto it = pending.begin(); it != pending.end(); ) { - EmbeddedPyBind &obj = **it; - if (obj.depsReady()) { - obj.init(_m5); - it = pending.erase(it); - } else { - ++it; - } - } - } + // Init all the modules that were waiting on the _m5 module itself. + initPending(""); } GEM5_PYBIND_MODULE_INIT(_m5, EmbeddedPyBind::initAll) diff --git a/src/sim/init.hh b/src/sim/init.hh index 9e7158a63e..3b86f82b29 100644 --- a/src/sim/init.hh +++ b/src/sim/init.hh @@ -43,6 +43,7 @@ #include "pybind11/pybind11.h" +#include #include #include @@ -64,14 +65,22 @@ class EmbeddedPyBind private: void (*initFunc)(pybind11::module_ &); - bool depsReady() const; - void init(pybind11::module_ &m); + void init(); - bool registered; + bool registered = false; const std::string name; const std::string base; - static std::map &getMap(); + // The _m5 module. + static pybind11::module_ *mod; + + // A map from initialized module names to their descriptors. + static std::map &getReady(); + // A map to pending modules from the name of what they're waiting on. + static std::multimap &getPending(); + + // Initialize all modules waiting on "finished". + static void initPending(const std::string &finished); }; } // namespace gem5 From b39c106eea6ecd768fe120820f1eedd2e73128a9 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Sat, 11 Dec 2021 08:24:03 -0800 Subject: [PATCH 06/18] ext: Fix compilation of the sst gem5 integration. Replace the old copied version of gem5's main function with an updated copy. This fixes compilation, but there's still a problem running an sst example where there's a segfault inside the python interpreter. Change-Id: I95714a9264636c14e1dda3174bc0d79e3d881727 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54006 Maintainer: Bobby Bruce Tested-by: kokoro Reviewed-by: Bobby Bruce Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54266 --- ext/sst/gem5.cc | 103 +++++++++--------------------------------------- ext/sst/gem5.hh | 2 - 2 files changed, 18 insertions(+), 87 deletions(-) diff --git a/ext/sst/gem5.cc b/ext/sst/gem5.cc index c93c722cf3..1d90bc5b38 100644 --- a/ext/sst/gem5.cc +++ b/ext/sst/gem5.cc @@ -89,6 +89,9 @@ #include #include +#include +#include + // gem5 Headers #include #include @@ -118,6 +121,8 @@ // More SST Headers #include +namespace py = pybind11; + gem5Component::gem5Component(SST::ComponentId_t id, SST::Params& params): SST::Component(id), threadInitialized(false) { @@ -365,98 +370,26 @@ gem5Component::execPythonCommands(const std::vector& commands) return 0; } -int -gem5Component::startM5(int argc, char **_argv) -{ - // This function should be similar to m5Main() of src/sim/init.cc - -#if HAVE_PROTOBUF - // Verify that the version of the protobuf library that we linked - // against is compatible with the version of the headers we - // compiled against. - GOOGLE_PROTOBUF_VERIFY_VERSION; -#endif - - -#if PY_MAJOR_VERSION >= 3 - typedef std::unique_ptr WArgUPtr; - std::vector v_argv; - std::vector vp_argv; - v_argv.reserve(argc); - vp_argv.reserve(argc); - for (int i = 0; i < argc; i++) { - v_argv.emplace_back(Py_DecodeLocale(_argv[i], NULL), &PyMem_RawFree); - vp_argv.emplace_back(v_argv.back().get()); - } - - wchar_t **argv = vp_argv.data(); -#else - char **argv = _argv; -#endif - - PySys_SetArgv(argc, argv); - - // We have to set things up in the special __main__ module - pythonMain = PyImport_AddModule(PyCC("__main__")); - if (pythonMain == NULL) - panic("Could not import __main__"); - - const std::vector commands = { - "import m5", - "m5.main()" - }; - execPythonCommands(commands); - -#if HAVE_PROTOBUF - google::protobuf::ShutdownProtobufLibrary(); -#endif - - return 0; -} - void gem5Component::initPython(int argc, char *_argv[]) { - // should be similar to main() in src/sim/main.cc - PyObject *mainModule, *mainDict; - - int ret; - - // Initialize m5 special signal handling. + // Initialize gem5 special signal handling. gem5::initSignals(); -#if PY_MAJOR_VERSION >= 3 - std::unique_ptr program( - Py_DecodeLocale(_argv[0], NULL), - &PyMem_RawFree); - Py_SetProgramName(program.get()); -#else - Py_SetProgramName(_argv[0]); -#endif + if (!Py_IsInitialized()) + py::initialize_interpreter(false, argc, _argv); - // Register native modules with Python's init system before - // initializing the interpreter. - if (!Py_IsInitialized()) { - gem5::registerNativeModules(); - // initialize embedded Python interpreter - Py_Initialize(); - } else { - // https://stackoverflow.com/a/28349174 - PyImport_AddModule("_m5"); - PyObject* module = gem5::EmbeddedPyBind::initAll(); - PyObject* sys_modules = PyImport_GetModuleDict(); - PyDict_SetItemString(sys_modules, "_m5", module); - Py_DECREF(module); + auto importer = py::module_::import("importer"); + importer.attr("install")(); + + try { + py::module_::import("m5").attr("main")(); + } catch (py::error_already_set &e) { + if (!e.matches(PyExc_SystemExit)) { + std::cerr << e.what(); + output.output(CALL_INFO, "Calling m5.main(...) failed.\n"); + } } - - - // Initialize the embedded m5 python library - ret = gem5::EmbeddedPython::initAll(); - - if (ret == 0) - startM5(argc, _argv); // start m5 - else - output.output(CALL_INFO, "Not calling m5Main due to ret=%d\n", ret); } void diff --git a/ext/sst/gem5.hh b/ext/sst/gem5.hh index 34d8bd616c..27124114b1 100644 --- a/ext/sst/gem5.hh +++ b/ext/sst/gem5.hh @@ -101,8 +101,6 @@ class gem5Component: public SST::Component void finish(); bool clockTick(SST::Cycle_t current_cycle); - int startM5(int argc, char **_argv); - // stuff needed for gem5 sim public: From d1d90c529c9d7b559b4ba5d3c4c0b5efd56c34c6 Mon Sep 17 00:00:00 2001 From: Giacomo Travaglini Date: Tue, 14 Dec 2021 15:13:51 +0000 Subject: [PATCH 07/18] configs: Stop using a PTW cache before L2 in Arm configs This implementation of a walk cache does not allow to skip walks as it is a simple cache placed in front of the table walker. It was meant to provide a faster retrieval of page table descriptors than fetching them from L2 or memory. This is not needed anymore for Arm as from [1] we implement partial translation caching in Arm TLBs. [1]: JIRA: https://gem5.atlassian.net/browse/GEM5-1108 Change-Id: I00d44a4e3961e15602bf4352f2f42ddccf2b746b Signed-off-by: Giacomo Travaglini Reviewed-by: Richard Cooper Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54243 Reviewed-by: Andreas Sandberg Maintainer: Jason Lowe-Power Tested-by: kokoro --- configs/common/CacheConfig.py | 4 ++-- configs/example/arm/baremetal.py | 4 +--- configs/example/arm/devices.py | 7 ++----- configs/example/arm/fs_bigLITTLE.py | 8 ++++---- configs/example/arm/ruby_fs.py | 4 +--- configs/example/arm/starter_fs.py | 4 +--- configs/example/arm/starter_se.py | 4 +--- 7 files changed, 12 insertions(+), 23 deletions(-) diff --git a/configs/common/CacheConfig.py b/configs/common/CacheConfig.py index 4979f7d86f..61c6a304d7 100644 --- a/configs/common/CacheConfig.py +++ b/configs/common/CacheConfig.py @@ -87,7 +87,7 @@ def config_cache(options, system): dcache_class, icache_class, l2_cache_class, walk_cache_class = \ core.O3_ARM_v7a_DCache, core.O3_ARM_v7a_ICache, \ core.O3_ARM_v7aL2, \ - core.O3_ARM_v7aWalkCache + None elif options.cpu_type == "HPI": try: import cores.arm.HPI as core @@ -96,7 +96,7 @@ def config_cache(options, system): sys.exit(1) dcache_class, icache_class, l2_cache_class, walk_cache_class = \ - core.HPI_DCache, core.HPI_ICache, core.HPI_L2, core.HPI_WalkCache + core.HPI_DCache, core.HPI_ICache, core.HPI_L2, None else: dcache_class, icache_class, l2_cache_class, walk_cache_class = \ L1_DCache, L1_ICache, L2Cache, None diff --git a/configs/example/arm/baremetal.py b/configs/example/arm/baremetal.py index 9655bb165a..094434495b 100644 --- a/configs/example/arm/baremetal.py +++ b/configs/example/arm/baremetal.py @@ -61,14 +61,12 @@ import workloads # the cache class may be 'None' if the particular cache is not present. cpu_types = { - "atomic" : ( AtomicSimpleCPU, None, None, None, None), + "atomic" : ( AtomicSimpleCPU, None, None, None), "minor" : (MinorCPU, devices.L1I, devices.L1D, - devices.WalkCache, devices.L2), "hpi" : ( HPI.HPI, HPI.HPI_ICache, HPI.HPI_DCache, - HPI.HPI_WalkCache, HPI.HPI_L2) } diff --git a/configs/example/arm/devices.py b/configs/example/arm/devices.py index 73aea59a1c..5217b08729 100644 --- a/configs/example/arm/devices.py +++ b/configs/example/arm/devices.py @@ -106,12 +106,11 @@ class MemBus(SystemXBar): class CpuCluster(SubSystem): def __init__(self, system, num_cpus, cpu_clock, cpu_voltage, - cpu_type, l1i_type, l1d_type, wcache_type, l2_type): + cpu_type, l1i_type, l1d_type, l2_type): super(CpuCluster, self).__init__() self._cpu_type = cpu_type self._l1i_type = l1i_type self._l1d_type = l1d_type - self._wcache_type = wcache_type self._l2_type = l2_type assert num_cpus > 0 @@ -140,9 +139,7 @@ class CpuCluster(SubSystem): for cpu in self.cpus: l1i = None if self._l1i_type is None else self._l1i_type() l1d = None if self._l1d_type is None else self._l1d_type() - iwc = None if self._wcache_type is None else self._wcache_type() - dwc = None if self._wcache_type is None else self._wcache_type() - cpu.addPrivateSplitL1Caches(l1i, l1d, iwc, dwc) + cpu.addPrivateSplitL1Caches(l1i, l1d) def addL2(self, clk_domain): if self._l2_type is None: diff --git a/configs/example/arm/fs_bigLITTLE.py b/configs/example/arm/fs_bigLITTLE.py index c590fe510f..3f8b0cfb19 100644 --- a/configs/example/arm/fs_bigLITTLE.py +++ b/configs/example/arm/fs_bigLITTLE.py @@ -79,7 +79,7 @@ class BigCluster(devices.CpuCluster): def __init__(self, system, num_cpus, cpu_clock, cpu_voltage="1.0V"): cpu_config = [ ObjectList.cpu_list.get("O3_ARM_v7a_3"), - devices.L1I, devices.L1D, devices.WalkCache, devices.L2 ] + devices.L1I, devices.L1D, devices.L2 ] super(BigCluster, self).__init__(system, num_cpus, cpu_clock, cpu_voltage, *cpu_config) @@ -87,7 +87,7 @@ class LittleCluster(devices.CpuCluster): def __init__(self, system, num_cpus, cpu_clock, cpu_voltage="1.0V"): cpu_config = [ ObjectList.cpu_list.get("MinorCPU"), devices.L1I, - devices.L1D, devices.WalkCache, devices.L2 ] + devices.L1D, devices.L2 ] super(LittleCluster, self).__init__(system, num_cpus, cpu_clock, cpu_voltage, *cpu_config) @@ -95,7 +95,7 @@ class Ex5BigCluster(devices.CpuCluster): def __init__(self, system, num_cpus, cpu_clock, cpu_voltage="1.0V"): cpu_config = [ ObjectList.cpu_list.get("ex5_big"), ex5_big.L1I, - ex5_big.L1D, ex5_big.WalkCache, ex5_big.L2 ] + ex5_big.L1D, ex5_big.L2 ] super(Ex5BigCluster, self).__init__(system, num_cpus, cpu_clock, cpu_voltage, *cpu_config) @@ -103,7 +103,7 @@ class Ex5LittleCluster(devices.CpuCluster): def __init__(self, system, num_cpus, cpu_clock, cpu_voltage="1.0V"): cpu_config = [ ObjectList.cpu_list.get("ex5_LITTLE"), - ex5_LITTLE.L1I, ex5_LITTLE.L1D, ex5_LITTLE.WalkCache, + ex5_LITTLE.L1I, ex5_LITTLE.L1D, ex5_LITTLE.L2 ] super(Ex5LittleCluster, self).__init__(system, num_cpus, cpu_clock, cpu_voltage, *cpu_config) diff --git a/configs/example/arm/ruby_fs.py b/configs/example/arm/ruby_fs.py index 3783f3389e..d820f86316 100644 --- a/configs/example/arm/ruby_fs.py +++ b/configs/example/arm/ruby_fs.py @@ -62,14 +62,12 @@ default_root_device = '/dev/vda1' # the cache class may be 'None' if the particular cache is not present. cpu_types = { - "noncaching" : ( NonCachingSimpleCPU, None, None, None, None), + "noncaching" : ( NonCachingSimpleCPU, None, None, None), "minor" : (MinorCPU, devices.L1I, devices.L1D, - devices.WalkCache, devices.L2), "hpi" : ( HPI.HPI, HPI.HPI_ICache, HPI.HPI_DCache, - HPI.HPI_WalkCache, HPI.HPI_L2) } diff --git a/configs/example/arm/starter_fs.py b/configs/example/arm/starter_fs.py index 11190dbd6d..40e645b564 100644 --- a/configs/example/arm/starter_fs.py +++ b/configs/example/arm/starter_fs.py @@ -65,14 +65,12 @@ default_root_device = '/dev/vda1' # the cache class may be 'None' if the particular cache is not present. cpu_types = { - "atomic" : ( AtomicSimpleCPU, None, None, None, None), + "atomic" : ( AtomicSimpleCPU, None, None, None), "minor" : (MinorCPU, devices.L1I, devices.L1D, - devices.WalkCache, devices.L2), "hpi" : ( HPI.HPI, HPI.HPI_ICache, HPI.HPI_DCache, - HPI.HPI_WalkCache, HPI.HPI_L2) } diff --git a/configs/example/arm/starter_se.py b/configs/example/arm/starter_se.py index 82fc49e5c6..d80f7498d6 100644 --- a/configs/example/arm/starter_se.py +++ b/configs/example/arm/starter_se.py @@ -59,14 +59,12 @@ import devices # l1_icache_class, l1_dcache_class, walk_cache_class, l2_Cache_class). Any of # the cache class may be 'None' if the particular cache is not present. cpu_types = { - "atomic" : ( AtomicSimpleCPU, None, None, None, None), + "atomic" : ( AtomicSimpleCPU, None, None, None), "minor" : (MinorCPU, devices.L1I, devices.L1D, - devices.WalkCache, devices.L2), "hpi" : ( HPI.HPI, HPI.HPI_ICache, HPI.HPI_DCache, - HPI.HPI_WalkCache, HPI.HPI_L2) } From 3fba052f3f7a2e45e52ddd361b1d0de5fd9057e3 Mon Sep 17 00:00:00 2001 From: Giacomo Travaglini Date: Wed, 15 Dec 2021 08:51:53 +0000 Subject: [PATCH 08/18] configs: Remove unused WalkCache models Change-Id: Iebda966e72b484ee15cbc7cd62256a950b2a90f1 Signed-off-by: Giacomo Travaglini Reviewed-by: Richard Cooper Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54244 Reviewed-by: Andreas Sandberg Maintainer: Jason Lowe-Power Tested-by: kokoro --- configs/common/cores/arm/HPI.py | 12 +----------- configs/common/cores/arm/O3_ARM_v7a.py | 15 --------------- configs/common/cores/arm/ex5_LITTLE.py | 15 --------------- configs/common/cores/arm/ex5_big.py | 15 --------------- configs/example/arm/devices.py | 11 ----------- 5 files changed, 1 insertion(+), 67 deletions(-) diff --git a/configs/common/cores/arm/HPI.py b/configs/common/cores/arm/HPI.py index 624c40c57a..620c01ebd5 100644 --- a/configs/common/cores/arm/HPI.py +++ b/configs/common/cores/arm/HPI.py @@ -1332,16 +1332,6 @@ class HPI_MMU(ArmMMU): itb = ArmTLB(entry_type="instruction", size=256) dtb = ArmTLB(entry_type="data", size=256) -class HPI_WalkCache(Cache): - data_latency = 4 - tag_latency = 4 - response_latency = 4 - mshrs = 6 - tgts_per_mshr = 8 - size = '1kB' - assoc = 8 - write_buffers = 16 - class HPI_BP(TournamentBP): localPredictorSize = 64 localCtrBits = 2 @@ -1442,7 +1432,7 @@ class HPI(MinorCPU): __all__ = [ "HPI_BP", - "HPI_ITB", "HPI_DTB", "HPI_WalkCache", + "HPI_ITB", "HPI_DTB", "HPI_ICache", "HPI_DCache", "HPI_L2", "HPI", ] diff --git a/configs/common/cores/arm/O3_ARM_v7a.py b/configs/common/cores/arm/O3_ARM_v7a.py index a402e5fa0a..8cacc65ebb 100644 --- a/configs/common/cores/arm/O3_ARM_v7a.py +++ b/configs/common/cores/arm/O3_ARM_v7a.py @@ -169,21 +169,6 @@ class O3_ARM_v7a_DCache(Cache): # Consider the L2 a victim cache also for clean lines writeback_clean = True -# TLB Cache -# Use a cache as a L2 TLB -class O3_ARM_v7aWalkCache(Cache): - tag_latency = 4 - data_latency = 4 - response_latency = 4 - mshrs = 6 - tgts_per_mshr = 8 - size = '1kB' - assoc = 8 - write_buffers = 16 - is_read_only = True - # Writeback clean lines as well - writeback_clean = True - # L2 Cache class O3_ARM_v7aL2(Cache): tag_latency = 12 diff --git a/configs/common/cores/arm/ex5_LITTLE.py b/configs/common/cores/arm/ex5_LITTLE.py index b3f1ad52ae..bcbaa922cd 100644 --- a/configs/common/cores/arm/ex5_LITTLE.py +++ b/configs/common/cores/arm/ex5_LITTLE.py @@ -112,21 +112,6 @@ class L1D(L1Cache): assoc = 4 write_buffers = 4 -# TLB Cache -# Use a cache as a L2 TLB -class WalkCache(Cache): - tag_latency = 2 - data_latency = 2 - response_latency = 2 - mshrs = 6 - tgts_per_mshr = 8 - size = '1kB' - assoc = 2 - write_buffers = 16 - is_read_only = True - # Writeback clean lines as well - writeback_clean = True - # L2 Cache class L2(Cache): tag_latency = 9 diff --git a/configs/common/cores/arm/ex5_big.py b/configs/common/cores/arm/ex5_big.py index c734c629e2..eb5f53ff0c 100644 --- a/configs/common/cores/arm/ex5_big.py +++ b/configs/common/cores/arm/ex5_big.py @@ -164,21 +164,6 @@ class L1D(L1Cache): assoc = 2 write_buffers = 16 -# TLB Cache -# Use a cache as a L2 TLB -class WalkCache(Cache): - tag_latency = 4 - data_latency = 4 - response_latency = 4 - mshrs = 6 - tgts_per_mshr = 8 - size = '1kB' - assoc = 8 - write_buffers = 16 - is_read_only = True - # Writeback clean lines as well - writeback_clean = True - # L2 Cache class L2(Cache): tag_latency = 15 diff --git a/configs/example/arm/devices.py b/configs/example/arm/devices.py index 5217b08729..9122e7ce3e 100644 --- a/configs/example/arm/devices.py +++ b/configs/example/arm/devices.py @@ -65,17 +65,6 @@ class L1D(L1_DCache): write_buffers = 16 -class WalkCache(PageTableWalkerCache): - tag_latency = 4 - data_latency = 4 - response_latency = 4 - mshrs = 6 - tgts_per_mshr = 8 - size = '1kB' - assoc = 8 - write_buffers = 16 - - class L2(L2Cache): tag_latency = 12 data_latency = 12 From c154888b2a6f328f51f10a2f8873109106361fdd Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Sat, 11 Dec 2021 21:13:04 -0800 Subject: [PATCH 09/18] arch,sim-se: Handle syscall retry/suppression in the syscall desc. Rather than make each ISA include boilerplate to ignore a SyscallReturn's value when it's marked as suppressed or needing a retry, put that code into the SyscallDesc::doSyscall method instead. That has two benefits. First, it removes a decent amount of code duplication which is nice from a maintenance perspective. Second, it puts the SyscallDesc in charge of figuring out what to do once a system call implementation finishes. That will let it schedule a retry of the system call for instance, without worrying about what the ISA is doing with the SyscallReturn behind its back. Jira Issue: https://gem5.atlassian.net/browse/GEM5-1123 Change-Id: I76760cba75fd23e6e3357f6169c0140bee3f01b6 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54204 Reviewed-by: Giacomo Travaglini Maintainer: Giacomo Travaglini Tested-by: kokoro --- src/arch/arm/freebsd/se_workload.hh | 3 --- src/arch/arm/linux/se_workload.hh | 3 --- src/arch/mips/se_workload.hh | 3 --- src/arch/power/se_workload.hh | 3 --- src/arch/riscv/se_workload.hh | 3 --- src/arch/sparc/se_workload.hh | 3 --- src/arch/x86/linux/linux.hh | 3 --- src/sim/syscall_desc.cc | 8 +++++--- src/sim/syscall_desc.hh | 2 +- 9 files changed, 6 insertions(+), 25 deletions(-) diff --git a/src/arch/arm/freebsd/se_workload.hh b/src/arch/arm/freebsd/se_workload.hh index 8069bd2563..4ec50907dc 100644 --- a/src/arch/arm/freebsd/se_workload.hh +++ b/src/arch/arm/freebsd/se_workload.hh @@ -82,9 +82,6 @@ struct ResultsetCCReg(ArmISA::CCREG_C, 0); diff --git a/src/arch/arm/linux/se_workload.hh b/src/arch/arm/linux/se_workload.hh index b22688fb8e..f0af647251 100644 --- a/src/arch/arm/linux/se_workload.hh +++ b/src/arch/arm/linux/se_workload.hh @@ -74,9 +74,6 @@ struct ResultsetIntReg(ArmISA::ReturnValueReg, ret.encodedValue()); if (ret.count() > 1) tc->setIntReg(ArmISA::SyscallPseudoReturnReg, ret.value2()); diff --git a/src/arch/mips/se_workload.hh b/src/arch/mips/se_workload.hh index 178093d837..c10ceb031a 100644 --- a/src/arch/mips/se_workload.hh +++ b/src/arch/mips/se_workload.hh @@ -77,9 +77,6 @@ struct Result static void store(ThreadContext *tc, const SyscallReturn &ret) { - if (ret.suppressed() || ret.needsRetry()) - return; - if (ret.successful()) { // no error tc->setIntReg(MipsISA::SyscallSuccessReg, 0); diff --git a/src/arch/power/se_workload.hh b/src/arch/power/se_workload.hh index 2b6279529d..c351d6560d 100644 --- a/src/arch/power/se_workload.hh +++ b/src/arch/power/se_workload.hh @@ -77,9 +77,6 @@ struct Result static void store(ThreadContext *tc, const SyscallReturn &ret) { - if (ret.suppressed() || ret.needsRetry()) - return; - PowerISA::Cr cr = tc->readIntReg(PowerISA::INTREG_CR); if (ret.successful()) { cr.cr0.so = 0; diff --git a/src/arch/riscv/se_workload.hh b/src/arch/riscv/se_workload.hh index 1f8a0677b8..484803e3c7 100644 --- a/src/arch/riscv/se_workload.hh +++ b/src/arch/riscv/se_workload.hh @@ -75,9 +75,6 @@ struct Result static void store(ThreadContext *tc, const SyscallReturn &ret) { - if (ret.suppressed() || ret.needsRetry()) - return; - if (ret.successful()) { // no error tc->setIntReg(RiscvISA::ReturnValueReg, ret.returnValue()); diff --git a/src/arch/sparc/se_workload.hh b/src/arch/sparc/se_workload.hh index 6d034f7ced..18988fe66d 100644 --- a/src/arch/sparc/se_workload.hh +++ b/src/arch/sparc/se_workload.hh @@ -89,9 +89,6 @@ struct ResultsetIntReg(X86ISA::INTREG_RAX, ret.encodedValue()); } }; diff --git a/src/sim/syscall_desc.cc b/src/sim/syscall_desc.cc index 74991020b7..7427f41990 100644 --- a/src/sim/syscall_desc.cc +++ b/src/sim/syscall_desc.cc @@ -44,12 +44,14 @@ SyscallDesc::doSyscall(ThreadContext *tc) SyscallReturn retval = executor(this, tc); - if (retval.needsRetry()) + if (retval.needsRetry()) { DPRINTF_SYSCALL(Base, "Needs retry.\n", name()); - else if (retval.suppressed()) + } else if (retval.suppressed()) { DPRINTF_SYSCALL(Base, "No return value.\n", name()); - else + } else { + returnInto(tc, retval); DPRINTF_SYSCALL(Base, "Returned %d.\n", retval.encodedValue()); + } } } // namespace gem5 diff --git a/src/sim/syscall_desc.hh b/src/sim/syscall_desc.hh index 0740632705..6ded904097 100644 --- a/src/sim/syscall_desc.hh +++ b/src/sim/syscall_desc.hh @@ -141,7 +141,7 @@ class SyscallDescABI : public SyscallDesc // Use invokeSimcall to gather the other arguments based on the // given ABI and pass them to the syscall implementation. - return invokeSimcall(tc, + return invokeSimcall(tc, std::function( partial)); }; From 7f8bbe01be12ba57604af080f7b855934dd67171 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Sat, 11 Dec 2021 21:49:09 -0800 Subject: [PATCH 10/18] sim-se: (Re)add support for retrying system calls. The previous incarnation of this support used faults to make the CPU reexecute the system call instruction again and again to prevent emulating/passing through blocking system calls from blocking gem5 as a whole. That support was accidentally removed a while ago. This new version suspends the thread context executing the system call, and periodically wakes it up to retry using a periodically scheduled event. Jira Issue: https://gem5.atlassian.net/browse/GEM5-1123 Change-Id: I155fa8205d7ea45e3d102216aeca6ee1979a522f Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54205 Reviewed-by: Giacomo Travaglini Maintainer: Giacomo Travaglini Tested-by: kokoro --- src/sim/syscall_desc.cc | 55 ++++++++++++++++++++++++++++++++++++++--- src/sim/syscall_desc.hh | 5 ++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/src/sim/syscall_desc.cc b/src/sim/syscall_desc.cc index 7427f41990..d293167995 100644 --- a/src/sim/syscall_desc.cc +++ b/src/sim/syscall_desc.cc @@ -30,6 +30,7 @@ #include "sim/syscall_desc.hh" #include "base/types.hh" +#include "sim/eventq.hh" #include "sim/syscall_debug_macros.hh" namespace gem5 @@ -45,12 +46,58 @@ SyscallDesc::doSyscall(ThreadContext *tc) SyscallReturn retval = executor(this, tc); if (retval.needsRetry()) { - DPRINTF_SYSCALL(Base, "Needs retry.\n", name()); - } else if (retval.suppressed()) { + // Suspend this ThreadContext while the syscall is pending. + tc->suspend(); + + DPRINTF_SYSCALL(Base, "%s needs retry.\n", name()); + setupRetry(tc); + return; + } + + handleReturn(tc, retval); +} + +void +SyscallDesc::retrySyscall(ThreadContext *tc) +{ + DPRINTF_SYSCALL(Base, "Retrying %s...\n", dumper(name(), tc)); + + SyscallReturn retval = executor(this, tc); + + if (retval.needsRetry()) { + DPRINTF_SYSCALL(Base, "%s still needs retry.\n", name()); + setupRetry(tc); + return; + } + + // We're done retrying, so reactivate this ThreadContext. + tc->activate(); + + handleReturn(tc, retval); +} + +void +SyscallDesc::setupRetry(ThreadContext *tc) +{ + // Create an event which will retry the system call later. + auto retry = [this, tc]() { retrySyscall(tc); }; + auto *event = new EventFunctionWrapper(retry, name(), true); + + // Schedule it in about 100 CPU cycles. That will give other contexts + // a chance to execute a bit of code before trying again. + auto *cpu = tc->getCpuPtr(); + curEventQueue()->schedule(event, + curTick() + cpu->cyclesToTicks(Cycles(100))); +} + +void +SyscallDesc::handleReturn(ThreadContext *tc, const SyscallReturn &ret) +{ + if (ret.suppressed()) { DPRINTF_SYSCALL(Base, "No return value.\n", name()); } else { - returnInto(tc, retval); - DPRINTF_SYSCALL(Base, "Returned %d.\n", retval.encodedValue()); + returnInto(tc, ret); + DPRINTF_SYSCALL(Base, "Returned %d.\n", ret.encodedValue()); } } diff --git a/src/sim/syscall_desc.hh b/src/sim/syscall_desc.hh index 6ded904097..e89cf4902b 100644 --- a/src/sim/syscall_desc.hh +++ b/src/sim/syscall_desc.hh @@ -95,11 +95,16 @@ class SyscallDesc _name(name), _num(num), executor(exec), dumper(dump) {} + void retrySyscall(ThreadContext *tc); + private: /** System call name (e.g., open, mmap, clone, socket, etc.) */ std::string _name; int _num; + void setupRetry(ThreadContext *tc); + void handleReturn(ThreadContext *tc, const SyscallReturn &ret); + /** Mechanism for ISAs to connect to the emul function definitions */ Executor executor; Dumper dumper; From 3e83208332e31137f0507c1909fda47c364ba30d Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Wed, 15 Dec 2021 21:17:39 -0800 Subject: [PATCH 11/18] ext: Stop using the uninitialized pythonMain in sst. Import the __main__ module when it's first used. Change-Id: If800bd575398970faa8cb88072becd3d2b4218c0 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54307 Maintainer: Gabe Black Reviewed-by: Jason Lowe-Power Tested-by: kokoro --- ext/sst/gem5.cc | 3 ++- ext/sst/gem5.hh | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/sst/gem5.cc b/ext/sst/gem5.cc index 1d90bc5b38..286f5195ec 100644 --- a/ext/sst/gem5.cc +++ b/ext/sst/gem5.cc @@ -355,7 +355,8 @@ gem5Component::doSimLoop(gem5::EventQueue* eventq) int gem5Component::execPythonCommands(const std::vector& commands) { - PyObject *dict = PyModule_GetDict(pythonMain); + static PyObject *dict = + py::module_::import("__main__").attr("__dict__").ptr(); PyObject *result; diff --git a/ext/sst/gem5.hh b/ext/sst/gem5.hh index 27124114b1..447c68c3b2 100644 --- a/ext/sst/gem5.hh +++ b/ext/sst/gem5.hh @@ -104,7 +104,6 @@ class gem5Component: public SST::Component // stuff needed for gem5 sim public: - PyObject *pythonMain; int execPythonCommands(const std::vector& commands); private: From 539aaa4bc3d46ddafc845bfb5cc96dea8f9c3244 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Wed, 15 Dec 2021 21:19:26 -0800 Subject: [PATCH 12/18] ext: In sst, don't assume existing imports in python blobs. When executing blobs of python code in the gem5 sst integration, don't assume previous blobs have already imported modules. Import them explicitly instead. Change-Id: I3aa008ffa092cf8ad82ad057eb73bc9de4bf77c5 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54308 Maintainer: Gabe Black Reviewed-by: Jason Lowe-Power Tested-by: kokoro --- ext/sst/gem5.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ext/sst/gem5.cc b/ext/sst/gem5.cc index 286f5195ec..01ea3ebea6 100644 --- a/ext/sst/gem5.cc +++ b/ext/sst/gem5.cc @@ -194,6 +194,7 @@ gem5Component::init(unsigned phase) initPython(args.size(), &args[0]); const std::vector m5_instantiate_commands = { + "import m5", "m5.instantiate()" }; execPythonCommands(m5_instantiate_commands); @@ -201,7 +202,10 @@ gem5Component::init(unsigned phase) // calling SimObject.startup() const std::vector simobject_setup_commands = { "import atexit", - "import _m5", + "import _m5.core", + "import m5", + "import m5.stats", + "import m5.objects.Root", "root = m5.objects.Root.getInstance()", "for obj in root.descendants(): obj.startup()", "atexit.register(m5.stats.dump)", @@ -258,6 +262,7 @@ gem5Component::clockTick(SST::Cycle_t currentCycle) ); // output gem5 stats const std::vector output_stats_commands = { + "import m5.stats" "m5.stats.dump()" }; execPythonCommands(output_stats_commands); From 2e4b7d90916efa9ebbbe9a978ab25fdeb0b80576 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Wed, 15 Dec 2021 22:33:17 -0800 Subject: [PATCH 13/18] sim,python: Use scoped_interpreter_guard's argc and argv ctor arguments. When pybind11's scoped_interpreter_guard is constructed, it can take argc and argv parameters which it uses to intitialize the sys.argv list in python. We can use that mechanism instead of setting that up manually ourselves. Change-Id: If34fac610d1f829aef313bb9ea4c9dfe6bc8fc0f Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54309 Reviewed-by: Andreas Sandberg Maintainer: Gabe Black Reviewed-by: Jason Lowe-Power Tested-by: kokoro --- src/sim/main.cc | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/src/sim/main.cc b/src/sim/main.cc index c44531e2f6..81a691d15d 100644 --- a/src/sim/main.cc +++ b/src/sim/main.cc @@ -60,28 +60,11 @@ main(int argc, char **argv) // It's probably not necessary, but is mostly harmless and might be useful. Py_SetProgramName(program.get()); - py::scoped_interpreter guard; + py::scoped_interpreter guard(true, argc, argv); auto importer = py::module_::import("importer"); importer.attr("install")(); - // Embedded python doesn't set up sys.argv, so we'll do that ourselves. - py::list py_argv; - auto sys = py::module::import("sys"); - if (py::hasattr(sys, "argv")) { - // sys.argv already exists, so grab that. - py_argv = sys.attr("argv"); - } else { - // sys.argv doesn't exist, so create it. - sys.add_object("argv", py_argv); - } - // Clear out argv just in case it has something in it. - py_argv.attr("clear")(); - - // Fill it with our argvs. - for (int i = 0; i < argc; i++) - py_argv.append(argv[i]); - try { py::module_::import("m5").attr("main")(); } catch (py::error_already_set &e) { From e35812f256085eb4342642425e1d67aa7f12d0a0 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Wed, 15 Dec 2021 22:35:01 -0800 Subject: [PATCH 14/18] ext: In sst, set sys.argv up even when not initializing python. pybind11 gives us a simple way to set up sys.argv when initializing the python interpreter, but we need to set that up even if the python interpreter is already running. We need to do that manually, which we do like gem5's main() used to. Change-Id: If9b79a80e05158226d13f68bf06a2a98a36c2602 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54310 Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power Tested-by: kokoro --- ext/sst/gem5.cc | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/ext/sst/gem5.cc b/ext/sst/gem5.cc index 01ea3ebea6..8eab628129 100644 --- a/ext/sst/gem5.cc +++ b/ext/sst/gem5.cc @@ -382,8 +382,27 @@ gem5Component::initPython(int argc, char *_argv[]) // Initialize gem5 special signal handling. gem5::initSignals(); - if (!Py_IsInitialized()) - py::initialize_interpreter(false, argc, _argv); + if (!Py_IsInitialized()) { + py::initialize_interpreter(true, argc, _argv); + } else { + // pybind doesn't provide a way to set sys.argv if not initializing the + // interpreter, so we have to do that manually if it's already running. + py::list py_argv; + auto sys = py::module::import("sys"); + if (py::hasattr(sys, "argv")) { + // sys.argv already exists, so grab that. + py_argv = sys.attr("argv"); + } else { + // sys.argv doesn't exist, so create it. + sys.add_object("argv", py_argv); + } + // Clear out argv just in case it has something in it. + py_argv.attr("clear")(); + + // Fill it with our argvs. + for (int i = 0; i < argc; i++) + py_argv.append(_argv[i]); + } auto importer = py::module_::import("importer"); importer.attr("install")(); From 52c04aa51795fe6047a8ca9de940a28917d60608 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Wed, 15 Dec 2021 23:10:03 -0800 Subject: [PATCH 15/18] systemc: Eliminate the unused PythonReadyFunc mechanism. Change-Id: I8892e4d209901454f2ab923aa3fa9932d7963274 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54323 Maintainer: Gabe Black Reviewed-by: Jason Lowe-Power Tested-by: kokoro --- src/systemc/core/SystemC.py | 8 -------- src/systemc/core/python.cc | 20 -------------------- src/systemc/core/python.hh | 9 --------- 3 files changed, 37 deletions(-) diff --git a/src/systemc/core/SystemC.py b/src/systemc/core/SystemC.py index 682817502d..592b950b67 100644 --- a/src/systemc/core/SystemC.py +++ b/src/systemc/core/SystemC.py @@ -63,11 +63,3 @@ class SystemC_ScModule(SystemC_ScObject): @cxxMethod(return_value_policy="reference", cxx_name="gem5_getPort") def getPort(self, if_name, iex): return None - -try: - import _m5 -except: - pass -else: - import _m5.systemc - _m5.systemc.python_ready() diff --git a/src/systemc/core/python.cc b/src/systemc/core/python.cc index 8f2d56b0ac..2283ae7ea2 100644 --- a/src/systemc/core/python.cc +++ b/src/systemc/core/python.cc @@ -38,13 +38,6 @@ namespace sc_gem5 namespace { -PythonReadyFunc *& -firstReadyFunc() -{ - static PythonReadyFunc *first = nullptr; - return first; -} - PythonInitFunc *& firstInitFunc() { @@ -52,18 +45,10 @@ firstInitFunc() return first; } -void -python_ready(pybind11::args args) -{ - for (auto ptr = firstReadyFunc(); ptr; ptr = ptr->next) - ptr->run(); -} - void systemc_pybind(pybind11::module_ &m_internal) { pybind11::module_ m = m_internal.def_submodule("systemc"); - m.def("python_ready", &python_ready); for (auto ptr = firstInitFunc(); ptr; ptr = ptr->next) ptr->run(m); } @@ -71,11 +56,6 @@ gem5::EmbeddedPyBind embed_("systemc", &systemc_pybind); } // anonymous namespace -PythonReadyFunc::PythonReadyFunc() : next(firstReadyFunc()) -{ - firstReadyFunc() = this; -} - PythonInitFunc::PythonInitFunc() : next(firstInitFunc()) { firstInitFunc() = this; diff --git a/src/systemc/core/python.hh b/src/systemc/core/python.hh index 61c9c80d0c..5a3f6efece 100644 --- a/src/systemc/core/python.hh +++ b/src/systemc/core/python.hh @@ -33,15 +33,6 @@ namespace sc_gem5 { -struct PythonReadyFunc -{ - PythonReadyFunc *next; - - PythonReadyFunc(); - ~PythonReadyFunc() {} - virtual void run() = 0; -}; - struct PythonInitFunc { PythonInitFunc *next; From 372601772c6e23aa617915e481d659d1baf35576 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Thu, 16 Dec 2021 02:35:45 -0800 Subject: [PATCH 16/18] systemc: Update the testing framework to get it working again. The systemc testing framework is not used regularly, and had bit rot and stopped working. This change updates it so that it runs again, and all previously passing tests pass again. These changes were mostly in the related SConscript now that top level targets are built a little differently and that the gem5 shared library is no longer stored in a special construction environment variable. verify.py also needed to be updated since warn() and info() lines now have file and line number information in them, throwing off pre diff filtering of gem5 outputs. Change-Id: Ifdcbd92eab8b9b2168c449bfbcebf52dbe1f016a Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54324 Maintainer: Gabe Black Reviewed-by: Jason Lowe-Power Tested-by: kokoro --- src/systemc/tests/SConscript | 25 ++++++++----------------- src/systemc/tests/verify.py | 6 +++--- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/systemc/tests/SConscript b/src/systemc/tests/SConscript index 7d544f2dd9..fb916d27f1 100644 --- a/src/systemc/tests/SConscript +++ b/src/systemc/tests/SConscript @@ -63,7 +63,8 @@ if env['USE_SYSTEMC'] and GetOption('with_systemc_tests'): test_dir = Dir('.') class SystemCTestBin(Executable): def __init__(self, test): - super().__init__(test.target, *test.sources) + all_sources = test.sources + [with_tag('main')] + super().__init__(test.target, *all_sources) self.reldir = test.reldir self.test_deps = test.deps @@ -78,26 +79,16 @@ if env['USE_SYSTEMC'] and GetOption('with_systemc_tests'): env.Append(CPPPATH=test_dir.Dir('include')) - shared_lib_path = env['SHARED_LIB'][0].abspath - sl_dir, sl_base = os.path.split(shared_lib_path) - env.Append(LIBPATH=[sl_dir], LIBS=[sl_base]) + env.Append(LIBPATH=['${BUILDDIR}'], LIBS=['gem5_${ENV_LABEL}']) + env.AddLocalRPATH('${BUILDDIR}') + + env['OBJSUFFIX'] = '.sc' + env['OBJSUFFIX'][1:] + env['SHOBJSUFFIX'] = '.sc' + env['OBJSUFFIX'][1:] super().declare_all(env) def declare(self, env): - env = env.Clone() - sources = list(self.sources) - for f in self.filters: - sources += Source.all.apply_filter(f) - objs = self.srcs_to_objs(env, sources) - objs = objs + env['MAIN_OBJS'] - relpath = os.path.relpath( - env['SHARED_LIB'][0].dir.abspath, - self.path(env).dir.abspath) - env.Append(LINKFLAGS=Split('-z origin')) - env.Append(RPATH=[ - env.Literal(os.path.join('\\$$ORIGIN', relpath))]) - test_bin = super().declare(env, objs) + test_bin, _u = super().declare(env) test_dir = self.dir.Dir(self.reldir) for dep in self.test_deps: env.Depends(test_bin, test_dir.File(dep)) diff --git a/src/systemc/tests/verify.py b/src/systemc/tests/verify.py index 54b463380d..818855aa31 100755 --- a/src/systemc/tests/verify.py +++ b/src/systemc/tests/verify.py @@ -277,9 +277,9 @@ class LogChecker(DiffingChecker): test_filt = merge_filts( r'^/.*:\d+: ', r'^Global frequency set at \d* ticks per second\n', - r'info: Entering event queue @ \d*\. Starting simulation\.\.\.\n', - r'warn: Ignoring request to set stack size\.\n', - r'^warn: No dot file generated. Please install pydot ' + + r'.*info: Entering event queue @ \d*\. Starting simulation\.\.\.\n', + r'.*warn: Ignoring request to set stack size\.\n', + r'^.*warn: No dot file generated. Please install pydot ' + r'to generate the dot file and pdf.\n', info_filt(804), in_file_filt, From f278b1ea3657a8cccb6ebdf73d3632257fe11e9a Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Thu, 16 Dec 2021 02:40:28 -0800 Subject: [PATCH 17/18] systemc: Change how python initialization callbacks are handled. Because the python environment may already be up and running by the time static initializers are run, specifically when gem5 is built as a library and loaded with dlopen, we can't rely on all of the objects declaring python initialization callbacks having been constructed by the time the code which would execute them runs. To address that problem, this change keeps track of whether the initialization has already happened when a callback is installed, and if so, runs the callback immediately. The original implementation also had users install callbacks by overriding a virtual function in the PythonInitFunc class, and then statically allocating an instance of that subclass so its constructor would be called at initialization time. Calling the function manually if initialization has already happened won't work in that case, because you can't call a virtual function from a constructor and get the behavior you'd want. Instead, this change makes the PythonInitFunc wrap the actual callback which is outside of the structure itself. Because the callback is not a virtual function of PythonInitFunc, we can call it in the constructor without issue. Also, the Callback type has to be a bare function pointer and not a std::function<...> because the argument it takes is a pybind11::module_ reference. Pybind11 sets the visibility of all of its code to hidden to improve binary size, but unfortunately that causes problems when accepting one as an argument in a publically accessible lambda in g++. clang doesn't raise a warning, but g++ does which breaks the build. We could potentially disable this warning, but accepting a function pointer instead works just as well, since captureless lambdas can be trivially converted into function pointers, and they don't seem to upset g++. Change-Id: I3fb321b577090df67c7be3be0e677c2c2055d446 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54325 Maintainer: Gabe Black Reviewed-by: Jason Lowe-Power Tested-by: kokoro --- src/systemc/core/python.cc | 15 +++- src/systemc/core/python.hh | 7 +- src/systemc/core/sc_main_python.cc | 15 ++-- src/systemc/core/sc_time_python.cc | 75 +++++++++---------- .../2/quantum/global_quantum_python.cc | 28 +++---- 5 files changed, 69 insertions(+), 71 deletions(-) diff --git a/src/systemc/core/python.cc b/src/systemc/core/python.cc index 2283ae7ea2..472a050520 100644 --- a/src/systemc/core/python.cc +++ b/src/systemc/core/python.cc @@ -45,20 +45,31 @@ firstInitFunc() return first; } +bool python_initialized = false; + void systemc_pybind(pybind11::module_ &m_internal) { pybind11::module_ m = m_internal.def_submodule("systemc"); for (auto ptr = firstInitFunc(); ptr; ptr = ptr->next) - ptr->run(m); + ptr->callback(m); + + python_initialized = true; } gem5::EmbeddedPyBind embed_("systemc", &systemc_pybind); } // anonymous namespace -PythonInitFunc::PythonInitFunc() : next(firstInitFunc()) +PythonInitFunc::PythonInitFunc(Callback run) : + callback(run), next(firstInitFunc()) { firstInitFunc() = this; + + // If the python was already initialized, run the callback immediately. + if (python_initialized) { + auto systemc_module = pybind11::module_::import("_m5.systemc"); + callback(systemc_module); + } } } // namespace sc_gem5 diff --git a/src/systemc/core/python.hh b/src/systemc/core/python.hh index 5a3f6efece..3c563db642 100644 --- a/src/systemc/core/python.hh +++ b/src/systemc/core/python.hh @@ -35,11 +35,12 @@ namespace sc_gem5 struct PythonInitFunc { + using Callback = void(*)(pybind11::module_ &systemc); + Callback callback; + PythonInitFunc *next; - PythonInitFunc(); - ~PythonInitFunc() {} - virtual void run(pybind11::module_ &systemc) = 0; + PythonInitFunc(Callback run); }; } // namespace sc_gem5 diff --git a/src/systemc/core/sc_main_python.cc b/src/systemc/core/sc_main_python.cc index 1697efe51e..8d2542e5e4 100644 --- a/src/systemc/core/sc_main_python.cc +++ b/src/systemc/core/sc_main_python.cc @@ -92,15 +92,10 @@ sc_main_result_str() // Make our sc_main wrapper available in the internal _m5 python module under // the systemc submodule. -struct InstallScMain : public ::sc_gem5::PythonInitFunc -{ - void - run(pybind11::module_ &systemc) override - { - systemc.def("sc_main", &sc_main); - systemc.def("sc_main_result_code", &sc_main_result_code); - systemc.def("sc_main_result_str", &sc_main_result_str); - } -} installScMain; +::sc_gem5::PythonInitFunc installScMain([](pybind11::module_ &systemc) { + systemc.def("sc_main", &sc_main); + systemc.def("sc_main_result_code", &sc_main_result_code); + systemc.def("sc_main_result_str", &sc_main_result_str); +}); } // anonymous namespace diff --git a/src/systemc/core/sc_time_python.cc b/src/systemc/core/sc_time_python.cc index 58fa65f6f0..be383bccbe 100644 --- a/src/systemc/core/sc_time_python.cc +++ b/src/systemc/core/sc_time_python.cc @@ -33,48 +33,43 @@ namespace { -struct InstallScTime : public ::sc_gem5::PythonInitFunc -{ - void - run(pybind11::module_ &systemc) override - { - pybind11::class_ sc_time(systemc, "sc_time"); - sc_time - // Constructors (omitting nonstandard and deprecated) - .def(pybind11::init<>()) - .def(pybind11::init()) - .def(pybind11::init()) +::sc_gem5::PythonInitFunc installScTime([](pybind11::module_ &systemc) { + pybind11::class_ sc_time(systemc, "sc_time"); + sc_time + // Constructors (omitting nonstandard and deprecated) + .def(pybind11::init<>()) + .def(pybind11::init()) + .def(pybind11::init()) - // Converters. - .def("value", &sc_core::sc_time::value) - .def("to_double", &sc_core::sc_time::to_double) - .def("to_seconds", &sc_core::sc_time::to_seconds) - .def("to_string", &sc_core::sc_time::to_string) - .def("__str__", &sc_core::sc_time::to_string) + // Converters. + .def("value", &sc_core::sc_time::value) + .def("to_double", &sc_core::sc_time::to_double) + .def("to_seconds", &sc_core::sc_time::to_seconds) + .def("to_string", &sc_core::sc_time::to_string) + .def("__str__", &sc_core::sc_time::to_string) - // Operators. - .def(pybind11::self == pybind11::self) - .def(pybind11::self != pybind11::self) - .def(pybind11::self < pybind11::self) - .def(pybind11::self <= pybind11::self) - .def(pybind11::self > pybind11::self) - .def(pybind11::self >= pybind11::self) - .def(pybind11::self += pybind11::self) - .def(pybind11::self -= pybind11::self) - .def(pybind11::self *= double()) - .def(pybind11::self /= double()) - ; + // Operators. + .def(pybind11::self == pybind11::self) + .def(pybind11::self != pybind11::self) + .def(pybind11::self < pybind11::self) + .def(pybind11::self <= pybind11::self) + .def(pybind11::self > pybind11::self) + .def(pybind11::self >= pybind11::self) + .def(pybind11::self += pybind11::self) + .def(pybind11::self -= pybind11::self) + .def(pybind11::self *= double()) + .def(pybind11::self /= double()) + ; - pybind11::enum_(sc_time, "sc_time_unit") - .value("SC_FS", sc_core::SC_FS) - .value("SC_PS", sc_core::SC_PS) - .value("SC_NS", sc_core::SC_NS) - .value("SC_US", sc_core::SC_US) - .value("SC_MS", sc_core::SC_MS) - .value("SC_SEC", sc_core::SC_SEC) - .export_values() - ; - } -} installScTime; + pybind11::enum_(sc_time, "sc_time_unit") + .value("SC_FS", sc_core::SC_FS) + .value("SC_PS", sc_core::SC_PS) + .value("SC_NS", sc_core::SC_NS) + .value("SC_US", sc_core::SC_US) + .value("SC_MS", sc_core::SC_MS) + .value("SC_SEC", sc_core::SC_SEC) + .export_values() + ; +}); } // anonymous namespace diff --git a/src/systemc/tlm_core/2/quantum/global_quantum_python.cc b/src/systemc/tlm_core/2/quantum/global_quantum_python.cc index c29a232d5e..a00db0c377 100644 --- a/src/systemc/tlm_core/2/quantum/global_quantum_python.cc +++ b/src/systemc/tlm_core/2/quantum/global_quantum_python.cc @@ -31,21 +31,17 @@ namespace { -struct InstallTlmGlobalQuantum : public ::sc_gem5::PythonInitFunc -{ - void - run(pybind11::module_ &systemc) override - { - pybind11::class_( - systemc, "tlm_global_quantum") - .def_static("instance", &tlm::tlm_global_quantum::instance, - pybind11::return_value_policy::reference) - .def("set", &tlm::tlm_global_quantum::set) - .def("get", &tlm::tlm_global_quantum::get) - .def("compute_local_quantum", - &tlm::tlm_global_quantum::compute_local_quantum) - ; - } -} installTlmGlobalQuantum; +::sc_gem5::PythonInitFunc installTlmGlobalQuantum( + [](pybind11::module_ &systemc) { + pybind11::class_( + systemc, "tlm_global_quantum") + .def_static("instance", &tlm::tlm_global_quantum::instance, + pybind11::return_value_policy::reference) + .def("set", &tlm::tlm_global_quantum::set) + .def("get", &tlm::tlm_global_quantum::get) + .def("compute_local_quantum", + &tlm::tlm_global_quantum::compute_local_quantum) + ; +}); } // anonymous namespace From c7689b0ddb907c1b593e04280afa3c990b9d65ad Mon Sep 17 00:00:00 2001 From: Hoa Nguyen Date: Thu, 16 Dec 2021 12:58:22 -0800 Subject: [PATCH 18/18] ext: Fix dumping stats error at the end of simulation A missing comma causing two commands to be one, which resulted in a Python interpreter syntax error. Change-Id: Ieb0e3c620c175731084ff0e2040388b15364691e Signed-off-by: Hoa Nguyen Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54343 Reviewed-by: Bobby Bruce Maintainer: Bobby Bruce Tested-by: kokoro --- ext/sst/gem5.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/sst/gem5.cc b/ext/sst/gem5.cc index 8eab628129..7af0eed7b7 100644 --- a/ext/sst/gem5.cc +++ b/ext/sst/gem5.cc @@ -262,7 +262,7 @@ gem5Component::clockTick(SST::Cycle_t currentCycle) ); // output gem5 stats const std::vector output_stats_commands = { - "import m5.stats" + "import m5.stats", "m5.stats.dump()" }; execPythonCommands(output_stats_commands);