[gem5-dev] Re: Broken SST due to python changes

[Gabe Black via gem5-dev]

Thanks, that was really helpful. I've fixed the compilation problems, and
then a few more that were hiding behind that, including a few ones that
were intrinsic to the way sst was initializing the gem5 library (with
dlopen). Was this working to begin with and all this breakage is new, or
was it just that the compilation broke and there were these other problems
already? I want to make sure I don't leave things in a worse state than
when I got here, but I also don't want to go chasing down things that
weren't working to begin with.

I'm going to upload what I have so far, and hopefully someone else can take
a look at it too. The current problem seems to be that the modules in _m5
aren't working right, either because they've been garbage collected, or
because they're not being set up properly, or...

Gabe

On Fri, Dec 10, 2021 at 8:17 PM Bobby Bruce  wrote:

> Hey gabe.
>
> No idea if this is the _best_ solution to your problem, but my solution
> would be to rebuild the image with this installed. Modify the
> `util/dockerfiles/sst-11.1.0/Dockerfile` to the environment you want. Then
> run `docker build -t  util/dockerfiles/sst-11.1.0` to build
> an image with the name "".
>
> Then you can execute `docker run -u $UID:$GID --volume $(pwd):$(pwd) -w
> $(pwd) -it ` within the gem5 directory to spin up and enter
> a container from the image you just built. I think you'll be able to do
> what you want inside the container.
>
> Kind regards,
> Bobby
> --
> Dr. Bobby R. Bruce
> Room 3050,
> Kemper Hall, UC Davis
> Davis,
> CA, 95616
>
> web: https://www.bobbybruce.net
>
>
> On Fri, Dec 10, 2021 at 7:24 PM Gabe Black  wrote:
> >
> > Dumb question: I'm trying to run gdb inside this container on the sst
> thing. How do I do that? It's not installed in the container now, and I
> can't (easily) figure out how to get it installed. I can tell docker to
> install it, but then it seems to throw that away as soon as the command
> ends.
> >
> > Gabe
> >
> > On Fri, Dec 10, 2021 at 5:09 PM Bobby Bruce  wrote:
> >>
> >> Thanks Gabe,
> >>
> >> This is very much appreciated. I'm going to create the release staging
> once a couple more things get in. Feel free to push any patches related to
> these bugs to the release staging branch.
> >>
> >> If there is an order of priority I'd say the bug affecting SST is of
> higher importance than that affecting the Weeklies (as far as I can see the
> latter is hard to trigger). That being said, we'll apply both to the new
> release one way or another.
> >>
> >> --
> >> Dr. Bobby R. Bruce
> >> Room 3050,
> >> Kemper Hall, UC Davis
> >> Davis,
> >> CA, 95616
> >>
> >> web: https://www.bobbybruce.net
> >>
> >>
> >> On Fri, Dec 10, 2021 at 5:02 PM Gabe Black 
> wrote:
> >>>
> >>> Hi Bobby, not yet, I meant to look into this for the last couple days
> but kept running out of time. I'm sitting down to work on it right now.
> >>>
> >>> Gabe
> >>>
> >>> On Fri, Dec 10, 2021 at 1:21 PM Bobby Bruce 
> wrote:
> 
>  Hey Gabe,
> 
>  Is there any update on this?
> 
>  Kind regards,
>  Bobby
>  --
>  Dr. Bobby R. Bruce
>  Room 3050,
>  Kemper Hall, UC Davis
>  Davis,
>  CA, 95616
> 
>  web: https://www.bobbybruce.net
> 
> 
>  On Wed, Dec 8, 2021 at 5:51 PM Hoa Nguyen via gem5-dev <
> gem5-dev@gem5.org> wrote:
> >
> > Hi Gabe,
> >
> > I have more details about this. In this use case, SST initialized the
> > Python environment before adding the "gem5 object". This gem5 object
> > will add more Python stuff from gem5 to the environment.
> >
> > The function that does that is initPython()
> >
> https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/ext/sst/gem5.cc#415
> >
> > The following commands will pull the docker image for SST testing
> > purposes (note that host_gem5_root and guest_gem5_root must be
> > specified),
> >
> > ```
> > docker run -u $UID:$GID --volume
> "${host_gem5_root}":"${guest_gem5_root}" -w \
> >  "${guest_gem5_root}" --rm gcr.io/gem5-test/sst-env \
> >  bash -c "\
> > scons build/RISCV/libgem5_opt.so -j${nproc} --without-tcmalloc; \
> > cd ext/sst; \
> > make clean; make; \
> > sst --add-lib-path=./ sst/example.py;
> > ```
> >
> > We appreciate your help!
> >
> > Regards,
> > Hoa Nguyen
> >
> > On 12/8/21, Jason Lowe-Power  wrote:
> > > Hey Gabe,
> > >
> > > This change breaks the SST integration. In the SST integration
> python is
> > > initialized from the SST module, not from init.cc (this is because
> SST has
> > > their own python interpreter).
> > >
> > > We would appreciate some help in fixing this. Hoa and Giacomo can
> give you
> > > an example that's breaking to help you fix it, I believe.
> > >
> > > https://gem5-review.googlesource.com/c/public/gem5/+/49413
> > >
> > > There's strong interest in having the SST integration working 

[gem5-dev] Re: Broken SST due to python changes

[Jason Lowe-Power via gem5-dev]

Everything was working as of last week-ish, if I remember correctly. We
were booting both Arm and RISCV full system with gem5 cores and SST
caches/memory.

Cheers,
Jason

On Sat, Dec 11, 2021, 8:23 AM Gabe Black  wrote:

> Thanks, that was really helpful. I've fixed the compilation problems, and
> then a few more that were hiding behind that, including a few ones that
> were intrinsic to the way sst was initializing the gem5 library (with
> dlopen). Was this working to begin with and all this breakage is new, or
> was it just that the compilation broke and there were these other problems
> already? I want to make sure I don't leave things in a worse state than
> when I got here, but I also don't want to go chasing down things that
> weren't working to begin with.
>
> I'm going to upload what I have so far, and hopefully someone else can
> take a look at it too. The current problem seems to be that the modules in
> _m5 aren't working right, either because they've been garbage collected, or
> because they're not being set up properly, or...
>
> Gabe
>
> On Fri, Dec 10, 2021 at 8:17 PM Bobby Bruce  wrote:
>
>> Hey gabe.
>>
>> No idea if this is the _best_ solution to your problem, but my solution
>> would be to rebuild the image with this installed. Modify the
>> `util/dockerfiles/sst-11.1.0/Dockerfile` to the environment you want. Then
>> run `docker build -t  util/dockerfiles/sst-11.1.0` to build
>> an image with the name "".
>>
>> Then you can execute `docker run -u $UID:$GID --volume $(pwd):$(pwd) -w
>> $(pwd) -it ` within the gem5 directory to spin up and enter
>> a container from the image you just built. I think you'll be able to do
>> what you want inside the container.
>>
>> Kind regards,
>> Bobby
>> --
>> Dr. Bobby R. Bruce
>> Room 3050,
>> Kemper Hall, UC Davis
>> Davis,
>> CA, 95616
>>
>> web: https://www.bobbybruce.net
>>
>>
>> On Fri, Dec 10, 2021 at 7:24 PM Gabe Black  wrote:
>> >
>> > Dumb question: I'm trying to run gdb inside this container on the sst
>> thing. How do I do that? It's not installed in the container now, and I
>> can't (easily) figure out how to get it installed. I can tell docker to
>> install it, but then it seems to throw that away as soon as the command
>> ends.
>> >
>> > Gabe
>> >
>> > On Fri, Dec 10, 2021 at 5:09 PM Bobby Bruce  wrote:
>> >>
>> >> Thanks Gabe,
>> >>
>> >> This is very much appreciated. I'm going to create the release staging
>> once a couple more things get in. Feel free to push any patches related to
>> these bugs to the release staging branch.
>> >>
>> >> If there is an order of priority I'd say the bug affecting SST is of
>> higher importance than that affecting the Weeklies (as far as I can see the
>> latter is hard to trigger). That being said, we'll apply both to the new
>> release one way or another.
>> >>
>> >> --
>> >> Dr. Bobby R. Bruce
>> >> Room 3050,
>> >> Kemper Hall, UC Davis
>> >> Davis,
>> >> CA, 95616
>> >>
>> >> web: https://www.bobbybruce.net
>> >>
>> >>
>> >> On Fri, Dec 10, 2021 at 5:02 PM Gabe Black 
>> wrote:
>> >>>
>> >>> Hi Bobby, not yet, I meant to look into this for the last couple days
>> but kept running out of time. I'm sitting down to work on it right now.
>> >>>
>> >>> Gabe
>> >>>
>> >>> On Fri, Dec 10, 2021 at 1:21 PM Bobby Bruce 
>> wrote:
>> 
>>  Hey Gabe,
>> 
>>  Is there any update on this?
>> 
>>  Kind regards,
>>  Bobby
>>  --
>>  Dr. Bobby R. Bruce
>>  Room 3050,
>>  Kemper Hall, UC Davis
>>  Davis,
>>  CA, 95616
>> 
>>  web: https://www.bobbybruce.net
>> 
>> 
>>  On Wed, Dec 8, 2021 at 5:51 PM Hoa Nguyen via gem5-dev <
>> gem5-dev@gem5.org> wrote:
>> >
>> > Hi Gabe,
>> >
>> > I have more details about this. In this use case, SST initialized
>> the
>> > Python environment before adding the "gem5 object". This gem5 object
>> > will add more Python stuff from gem5 to the environment.
>> >
>> > The function that does that is initPython()
>> >
>> https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/ext/sst/gem5.cc#415
>> >
>> > The following commands will pull the docker image for SST testing
>> > purposes (note that host_gem5_root and guest_gem5_root must be
>> > specified),
>> >
>> > ```
>> > docker run -u $UID:$GID --volume
>> "${host_gem5_root}":"${guest_gem5_root}" -w \
>> >  "${guest_gem5_root}" --rm gcr.io/gem5-test/sst-env \
>> >  bash -c "\
>> > scons build/RISCV/libgem5_opt.so -j${nproc} --without-tcmalloc; \
>> > cd ext/sst; \
>> > make clean; make; \
>> > sst --add-lib-path=./ sst/example.py;
>> > ```
>> >
>> > We appreciate your help!
>> >
>> > Regards,
>> > Hoa Nguyen
>> >
>> > On 12/8/21, Jason Lowe-Power  wrote:
>> > > Hey Gabe,
>> > >
>> > > This change breaks the SST integration. In the SST integration
>> python is
>> > > initialized from the SST module, not from init.cc (this is
>> be

[gem5-dev] Change in gem5/gem5[develop]: python: Replace PYBIND11_EMBEDDED_MODULE with GEM5_PYBIND_MODULE_INIT.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54004 )



Change subject: python: Replace PYBIND11_EMBEDDED_MODULE with  
GEM5_PYBIND_MODULE_INIT.

..

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
---
M src/python/importer.cc
M src/sim/init.cc
2 files changed, 26 insertions(+), 7 deletions(-)



diff --git a/src/python/importer.cc b/src/python/importer.cc
index 2d10e96..dde86a8 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 @@
 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 8c0d1aa..d594b08 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 @@
 }
 }

-PYBIND11_EMBEDDED_MODULE(_m5, _m5)
-{
-EmbeddedPyBind::initAll(_m5);
-}
+GEM5_PYBIND_MODULE_INIT(_m5, EmbeddedPyBind::initAll)

 } // namespace gem5

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ic3574c32244e5ac475222f6d305ddc70dd6298d6
Gerrit-Change-Number: 54004
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: ext: Fix compilation of the sst gem5 integration.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54006 )



Change subject: ext: Fix compilation of the sst gem5 integration.
..

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
---
M ext/sst/gem5.cc
M ext/sst/gem5.hh
2 files changed, 31 insertions(+), 87 deletions(-)



diff --git a/ext/sst/gem5.cc b/ext/sst/gem5.cc
index c93c722..1d90bc5 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 @@
 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 34d8bd6..2712411 100644
--- a/ext/sst/gem5.hh
+++ b/ext/sst/gem5.hh
@@ -101,8 +101,6 @@
 void finish();
 bool clockTick(SST::Cycle_t current_cycle);

-int startM5(int argc, char **_argv);
-

   // stuff needed for gem5 sim
   public:

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I95714a9264636c14e1dda3174bc0d79e3d881727
Gerrit-Change-Number: 54006
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@

[gem5-dev] Change in gem5/gem5[develop]: python: Add a mechanism for installing pybind modules.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54003 )



Change subject: python: Add a mechanism for installing pybind modules.
..

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
---
A src/python/pybind_init.hh
1 file changed, 89 insertions(+), 0 deletions(-)



diff --git a/src/python/pybind_init.hh b/src/python/pybind_init.hh
new file mode 100644
index 000..102f3f1
--- /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__

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I6f36c5f3831dda893039b4923902e9ce46a91808
Gerrit-Change-Number: 54003
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: sim: Make the EmbeddedPyBind::initAll method work correctly.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/54005 )



Change subject: sim: Make the EmbeddedPyBind::initAll method work correctly.
..

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
---
M src/sim/init.cc
M src/sim/init.hh
2 files changed, 87 insertions(+), 46 deletions(-)



diff --git a/src/sim/init.cc b/src/sim/init.cc
index d594b08..1c3d3aa 100644
--- a/src/sim/init.cc
+++ b/src/sim/init.cc
@@ -55,76 +55,89 @@
 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 9e7158a..3b86f82 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 @@
   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 pybind1

[gem5-dev] Re: Broken SST due to python changes

[Gabe Black via gem5-dev]

https://gem5-review.googlesource.com/c/public/gem5/+/54006

At least one problem I ran into was that the order of static initializers
was not determinstic, so the structures which said what embedded python
modules existed might be statically constructed after the thing that
consumes and sets them up. That could work by coincidence if the order is
favorable, but it wasn't for me and so lots of things didn't exist as far
as the python was concerned.

To be clear, there is some mysterious deeper issue here which is probably
separate from that. As far as I can tell running gem5 itself is working
fine, but when running as a library in sst there is some sort of memory
corruption or premature garbage collection or something, and it leads to a
segfault or incorrect behavior (failure to import modules, failure to find
things inside them, etc). Things were even more tempermental until I added
what feels like an unnecessary increment of the reference count for the
modules in the new pybind_init.hh, which seemed to tame things in gem5
proper at least. I'm too tired to keep looking for it right now, but
hopefully this will give you a good leg up.

Gabe

On Sat, Dec 11, 2021 at 8:25 AM Jason Lowe-Power 
wrote:

> Everything was working as of last week-ish, if I remember correctly. We
> were booting both Arm and RISCV full system with gem5 cores and SST
> caches/memory.
>
> Cheers,
> Jason
>
> On Sat, Dec 11, 2021, 8:23 AM Gabe Black  wrote:
>
>> Thanks, that was really helpful. I've fixed the compilation problems, and
>> then a few more that were hiding behind that, including a few ones that
>> were intrinsic to the way sst was initializing the gem5 library (with
>> dlopen). Was this working to begin with and all this breakage is new, or
>> was it just that the compilation broke and there were these other problems
>> already? I want to make sure I don't leave things in a worse state than
>> when I got here, but I also don't want to go chasing down things that
>> weren't working to begin with.
>>
>> I'm going to upload what I have so far, and hopefully someone else can
>> take a look at it too. The current problem seems to be that the modules in
>> _m5 aren't working right, either because they've been garbage collected, or
>> because they're not being set up properly, or...
>>
>> Gabe
>>
>> On Fri, Dec 10, 2021 at 8:17 PM Bobby Bruce  wrote:
>>
>>> Hey gabe.
>>>
>>> No idea if this is the _best_ solution to your problem, but my solution
>>> would be to rebuild the image with this installed. Modify the
>>> `util/dockerfiles/sst-11.1.0/Dockerfile` to the environment you want. Then
>>> run `docker build -t  util/dockerfiles/sst-11.1.0` to build
>>> an image with the name "".
>>>
>>> Then you can execute `docker run -u $UID:$GID --volume $(pwd):$(pwd) -w
>>> $(pwd) -it ` within the gem5 directory to spin up and enter
>>> a container from the image you just built. I think you'll be able to do
>>> what you want inside the container.
>>>
>>> Kind regards,
>>> Bobby
>>> --
>>> Dr. Bobby R. Bruce
>>> Room 3050,
>>> Kemper Hall, UC Davis
>>> Davis,
>>> CA, 95616
>>>
>>> web: https://www.bobbybruce.net
>>>
>>>
>>> On Fri, Dec 10, 2021 at 7:24 PM Gabe Black  wrote:
>>> >
>>> > Dumb question: I'm trying to run gdb inside this container on the sst
>>> thing. How do I do that? It's not installed in the container now, and I
>>> can't (easily) figure out how to get it installed. I can tell docker to
>>> install it, but then it seems to throw that away as soon as the command
>>> ends.
>>> >
>>> > Gabe
>>> >
>>> > On Fri, Dec 10, 2021 at 5:09 PM Bobby Bruce 
>>> wrote:
>>> >>
>>> >> Thanks Gabe,
>>> >>
>>> >> This is very much appreciated. I'm going to create the release
>>> staging once a couple more things get in. Feel free to push any patches
>>> related to these bugs to the release staging branch.
>>> >>
>>> >> If there is an order of priority I'd say the bug affecting SST is of
>>> higher importance than that affecting the Weeklies (as far as I can see the
>>> latter is hard to trigger). That being said, we'll apply both to the new
>>> release one way or another.
>>> >>
>>> >> --
>>> >> Dr. Bobby R. Bruce
>>> >> Room 3050,
>>> >> Kemper Hall, UC Davis
>>> >> Davis,
>>> >> CA, 95616
>>> >>
>>> >> web: https://www.bobbybruce.net
>>> >>
>>> >>
>>> >> On Fri, Dec 10, 2021 at 5:02 PM Gabe Black 
>>> wrote:
>>> >>>
>>> >>> Hi Bobby, not yet, I meant to look into this for the last couple
>>> days but kept running out of time. I'm sitting down to work on it right now.
>>> >>>
>>> >>> Gabe
>>> >>>
>>> >>> On Fri, Dec 10, 2021 at 1:21 PM Bobby Bruce 
>>> wrote:
>>> 
>>>  Hey Gabe,
>>> 
>>>  Is there any update on this?
>>> 
>>>  Kind regards,
>>>  Bobby
>>>  --
>>>  Dr. Bobby R. Bruce
>>>  Room 3050,
>>>  Kemper Hall, UC Davis
>>>  Davis,
>>>  CA, 95616
>>> 
>>>  web: https://www.bobbybruce.net
>>> 
>>> 
>>>  On Wed, Dec 8, 2021 at 5:51 PM Hoa Nguyen via gem5-dev <
>>> gem5

[gem5-dev] Build failed in Jenkins: nightly #70

[jenkins-no-reply--- via gem5-dev]

See 

Changes:

[giacomo.travaglini] arch-arm: Add partial param to TlbEntry

[Bobby R. Bruce] stdlib: Fix CustomResource metadata

[Bobby R. Bruce] stdlib: Add CustomDiskImageResource

[Bobby R. Bruce] stdlib,configs: Set SPEC examples partition param to optional

[Bobby R. Bruce] configs: Fix terminology to 'ticks' in NPB configs

[Bobby R. Bruce] tests: Add default DRAM class for riscv/x86 boot tests

[Bobby R. Bruce] tests: Add test for the lupv example

[Bobby R. Bruce] stdlib: Add beta simulate module to the gem5 stdlib

[Bobby R. Bruce] arch-vega: Implement V_OR3_B32

[Bobby R. Bruce] arch-vega: Implement V_LSHL_OR_B32

[Bobby R. Bruce] arch-vega: Implement V_LSHL_ADD_U32

[Bobby R. Bruce] arch-vega: Impelemnt V_ADD_LSHL_U32

[Bobby R. Bruce] arch-vega: Implement V_ADD3_U32

[Bobby R. Bruce] arch-vega: Implement V_AND_OR_B32

[Bobby R. Bruce] arch-vega: Implement S_SLEEP

[Bobby R. Bruce] python: Add simulator instantiation checks

[Bobby R. Bruce] misc: Remove AMD license addition


--
[...truncated 2.52 MB...]
 [   SHCXX] RISCV/cpu/minor/pipeline.cc -> .os
 [   SHCXX] RISCV/python/_m5/param_NonCachingSimpleCPU.cc -> .os
 [   SHCXX] RISCV/cpu/o3/inst_queue.cc -> .os
 [   SHCXX] RISCV/cpu/o3/cpu.cc -> .os
 [   SHCXX] RISCV/python/_m5/param_AtomicSimpleCPU.cc -> .os
 [   SHCXX] RISCV/cpu/o3/fetch.cc -> .os
 [   SHCXX] RISCV/cpu/minor/fetch1.cc -> .os
 [   SHCXX] RISCV/python/_m5/param_BaseSimpleCPU.cc -> .os
 [   SHCXX] RISCV/cpu/simple_thread.cc -> .os
 [   SHCXX] RISCV/cpu/o3/probe/simple_trace.cc -> .os
 [   SHCXX] RISCV/cpu/o3/commit.cc -> .os
 [   SHCXX] RISCV/cpu/simple/base.cc -> .os
 [   SHCXX] RISCV/cpu/o3/mem_dep_unit.cc -> .os
 [   SHCXX] RISCV/cpu/minor/lsq.cc -> .os
 [   SHCXX] RISCV/cpu/checker/cpu.cc -> .os
 [   SHCXX] RISCV/mem/ruby/network/garnet/GarnetNetwork.cc -> .os
 [   SHCXX] RISCV/python/_m5/param_GarnetExtLink.cc -> .os
 [   SHCXX] RISCV/mem/ruby/network/garnet/NetworkBridge.cc -> .os
 [   SHCXX] RISCV/python/_m5/param_GarnetIntLink.cc -> .os
 [   SHCXX] RISCV/mem/ruby/network/garnet/GarnetLink.cc -> .os
 [   SHCXX] RISCV/python/_m5/param_NetworkBridge.cc -> .os
 [   SHCXX] RISCV/mem/ruby/profiler/Profiler.cc -> .os
 [   SHCXX] RISCV/python/_m5/param_RiscvEmuLinux.cc -> .os
 [   SHCXX] RISCV/arch/riscv/linux/se_workload.cc -> .os
 [   SHCXX] RISCV/python/_m5/param_TimingSimpleCPU.cc -> .os
 [   SHCXX] RISCV/cpu/simple/timing.cc -> .os
 [SHCC] softfloat/f64_to_i32_r_minMag.c -> .os
 [SHCC] softfloat/f64_to_i64.c -> .os
 [SHCC] softfloat/f64_to_i64_r_minMag.c -> .os
 [SHCC] softfloat/f64_to_ui32.c -> .os
 [SHCC] softfloat/f64_to_ui32_r_minMag.c -> .os
 [SHCC] softfloat/f64_to_ui64.c -> .os
 [SHCC] softfloat/f64_to_ui64_r_minMag.c -> .os
 [SHCC] softfloat/i32_to_f128.c -> .os
 [SHCC] softfloat/i32_to_f16.c -> .os
 [SHCC] softfloat/i32_to_f32.c -> .os
 [SHCC] softfloat/i32_to_f64.c -> .os
 [SHCC] softfloat/i64_to_f128.c -> .os
 [SHCC] softfloat/i64_to_f16.c -> .os
 [SHCC] softfloat/i64_to_f32.c -> .os
 [SHCC] softfloat/i64_to_f64.c -> .os
 [SHCC] softfloat/s_add128.c -> .os
 [SHCC] softfloat/s_add256M.c -> .os
 [SHCC] softfloat/s_addCarryM.c -> .os
 [SHCC] softfloat/s_addComplCarryM.c -> .os
 [SHCC] softfloat/s_addMagsF128.c -> .os
 [SHCC] softfloat/s_addMagsF16.c -> .os
 [SHCC] softfloat/s_addMagsF32.c -> .os
 [SHCC] softfloat/s_addMagsF64.c -> .os
 [SHCC] softfloat/s_addM.c -> .os
 [SHCC] softfloat/s_approxRecip_1Ks.c -> .os
 [SHCC] softfloat/s_approxRecip32_1.c -> .os
 [SHCC] softfloat/s_approxRecipSqrt_1Ks.c -> .os
 [SHCC] softfloat/s_approxRecipSqrt32_1.c -> .os
 [SHCC] softfloat/s_commonNaNToF128UI.c -> .os
 [SHCC] softfloat/s_commonNaNToF16UI.c -> .os
 [SHCC] softfloat/s_commonNaNToF32UI.c -> .os
 [SHCC] softfloat/s_commonNaNToF64UI.c -> .os
 [SHCC] softfloat/s_compare128M.c -> .os
 [SHCC] softfloat/s_compare96M.c -> .os
 [SHCC] softfloat/s_countLeadingZeros16.c -> .os
 [SHCC] softfloat/s_countLeadingZeros32.c -> .os
 [SHCC] softfloat/s_countLeadingZeros64.c -> .os
 [SHCC] softfloat/s_countLeadingZeros8.c -> .os
 [SHCC] softfloat/s_eq128.c -> .os
 [SHCC] softfloat/s_f128UIToCommonNaN.c -> .os
 [SHCC] softfloat/s_f16UIToCommonNaN.c -> .os
 [SHCC] softfloat/s_f32UIToCommonNaN.c -> .os
 [SHCC] softfloat/s_f64UIToCommonNaN.c -> .os
 [SHCC] softfloat/s_le128.c -> .os
 [SHCC] softfloat/s_lt128.c -> .os
 [SHCC] softfloat/s_mul128By32.c -> .os
 [SHCC] softfloat/s_mul128MTo256M.c -> .os
 [SHCC] softfloat/s_mul128To256M.c -> .os
 [SHCC] softfloat/s_mul64ByShifted32To128.c -> .os
 [SHCC] softfloat/s_mul64To128.c -> .os
 [SHCC] softfloat/s_mul64To128M.c -> .os
 [SHCC] softfloat/s_mulAddF128.c -> .os
 [SHCC] softfloat/s_mulAddF16.c -> .os
 [SHCC] softfloat/s_mulAdd

[gem5-dev] Change in gem5/gem5[develop]: arch,sim-se: Update the PC before emulating a system call.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/53983 )


Change subject: arch,sim-se: Update the PC before emulating a system call.
..

arch,sim-se: Update the PC before emulating a system call.

For most system calls, it doesn't matter if the PC is advanced to the
instruction after the system call instruction before or after the system
call itself is invoked, because the system call itself doesn't interact
with it.

For some system calls however, specifically "clone" and "execve",
advancing the PC *after* the system call complicates things, because it
means the system call needs to set the PC to something that will equal
the desired value only *after* it's advanced.

By setting the PC *before* the system call, the system call can set the
PC to whatever it needs to. This means the new cloned context doesn't
need to advance the PC because it's already advanced, and execve doesn't
need to set NPC, it can leave the PC set to the correct value from the
entry point set during Process initialization.

Change-Id: I830607c2e9adcc22e738178fd3663417512e2e56
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/53983
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
Reviewed-by: Matt Sinclair 
Maintainer: Matt Sinclair 
Reviewed-by: Kyle Roarty 
---
M src/arch/x86/linux/se_workload.cc
M src/sim/faults.cc
M src/arch/arm/faults.cc
M src/sim/syscall_emul.hh
M src/arch/riscv/faults.cc
5 files changed, 42 insertions(+), 14 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, but someone else must approve; Looks  
good to me, approved
  Matt Sinclair: Looks good to me, but someone else must approve; Looks  
good to me, approved

  Kyle Roarty: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/arch/arm/faults.cc b/src/arch/arm/faults.cc
index e340d07..ba5bcc9 100644
--- a/src/arch/arm/faults.cc
+++ b/src/arch/arm/faults.cc
@@ -881,15 +881,15 @@
 return;
 }

-// As of now, there isn't a 32 bit thumb version of this instruction.
-assert(!machInst.bigThumb);
-tc->getSystemPtr()->workload->syscall(tc);
-
 // Advance the PC since that won't happen automatically.
 PCState pc = tc->pcState().as();
 assert(inst);
 inst->advancePC(pc);
 tc->pcState(pc);
+
+// As of now, there isn't a 32 bit thumb version of this instruction.
+assert(!machInst.bigThumb);
+tc->getSystemPtr()->workload->syscall(tc);
 }

 bool
diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc
index 129e767..eea42fe 100644
--- a/src/arch/riscv/faults.cc
+++ b/src/arch/riscv/faults.cc
@@ -157,11 +157,12 @@
 if (isInterrupt() && bits(tc->readMiscReg(tvec), 1, 0) == 1)
 addr += 4 * _code;
 pc_state.set(addr);
+tc->pcState(pc_state);
 } else {
-invokeSE(tc, inst);
 inst->advancePC(pc_state);
+tc->pcState(pc_state);
+invokeSE(tc, inst);
 }
-tc->pcState(pc_state);
 }

 void
diff --git a/src/arch/x86/linux/se_workload.cc  
b/src/arch/x86/linux/se_workload.cc

index f5fa519..d280c7c 100644
--- a/src/arch/x86/linux/se_workload.cc
+++ b/src/arch/x86/linux/se_workload.cc
@@ -120,7 +120,7 @@
 Addr eip = pc.pc();
 const auto &vsyscall = proc32->getVSyscallPage();
 if (eip >= vsyscall.base && eip < vsyscall.base + vsyscall.size) {
-pc.npc(vsyscall.base + vsyscall.vsysexitOffset);
+pc.set(vsyscall.base + vsyscall.vsysexitOffset);
 tc->pcState(pc);
 }
 syscallDescs32.get(rax)->doSyscall(tc);
diff --git a/src/sim/faults.cc b/src/sim/faults.cc
index 98778f2..115c0ed 100644
--- a/src/sim/faults.cc
+++ b/src/sim/faults.cc
@@ -71,11 +71,12 @@
 void
 SESyscallFault::invoke(ThreadContext *tc, const StaticInstPtr &inst)
 {
-tc->getSystemPtr()->workload->syscall(tc);
 // Move the PC forward since that doesn't happen automatically.
 std::unique_ptr pc(tc->pcState().clone());
 inst->advancePC(*pc);
 tc->pcState(*pc);
+
+tc->getSystemPtr()->workload->syscall(tc);
 }

 void
diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh
index a74aabf..546ae75 100644
--- a/src/sim/syscall_emul.hh
+++ b/src/sim/syscall_emul.hh
@@ -1703,9 +1703,6 @@

 desc->returnInto(ctc, 0);

-std::unique_ptr cpc(tc->pcState().clone());
-cpc->advance();
-ctc->pcState(*cpc);
 ctc->activate();

 if (flags & OS::TGT_CLONE_VFORK) {
@@ -2267,9 +2264,6 @@
 new_p->init();
 new_p->initState();
 tc->activate();
-std::unique_ptr pc_state(tc->pcState().clone());
-pc_state->advance();
-tc->pcState(*pc_state);

 return SyscallReturn();
 }

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


Gerrit-Project: public/ge

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Allow the L2 unified TLB to store partial translations

[Bobby Bruce (Gerrit) via gem5-dev]

Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/52126 )


 (

1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.
 )Change subject: arch-arm: Allow the L2 unified TLB to store partial  
translations

..

arch-arm: Allow the L2 unified TLB to store partial translations

We are allowing the L2 TLB to store partial translations from the
second level of lookup

JIRA: https://gem5.atlassian.net/browse/GEM5-1108

Change-Id: I1286c14a256470c2075fe5533930617139d4d087
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52126
Reviewed-by: Jason Lowe-Power 
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M src/arch/arm/ArmMMU.py
1 file changed, 22 insertions(+), 1 deletion(-)

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




diff --git a/src/arch/arm/ArmMMU.py b/src/arch/arm/ArmMMU.py
index abf36e0..a0bbda8 100644
--- a/src/arch/arm/ArmMMU.py
+++ b/src/arch/arm/ArmMMU.py
@@ -65,7 +65,8 @@
 cxx_header = 'arch/arm/mmu.hh'

 # L2 TLBs
-l2_shared = ArmTLB(entry_type="unified", size=1280)
+l2_shared = ArmTLB(entry_type="unified", size=1280,
+partial_levels=["L2"])

 # L1 TLBs
 itb = ArmTLB(entry_type="instruction", next_level=Parent.l2_shared)

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I1286c14a256470c2075fe5533930617139d4d087
Gerrit-Change-Number: 52126
Gerrit-PatchSet: 4
Gerrit-Owner: Giacomo Travaglini 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Allow TLB to be used as a WalkCache

[Bobby Bruce (Gerrit) via gem5-dev]

Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/52124 )


Change subject: arch-arm: Allow TLB to be used as a WalkCache
..

arch-arm: Allow TLB to be used as a WalkCache

This patch allows partial translation entries (intermediate PAs obtained
from a table walk) to be stored in an ArmTLB. This effectively means
reserving a fraction of the TLB entries to cache table walks

JIRA: https://gem5.atlassian.net/browse/GEM5-1108

Change-Id: Id0efb7d75dd017366c4c3b74de7b57355a53a01a
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52124
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M src/arch/arm/ArmTLB.py
M src/arch/arm/mmu.cc
M src/arch/arm/mmu.hh
M src/arch/arm/tlb.cc
M src/arch/arm/tlb.hh
5 files changed, 209 insertions(+), 48 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/arch/arm/ArmTLB.py b/src/arch/arm/ArmTLB.py
index 4c86f72..10ed48b 100644
--- a/src/arch/arm/ArmTLB.py
+++ b/src/arch/arm/ArmTLB.py
@@ -51,6 +51,11 @@
 size = Param.Int(64, "TLB size")
 is_stage2 = Param.Bool(False, "Is this a stage 2 TLB?")

+partial_levels = VectorParam.ArmLookupLevel([],
+"List of intermediate lookup levels allowed to be cached in the  
TLB "

+"(=holding intermediate PAs obtained during a table walk")
+
+
 class ArmStage2TLB(ArmTLB):
 size = 32
 is_stage2 = True
diff --git a/src/arch/arm/mmu.cc b/src/arch/arm/mmu.cc
index cffbb20..f4a2114 100644
--- a/src/arch/arm/mmu.cc
+++ b/src/arch/arm/mmu.cc
@@ -67,6 +67,7 @@
 s1State(this, false), s2State(this, true),
 _attr(0),
 _release(nullptr),
+_hasWalkCache(false),
 stats(this)
 {
 // Cache system-level properties
@@ -102,6 +103,27 @@
 getDTBPtr()->setTableWalker(dtbWalker);

 BaseMMU::init();
+
+_hasWalkCache = checkWalkCache();
+}
+
+bool
+MMU::checkWalkCache() const
+{
+for (auto tlb : instruction) {
+if (static_cast(tlb)->walkCache())
+return true;
+}
+for (auto tlb : data) {
+if (static_cast(tlb)->walkCache())
+return true;
+}
+for (auto tlb : unified) {
+if (static_cast(tlb)->walkCache())
+return true;
+}
+
+return false;
 }

 void
@@ -858,11 +880,11 @@
 Fault fault = getResultTe(&te, req, tc, mode, translation, timing,
   functional, &mergeTe, state);
 // only proceed if we have a valid table entry
-if ((te == NULL) && (fault == NoFault)) delay = true;
+if (!isCompleteTranslation(te) && (fault == NoFault)) delay = true;

 // If we have the table entry transfer some of the attributes to the
 // request that triggered the translation
-if (te != NULL) {
+if (isCompleteTranslation(te)) {
 // Set memory attributes
 DPRINTF(TLBVerbose,
 "Setting memory attributes: shareable: %d,  
innerAttrs: %d, "

@@ -1429,7 +1451,7 @@
 *te = lookup(vaddr, state.asid, state.vmid, state.isHyp, is_secure,  
false,

  false, target_el, false, state.isStage2, mode);

-if (*te == NULL) {
+if (!isCompleteTranslation(*te)) {
 if (req->isPrefetch()) {
 // if the request is a prefetch don't attempt to fill the TLB  
or go
 // any further with the memory access (here we can safely use  
the

@@ -1474,12 +1496,12 @@
 if (state.isStage2) {
 // We are already in the stage 2 TLB. Grab the table entry for  
stage

 // 2 only. We are here because stage 1 translation is disabled.
-TlbEntry *s2_te = NULL;
+TlbEntry *s2_te = nullptr;
 // Get the stage 2 table entry
 fault = getTE(&s2_te, req, tc, mode, translation, timing,  
functional,

   state.isSecure, state.curTranType, state);
 // Check permissions of stage 2
-if ((s2_te != NULL) && (fault == NoFault)) {
+if (isCompleteTranslation(s2_te) && (fault == NoFault)) {
 if (state.aarch64)
 fault = checkPermissions64(s2_te, req, mode, tc, state);
 else
@@ -1489,22 +1511,22 @@
 return fault;
 }

-TlbEntry *s1Te = NULL;
+TlbEntry *s1_te = nullptr;

 Addr vaddr_tainted = req->getVaddr();

 // Get the stage 1 table entry
-fault = getTE(&s1Te, req, tc, mode, translation, timing, functional,
+fault = getTE(&s1_te, req, tc, mode, translation, timing, functional,
   state.isSecure, state.curTranType, state);
 // only proceed if we have a valid table entry
-if ((s1Te != NULL) && (fault == NoFault)) {
+if (isCompleteTranslation(s1_te) && (fault == NoFault)) {
 // Check stage 1 permiss

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Allowing table descriptor to be inserted in TLB

[Bobby Bruce (Gerrit) via gem5-dev]

Bobby Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/52125 )


Change subject: arch-arm: Allowing table descriptor to be inserted in TLB
..

arch-arm: Allowing table descriptor to be inserted in TLB

This patch is modifying both TableWalker and MMU to effectively
store/use partial translations

* TableWalker changes: If there is a TLB supporting partial
translations (implemented with previous patch), the TableWalker will
craft partial entries and forward them to the TLB as walks are performed

* MMU changes: We now instruct the table walker to start a page
table traversal even if we hit in the TLB, if the matching entry
holds a partial translation

JIRA: https://gem5.atlassian.net/browse/GEM5-1108

Change-Id: Id20aaf4ea02960d50d8345f3e174c698af21ad1c
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52125
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M src/arch/arm/mmu.cc
M src/arch/arm/table_walker.cc
M src/arch/arm/table_walker.hh
3 files changed, 176 insertions(+), 45 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/arch/arm/mmu.cc b/src/arch/arm/mmu.cc
index f4a2114..79a5240 100644
--- a/src/arch/arm/mmu.cc
+++ b/src/arch/arm/mmu.cc
@@ -1471,7 +1471,7 @@
 fault = getTableWalker(mode, state.isStage2)->walk(
 req, tc, state.asid, state.vmid, state.isHyp, mode,
 translation, timing, functional, is_secure,
-tran_type, state.stage2DescReq);
+tran_type, state.stage2DescReq, *te);

 // for timing mode, return and wait for table walk,
 if (timing || fault != NoFault) {
diff --git a/src/arch/arm/table_walker.cc b/src/arch/arm/table_walker.cc
index c70d856..f94e765 100644
--- a/src/arch/arm/table_walker.cc
+++ b/src/arch/arm/table_walker.cc
@@ -290,7 +290,7 @@
   vmid_t _vmid, bool _isHyp, MMU::Mode _mode,
   MMU::Translation *_trans, bool _timing, bool _functional,
   bool secure, MMU::ArmTranslationType tranType,
-  bool _stage2Req)
+  bool _stage2Req, const TlbEntry *walk_entry)
 {
 assert(!(_functional && _timing));
 ++stats.walks;
@@ -344,6 +344,11 @@
 }
 currState->transState = _trans;
 currState->req = _req;
+if (walk_entry) {
+currState->walkEntry = *walk_entry;
+} else {
+currState->walkEntry = TlbEntry();
+}
 currState->fault = NoFault;
 currState->asid = _asid;
 currState->vmid = _vmid;
@@ -494,11 +499,15 @@
 // a match, we just want to get rid of the walk. The latter could  
happen

 // when there are multiple outstanding misses to a single page and a
 // previous request has been successfully translated.
-if (!currState->transState->squashed() && !te) {
+if (!currState->transState->squashed() && (!te || te->partial)) {
 // We've got a valid request, lets process it
 pending = true;
 pendingQueue.pop_front();
 // Keep currState in case one of the processWalk... calls NULLs it
+
+if (te && te->partial) {
+currState->walkEntry = *te;
+}
 WalkerState *curr_state_copy = currState;
 Fault f;
 if (currState->aarch64)
@@ -897,7 +906,6 @@
 currState->tcr,
 currState->el);

-LookupLevel start_lookup_level = LookupLevel::Num_ArmLookupLevel;
 bool vaddr_fault = false;
 switch (currState->el) {
   case EL0:
@@ -959,11 +967,6 @@
 tsz = 64 - currState->vtcr.t0sz64;
 tg = GrainMap_tg0[currState->vtcr.tg0];

-start_lookup_level = getPageTableOps(tg)->firstS2Level(
-currState->vtcr.sl0);
-
-panic_if(start_lookup_level == LookupLevel::Num_ArmLookupLevel,
- "Cannot discern lookup level from vtcr.{sl0,tg0}");
 ps = currState->vtcr.ps;
 currState->isUncacheable = currState->vtcr.irgn0 == 0;
 } else {
@@ -1096,15 +1099,6 @@
 tg = Grain4KB;
 }

-// Determine starting lookup level
-if (start_lookup_level == LookupLevel::Num_ArmLookupLevel) {
-const auto* ptops = getPageTableOps(tg);
-
-start_lookup_level = ptops->firstLevel(64 - tsz);
-panic_if(start_lookup_level == LookupLevel::Num_ArmLookupLevel,
- "Table walker couldn't find lookup level\n");
-}
-
 // Clamp to lower limit
 int pa_range = decodePhysAddrRange64(ps);
 if (pa_range > _physAddrRange) {
@@ -1113,22 +1107,12 @@
 currState->physAddrRange = pa_range;
 }

-// Determine table base address
-int stride = tg - 3;
-int base_addr_lo = 3 + tsz - str

[gem5-dev] v21.2 staging branch created; v21.2 release scheduled for Dec 21st

[Bobby Bruce via gem5-dev]

Dear all,

The v21.2 staging branch has been created. Thank you to everyone who worked
hard over the past week to get their changes into the develop branch.

We'll still be accepting bug-fix changes to the v21.2 staging branch.

Kind regards,
Bobby
--
Dr. Bobby R. Bruce
Room 3050,
Kemper Hall, UC Davis
Davis,
CA, 95616

web: https://www.bobbybruce.net
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s