[gem5-dev] Change in gem5/gem5[develop]: scons: Simplify the makeDebugFlagCC python function.

2021-09-15 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/49384 )




9 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

Change subject: scons: Simplify the makeDebugFlagCC python function.
..

scons: Simplify the makeDebugFlagCC python function.

Change-Id: I3fdbdc5a4f2b45153550c65e0d447a3d6cec34f1
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49384
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Hoa Nguyen 
---
M src/SConscript
1 file changed, 23 insertions(+), 37 deletions(-)

Approvals:
  Hoa Nguyen: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/SConscript b/src/SConscript
index 932bed1..51c55f4 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -892,10 +892,6 @@

 code = code_formatter()

-# delay definition of CompoundFlags until after all the definition
-# of all constituent SimpleFlags
-comp_code = code_formatter()
-
 # file header
 code('''
 #include "base/compiler.hh" // For namespace deprecation
@@ -907,43 +903,33 @@
 GEM5_DEPRECATED_NAMESPACE(Debug, debug);
 namespace debug
 {
-
 ''')

-for flag in sorted(env.get('DEBUG_FLAGS', [])):
-name, compound, desc, fmt = flag
+all_flags = env.get('DEBUG_FLAGS', [])
+simple_flags = sorted(filter(lambda f: not f[1], all_flags))
+compound_flags = sorted(filter(lambda f: f[1], all_flags))

-# We intentionally make flag a reference to a heap allocated  
object so

-# (1) It has a similar interface to a global object like before
-# (2) It does not get destructed at the end of simulation
-# The second property is desirable as global objects from different
-# translation units do not have a defined destruction order, so  
it'll
-# be unsafe to access debug flags in their destructor in such  
cases.

-if not compound:
-code('SimpleFlag& $name = *(')
-code.indent()
-if fmt:
-code('new SimpleFlag("$name", "$desc", true)')
-else:
-code('new SimpleFlag("$name", "$desc", false)')
-code.dedent()
-code(');')
-else:
-comp_code('CompoundFlag& $name = *(')
-comp_code.indent()
-comp_code('new CompoundFlag("$name", "$desc", {')
-comp_code.indent()
-for flag in compound:
-comp_code('&$flag,')
-comp_code.dedent()
-comp_code('})')
-comp_code.dedent()
-comp_code(');')
+# We intentionally make flag a reference to a heap allocated object so
+# (1) It has a similar interface to a global object like before
+# (2) It does not get destructed at the end of simulation
+# The second property is desirable as global objects from different
+# translation units do not have a defined destruction order, so it'll
+# be unsafe to access debug flags in their destructor in such cases.
+for name, _, desc, fmt in simple_flags:
+fmt = 'true' if fmt else 'false'
+code('''
+SimpleFlag& $name = *(
+new SimpleFlag("$name", "$desc", $fmt));''')

-code.append(comp_code)
-code()
-code('} // namespace debug')
-code('} // namespace gem5')
+for name, compound, desc, _ in compound_flags:
+code('''
+CompoundFlag& $name = *(new CompoundFlag("$name", "$desc", {
+${{', '.join('&' + flag for flag in compound)}}
+}));''')
+
+code('''
+} // namespace debug
+} // namespace gem5''')

 code.write(str(target[0]))


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49384
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: I3fdbdc5a4f2b45153550c65e0d447a3d6cec34f1
Gerrit-Change-Number: 49384
Gerrit-PatchSet: 11
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Hoa Nguyen 
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]: scons: Create a namedtuple for debug flag info.

2021-09-15 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/49385 )




10 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

Change subject: scons: Create a namedtuple for debug flag info.
..

scons: Create a namedtuple for debug flag info.

This avoids having to rely on certain bits of information being in
certain positions, and also makes it more obvious which piece of
information you're referring to when manipulating the objects.

Change-Id: I93799d00261002996a42a62a7de34c4c275847c5
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49385
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Andreas Sandberg 
Reviewed-by: Hoa Nguyen 
---
M src/SConscript
1 file changed, 30 insertions(+), 19 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved
  Hoa Nguyen: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/SConscript b/src/SConscript
index 51c55f4..f3cac80 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -38,6 +38,7 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

 import bisect
+import collections
 import distutils.spawn
 import importlib
 import importlib.abc
@@ -406,15 +407,17 @@
 #

 def makeDebugFlagHH(target, source, env):
-assert len(target) == 1 and len(source) == 1
+assert len(target) == 1

-name, components, desc, fmt = FromValue(source[0])
+flag = env['DEBUG_FLAG'][0]
+name, desc, components, fmt = \
+flag.name, flag.desc, flag.components, flag.fmt

 code = code_formatter()

-typename = "CompoundFlag" if components else "SimpleFlag"
-component_flag_decls = \
-''.join('extern SimpleFlag& %s;\n' % flag for flag in  
components)

+typename = "CompoundFlag" if flag.components else "SimpleFlag"
+component_flag_decls = ''.join('extern SimpleFlag& %s;\n' % simple for
+simple in components)

 # file header boilerplate
 code('''\
@@ -443,19 +446,22 @@

 code.write(str(target[0]))

+DebugFlagInfo = collections.namedtuple('DebugFlag',
+['name', 'desc', 'components', 'fmt'])
 def DebugFlag(name, desc=None, fmt=False):
 if name == "All":
 raise AttributeError('The "All" flag name is reserved')
 debug_flags = env.get('DEBUG_FLAGS', [])
-if any(name == flag[0] for flag in debug_flags):
+if any(name == flag.name for flag in debug_flags):
 raise AttributeError(f'Flag {name} already specified')

-flag = (name, (), desc, fmt)
+flag = DebugFlagInfo(name, desc, (), fmt)
 env.Append(DEBUG_FLAGS=[flag])

 hh_file = Dir(env['BUILDDIR']).Dir('debug').File(f'{name}.hh')
-env.Command(hh_file, ToValue(flag),
-MakeAction(makeDebugFlagHH, Transform("TRACING", 0)))
+env.Command(hh_file, [], DEBUG_FLAG=[flag],
+action=MakeAction(makeDebugFlagHH, Transform("TRACING", 0),
+varlist=['DEBUG_FLAG']))

 def CompoundFlag(name, flags, desc=None):
 if name == "All":
@@ -464,12 +470,13 @@
 if any(name == flag[0] for flag in debug_flags):
 raise AttributeError(f'Flag {name} already specified')

-flag = (name, flags, desc, False)
+flag = DebugFlagInfo(name, desc, flags, None)
 env.Append(DEBUG_FLAGS=[flag])

-env.Command(Dir(env['BUILDDIR']).Dir('debug').File(f'{name}.hh'),
-ToValue(flag),
-MakeAction(makeDebugFlagHH, Transform("TRACING", 0)))
+hh_file = Dir(env['BUILDDIR']).Dir('debug').File(f'{name}.hh')
+env.Command(hh_file, [], DEBUG_FLAG=[flag],
+action=MakeAction(makeDebugFlagHH, Transform("TRACING", 0),
+varlist=['DEBUG_FLAG']))

 def DebugFormatFlag(name, desc=None):
 DebugFlag(name, desc, True)
@@ -906,8 +913,8 @@
 ''')

 all_flags = env.get('DEBUG_FLAGS', [])
-simple_flags = sorted(filter(lambda f: not f[1], all_flags))
-compound_flags = sorted(filter(lambda f: f[1], all_flags))
+simple_flags = sorted(filter(lambda f: not f.components, all_flags))
+compound_flags = sorted(filter(lambda f: f.components, all_flags))

 # We intentionally make flag a reference to a heap allocated object so
 # (1) It has a similar interface to a global object like before
@@ -915,16 +922,20 @@
 # The second property is desirable as global objects from different
 # translation units do not have a defined destruction order, so it'll
 # be unsafe to access debug flags in their destructor in such cases.
-for name, _, desc, fmt in simple_flags:
-fmt = 'true' if fmt else 'false'
+for flag in simple_flags:
+name, desc, components, fmt = \
+flag.name, flag.desc, flag.components, flag.fmt
+fmt = 'true' if flag.fmt else 'false'
 code('''
 SimpleFlag& $name = *(
 new 

[gem5-dev] Change in gem5/gem5[develop]: scons: Declare PySource Source files in the PySource __init__.

2021-09-15 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/49386 )




10 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.
Change subject: scons: Declare PySource Source files in the PySource  
__init__.

..

scons: Declare PySource Source files in the PySource __init__.

There's no reason to wait until the end to loop over all PySource files
and declare their Source-s then.

Change-Id: I94de1b2123bb94324a647bbc005a923012080cab
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49386
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Hoa Nguyen 
Reviewed-by: Jason Lowe-Power 
---
M src/SConscript
1 file changed, 2 insertions(+), 4 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Hoa Nguyen: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/SConscript b/src/SConscript
index f3cac80..8c5c2b9 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -175,6 +175,8 @@

 marshal_env.Command(self.cpp, [ py_marshal, self.tnode ],
 MakeAction(embedPyFile, Transform("EMBED PY")))
+if main['USE_PYTHON']:
+Source(self.cpp, tags=self.tags, add_tags='python')

 class SimObject(PySource):
 '''Add a SimObject python file as a python source object and add
@@ -957,10 +959,6 @@
Transform("VER TAGS")))
 env.AlwaysBuild(tags)

-if main['USE_PYTHON']:
-for source in PySource.all:
-Source(source.cpp, tags=source.tags, add_tags='python')
-
 
 #
 # Define binaries.  Each different build type (debug, opt, etc.) gets

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49386
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: I94de1b2123bb94324a647bbc005a923012080cab
Gerrit-Change-Number: 49386
Gerrit-PatchSet: 12
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Hoa Nguyen 
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]: base: Fix some style problems in addr_range.hh.

2021-09-15 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/50346 )




1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

Change subject: base: Fix some style problems in addr_range.hh.
..

base: Fix some style problems in addr_range.hh.

Change-Id: Ib55b86350c4bc3f1f44af996db25a1e44826d077
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/50346
Tested-by: kokoro 
Reviewed-by: Daniel Carvalho 
Maintainer: Daniel Carvalho 
---
M src/base/addr_range.hh
1 file changed, 42 insertions(+), 21 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/base/addr_range.hh b/src/base/addr_range.hh
index 04dc8ec..eb611cc 100644
--- a/src/base/addr_range.hh
+++ b/src/base/addr_range.hh
@@ -266,7 +266,8 @@
  *
  * @ingroup api_addr_range
  */
-uint64_t granularity() const
+uint64_t
+granularity() const
 {
 if (interleaved()) {
 auto combined_mask = 0;
@@ -297,7 +298,8 @@
  *
  * @ingroup api_addr_range
  */
-Addr size() const
+Addr
+size() const
 {
 return (_end - _start) >> masks.size();
 }
@@ -330,7 +332,8 @@
  *
  * @ingroup api_addr_range
  */
-std::string to_string() const
+std::string
+to_string() const
 {
 if (interleaved()) {
 std::string str;
@@ -360,7 +363,8 @@
  *
  * @ingroup api_addr_range
  */
-bool mergesWith(const AddrRange& r) const
+bool
+mergesWith(const AddrRange& r) const
 {
 return r._start == _start && r._end == _end &&
 r.masks == masks;
@@ -376,28 +380,31 @@
  *
  * @ingroup api_addr_range
  */
-bool intersects(const AddrRange& r) const
+bool
+intersects(const AddrRange& r) const
 {
-if (_start >= r._end || _end <= r._start)
+if (_start >= r._end || _end <= r._start) {
 // start with the simple case of no overlap at all,
 // applicable even if we have interleaved ranges
 return false;
-else if (!interleaved() && !r.interleaved())
+} else if (!interleaved() && !r.interleaved()) {
 // if neither range is interleaved, we are done
 return true;
+}

 // now it gets complicated, focus on the cases we care about
-if (r.size() == 1)
+if (r.size() == 1) {
 // keep it simple and check if the address is within
 // this range
 return contains(r.start());
-else if (mergesWith(r))
+} else if (mergesWith(r)) {
 // restrict the check to ranges that belong to the
 // same chunk
 return intlvMatch == r.intlvMatch;
-else
+} else {
 panic("Cannot test intersection of %s and %s\n",
   to_string(), r.to_string());
+}
 }

 /**
@@ -410,7 +417,8 @@
  *
  * @ingroup api_addr_range
  */
-bool isSubset(const AddrRange& r) const
+bool
+isSubset(const AddrRange& r) const
 {
 if (interleaved())
 panic("Cannot test subset of interleaved range %s\n",  
to_string());

@@ -435,7 +443,8 @@
  *
  * @ingroup api_addr_range
  */
-bool contains(const Addr& a) const
+bool
+contains(const Addr& a) const
 {
 // check if the address is in the range and if there is either
 // no interleaving, or with interleaving also if the selected
@@ -480,7 +489,8 @@
  *
  * @ingroup api_addr_range
  */
-inline Addr removeIntlvBits(Addr a) const
+inline Addr
+removeIntlvBits(Addr a) const
 {
 // Directly return the address if the range is not interleaved
 // to prevent undefined behavior.
@@ -518,7 +528,8 @@
  *
  * @ingroup api_addr_range
  */
-inline Addr addIntlvBits(Addr a) const
+inline Addr
+addIntlvBits(Addr a) const
 {
 // Directly return the address if the range is not interleaved
 // to prevent undefined behavior.
@@ -572,7 +583,8 @@
  *
  * @ingroup api_addr_range
  */
-Addr getOffset(const Addr& a) const
+Addr
+getOffset(const Addr& a) const
 {
 bool in_range = a >= _start && a < _end;
 if (!in_range) {
@@ -650,7 +662,8 @@
  *
  * @ingroup api_addr_range
  */
-bool operator<(const AddrRange& r) const
+bool
+operator<(const AddrRange& r) const
 {
 if (_start != r._start) {
 return _start < r._start;
@@ -670,7 +683,8 @@
 /**
  * @ingroup api_addr_range
  */
-bool operator==(const AddrRange& r) const
+bool
+operator==(const AddrRange& r) const
 {
 if (_start != r._start)return false;
 

[gem5-dev] Change in gem5/gem5[develop]: scons: Accumulate debug flags in a construction variable.

2021-09-15 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/49383 )




9 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

Change subject: scons: Accumulate debug flags in a construction variable.
..

scons: Accumulate debug flags in a construction variable.

Do this instead of putting them in a dictionary side channel.

Change-Id: I52319f2d42c87ef8e7861e7dc700ba45b8e1629e
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49383
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
Reviewed-by: Hoa Nguyen 
---
M src/SConscript
1 file changed, 16 insertions(+), 15 deletions(-)

Approvals:
  Hoa Nguyen: Looks good to me, approved
  Bobby R. Bruce: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/SConscript b/src/SConscript
index 50633df..932bed1 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -443,30 +443,31 @@

 code.write(str(target[0]))

-debug_flags = {}
 def DebugFlag(name, desc=None, fmt=False):
 if name == "All":
 raise AttributeError('The "All" flag name is reserved')
-if name in debug_flags:
-raise AttributeError("Flag {} already specified".format(name))
+debug_flags = env.get('DEBUG_FLAGS', [])
+if any(name == flag[0] for flag in debug_flags):
+raise AttributeError(f'Flag {name} already specified')

 flag = (name, (), desc, fmt)
-debug_flags[name] = flag
+env.Append(DEBUG_FLAGS=[flag])

-hh_file = Dir(env['BUILDDIR']).Dir('debug').File('%s.hh' % name)
+hh_file = Dir(env['BUILDDIR']).Dir('debug').File(f'{name}.hh')
 env.Command(hh_file, ToValue(flag),
 MakeAction(makeDebugFlagHH, Transform("TRACING", 0)))

 def CompoundFlag(name, flags, desc=None):
 if name == "All":
 raise AttributeError('The "All" flag name is reserved')
-if name in debug_flags:
-raise AttributeError('Flag {} already specified'.format(name))
+debug_flags = env.get('DEBUG_FLAGS', [])
+if any(name == flag[0] for flag in debug_flags):
+raise AttributeError(f'Flag {name} already specified')

 flag = (name, flags, desc, False)
-debug_flags[name] = flag
+env.Append(DEBUG_FLAGS=[flag])

-env.Command(Dir(env['BUILDDIR']).Dir('debug').File('%s.hh' % name),
+env.Command(Dir(env['BUILDDIR']).Dir('debug').File(f'{name}.hh'),
 ToValue(flag),
 MakeAction(makeDebugFlagHH, Transform("TRACING", 0)))

@@ -887,7 +888,7 @@
 # Handle debug flags
 #
 def makeDebugFlagCC(target, source, env):
-assert(len(target) == 1 and len(source) == 1)
+assert(len(target) == 1)

 code = code_formatter()

@@ -909,9 +910,8 @@

 ''')

-for name, flag in sorted(source[0].read().items()):
-n, compound, desc, fmt = flag
-assert n == name
+for flag in sorted(env.get('DEBUG_FLAGS', [])):
+name, compound, desc, fmt = flag

 # We intentionally make flag a reference to a heap allocated  
object so

 # (1) It has a similar interface to a global object like before
@@ -948,8 +948,9 @@
 code.write(str(target[0]))

 # Generate the files for the debug and debug-format flags
-env.Command('debug/flags.cc', Value(debug_flags),
-MakeAction(makeDebugFlagCC, Transform("TRACING", 0)))
+env.Command('debug/flags.cc', [],
+MakeAction(makeDebugFlagCC, Transform("TRACING", 0),
+varlist=['DEBUG_FLAGS']))
 Source('debug/flags.cc', add_tags='gem5 trace')

 # version tags

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49383
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: I52319f2d42c87ef8e7861e7dc700ba45b8e1629e
Gerrit-Change-Number: 49383
Gerrit-PatchSet: 11
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Hoa Nguyen 
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]: scons: Clean up the definition of m5.defines a little bit.

2021-09-15 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/48381 )




10 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

Change subject: scons: Clean up the definition of m5.defines a little bit.
..

scons: Clean up the definition of m5.defines a little bit.

Use the new helper functions to go to/from a Value(), and tidy things up
slightly.

Change-Id: I9a31004b5a610bb8e94848d1fb88606dda6fc3c2
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48381
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
Reviewed-by: Hoa Nguyen 
---
M src/SConscript
1 file changed, 2 insertions(+), 5 deletions(-)

Approvals:
  Hoa Nguyen: Looks good to me, approved
  Bobby R. Bruce: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/SConscript b/src/SConscript
index c53567f..50633df 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -690,15 +690,12 @@
 # Generate Python file containing a dict specifying the current
 # buildEnv flags.
 def makeDefinesPyFile(target, source, env):
-build_env = source[0].get_contents().decode('utf-8')
-
 code = code_formatter()
-code("buildEnv = dict($build_env)")
+code("buildEnv = $0", FromValue(source[0]))
 code.write(target[0].abspath)

-defines_info = Value(build_env)
 # Generate a file with all of the compile options in it
-env.Command('python/m5/defines.py', defines_info,
+env.Command('python/m5/defines.py', ToValue(dict(build_env)),
 MakeAction(makeDefinesPyFile, Transform("DEFINES", 0)))
 PySource('m5', 'python/m5/defines.py')


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48381
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: I9a31004b5a610bb8e94848d1fb88606dda6fc3c2
Gerrit-Change-Number: 48381
Gerrit-PatchSet: 12
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Hoa Nguyen 
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]: tests: Add a test for KVM boot then switching cpus

2021-09-15 Thread Austin Harris (Gerrit) via gem5-dev
Austin Harris has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/50230 )


Change subject: tests: Add a test for KVM boot then switching cpus
..

tests: Add a test for KVM boot then switching cpus

A simple test for the switchable processor to boot linux, switch cpus,
then simulate to completion. The boot script runs m5 exit twice, first
to signal the test to switch cpus and then to actually exit.

Jira Issue: https://gem5.atlassian.net/browse/GEM5-1086

Change-Id: I9a7a6539b94b7b3f6d789ddf879d321613aef87a
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/50230
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
A tests/gem5/configs/components-library/boot_kvm_switch_exit.py
1 file changed, 250 insertions(+), 0 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/tests/gem5/configs/components-library/boot_kvm_switch_exit.py  
b/tests/gem5/configs/components-library/boot_kvm_switch_exit.py

new file mode 100644
index 000..d508e04
--- /dev/null
+++ b/tests/gem5/configs/components-library/boot_kvm_switch_exit.py
@@ -0,0 +1,250 @@
+# Copyright (c) 2021 The Univerity of Texas at Austin
+# All rights reserved.
+#
+# 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.
+
+"""
+This script boots with KVM then switches processors and exits.
+"""
+
+import argparse
+import os
+import sys
+
+import m5
+from m5.objects import Root
+
+# This is a lame hack to get the imports working correctly.
+# TODO: This needs fixed.
+sys.path.append(
+os.path.join(
+os.path.dirname(os.path.abspath(__file__)),
+os.pardir,
+os.pardir,
+os.pardir,
+os.pardir,
+)
+)
+
+from components_library.boards.x86_board import X86Board
+from components_library.coherence_protocol import CoherenceProtocol
+from components_library.isas import ISA
+from components_library.memory.single_channel import SingleChannelDDR3_1600
+from components_library.processors.cpu_types import CPUTypes
+from components_library.processors.simple_switchable_processor import (
+SimpleSwitchableProcessor,
+)
+from components_library.resources.resource import Resource
+from components_library.runtime import (
+get_runtime_coherence_protocol, get_runtime_isa
+)
+from components_library.utils.requires import requires
+
+parser = argparse.ArgumentParser(
+description="A script to test switching cpus. This test boots"
+"the linux kernel with the KVM cpu, then switches cpus until exiting."
+)
+parser.add_argument(
+"-m",
+"--mem-system",
+type=str,
+choices=("classic", "mi_example", "mesi_two_level"),
+required=True,
+help="The memory system.",
+)
+parser.add_argument(
+"-n",
+"--num-cpus",
+type=int,
+choices=(1, 2, 4, 8),
+required=True,
+help="The number of CPUs.",
+)
+parser.add_argument(
+"-c",
+"--cpu",
+type=str,
+choices=("kvm", "atomic", "timing", "o3"),
+required=True,
+help="The CPU type.",
+)
+parser.add_argument(
+"-r",
+"--resource-directory",
+type=str,
+required=False,
+help="The directory in which resources will be downloaded or exist.",
+)
+parser.add_argument(
+"-o",
+"--override-download",
+action="store_true",
+help="Override a local resource if the hashes do not match.",
+)
+parser.add_argument(
+"-k",
+"--kernel-args",
+type=str,
+

[gem5-dev] Change in gem5/gem5[develop]: cons: Allow clean non-interactive builds

2021-09-15 Thread Eric Ye (Gerrit) via gem5-dev
Eric Ye has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/50410 )



Change subject: cons: Allow clean non-interactive builds
..

cons: Allow clean non-interactive builds

On a clean build, the git tool will wait for input() before installing
git hooks. Allow bypassing this via a command-line flag, making it
possible to perform a clean build non-interactively.

Bug: 199780674
Test: build_gem5 --install_hooks
Change-Id: I48be2c1a7c2335a2f4f6359adf582ca8b0ae5939
---
M SConstruct
M site_scons/site_tools/git.py
2 files changed, 11 insertions(+), 5 deletions(-)



diff --git a/SConstruct b/SConstruct
index 4e2ed47..aae68e2 100755
--- a/SConstruct
+++ b/SConstruct
@@ -123,6 +123,8 @@
   help='Build with Address Sanitizer if available')
 AddOption('--with-systemc-tests', action='store_true',
   help='Build systemc tests')
+AddOption('--install_hooks', action='store_true',
+  help='Install git hooks non-interactively')

 # Imports of gem5_scons happen here since it depends on some options which  
are

 # declared above.
diff --git a/site_scons/site_tools/git.py b/site_scons/site_tools/git.py
index 3a71c9f..8236bde 100644
--- a/site_scons/site_tools/git.py
+++ b/site_scons/site_tools/git.py
@@ -42,6 +42,7 @@
 import sys

 import gem5_scons.util
+import SCons.Script

 git_style_message = """
 You're missing the gem5 style or commit message hook. These hooks help
@@ -99,11 +100,14 @@
 return

 print(git_style_message, end=' ')
-try:
-input()
-except:
-print("Input exception, exiting scons.\n")
-sys.exit(1)
+if SCons.Script.GetOption('install_hooks'):
+print("Got install_hooks flags so installing hooks  
non-interactively.")

+else:
+try:
+input()
+except:
+print("Input exception, exiting scons.\n")
+sys.exit(1)

 git_style_script = env.Dir("#util").File("git-pre-commit.py")
 git_msg_script = env.Dir("#ext").File("git-commit-msg")

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50410
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: I48be2c1a7c2335a2f4f6359adf582ca8b0ae5939
Gerrit-Change-Number: 50410
Gerrit-PatchSet: 1
Gerrit-Owner: Eric Ye 
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]: scons: Re-enable TRACING_ON flag

2021-09-15 Thread Richard Cooper (Gerrit) via gem5-dev
Richard Cooper has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/50427 )



Change subject: scons: Re-enable TRACING_ON flag
..

scons: Re-enable TRACING_ON flag

The TRACING_ON flag was removed in a previous commit [1], but is still
used by the _check_tracing() function in main.py. This breaks gem5
simulations when debug flags are enabled.

This patch re-enables the TRACING_ON flag.

[1] https://gem5-review.googlesource.com/c/public/gem5/+/48379

Change-Id: I90ed8a46938fa2748b96c1b378329a4ba1ef047e
---
M src/python/m5/main.py
M src/python/pybind11/core.cc
2 files changed, 4 insertions(+), 2 deletions(-)



diff --git a/src/python/m5/main.py b/src/python/m5/main.py
index a4af295..92878af 100644
--- a/src/python/m5/main.py
+++ b/src/python/m5/main.py
@@ -205,9 +205,9 @@


 def _check_tracing():
-from . import defines
+import _m5.core

-if defines.TRACING_ON:
+if _m5.core.TRACING_ON:
 return

 fatal("Tracing is not enabled.  Compile with TRACING_ON")
diff --git a/src/python/pybind11/core.cc b/src/python/pybind11/core.cc
index 965160f..6fe2efd 100644
--- a/src/python/pybind11/core.cc
+++ b/src/python/pybind11/core.cc
@@ -289,6 +289,8 @@
 m_core.attr("compileDate") = py::cast(compileDate);
 m_core.attr("gem5Version") = py::cast(gem5Version);

+m_core.attr("TRACING_ON") = py::cast(TRACING_ON);
+
 m_core.attr("MaxTick") = py::cast(MaxTick);

 /*

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50427
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: I90ed8a46938fa2748b96c1b378329a4ba1ef047e
Gerrit-Change-Number: 50427
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Cooper 
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]: cpu-kvm: Reinitialize threads on drainResume

2021-09-15 Thread Andreas Sandberg (Gerrit) via gem5-dev
Andreas Sandberg has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/50409 )



Change subject: cpu-kvm: Reinitialize threads on drainResume
..

cpu-kvm: Reinitialize threads on drainResume

Event queue service threads may have been re-created while the
simulator was drained. We therefore need to initialize the new thread
by setting correct signal masks and re-attaching performance counters.

Change-Id: Ic0dab80543928327021cade037770c917e73a47f
Signed-off-by: Andreas Sandberg 
---
M src/cpu/kvm/base.cc
M src/cpu/kvm/base.hh
2 files changed, 21 insertions(+), 8 deletions(-)



diff --git a/src/cpu/kvm/base.cc b/src/cpu/kvm/base.cc
index c7c72a8..ea43595 100644
--- a/src/cpu/kvm/base.cc
+++ b/src/cpu/kvm/base.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2015, 2017 ARM Limited
+ * Copyright (c) 2012, 2015, 2017, 2021 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -154,9 +154,9 @@
 inform("KVM: Coalesced not supported by host OS\n");
 }

-Event *startupEvent(
-new EventFunctionWrapper([this]{ startupThread(); }, name(),  
true));

-schedule(startupEvent, curTick());
+schedule(new EventFunctionWrapper([this]{
+restartEqThread();
+}, name(), true), curTick());
 }

 BaseKvmCPU::Status
@@ -228,7 +228,7 @@
 }

 void
-BaseKvmCPU::startupThread()
+BaseKvmCPU::restartEqThread()
 {
 // Do thread-specific initialization. We need to setup signal
 // delivery for counters and timers from within the thread that
@@ -381,6 +381,13 @@
 {
 assert(!tickEvent.scheduled());

+/* The simulator may have terminated the threads servicing event
+ * queues. In that case, we need to re-initialize the new
+ * threads. */
+schedule(new EventFunctionWrapper([this]{
+restartEqThread();
+}, name(), true), curTick());
+
 // We might have been switched out. In that case, we don't need to
 // do anything.
 if (switchedOut())
@@ -1275,6 +1282,11 @@
 .samplePeriod(42);
 }

+// We might be re-attaching counters due threads being
+// re-initialised after fork.
+if (hwCycles.attached())
+hwCycles.detach();
+
 hwCycles.attach(cfgCycles,
 0); // TID (0 => currentThread)

diff --git a/src/cpu/kvm/base.hh b/src/cpu/kvm/base.hh
index e5b047e..4f40064 100644
--- a/src/cpu/kvm/base.hh
+++ b/src/cpu/kvm/base.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012 ARM Limited
+ * Copyright (c) 2012, 2021 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -682,11 +682,12 @@
  * example, when setting up timers, we need to know the TID of the
  * thread executing in KVM in order to deliver the timer signal to
  * that thread. This method is called as the first event in this
- * SimObject's event queue.
+ * SimObject's event queue and after drainResume to handle changes
+ * to event queue service threads.
  *
  * @see startup
  */
-void startupThread();
+void restartEqThread();

 /** Try to drain the CPU if a drain is pending */
 bool tryDrain();

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50409
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: Ic0dab80543928327021cade037770c917e73a47f
Gerrit-Change-Number: 50409
Gerrit-PatchSet: 1
Gerrit-Owner: Andreas Sandberg 
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]: python: Remove unnecessary Python 2.x workaround

2021-09-15 Thread Andreas Sandberg (Gerrit) via gem5-dev
Andreas Sandberg has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/50407 )



Change subject: python: Remove unnecessary Python 2.x workaround
..

python: Remove unnecessary Python 2.x workaround

We needed to explicitly cast the return value from getCode() to int to
avoid a Python 2.x issue where sys.exit() got confused by an
unexpected long argument. This isn't an issue in Python 3 since long
has been removed as a separate type.

Change-Id: I7770d0f180e826ac7e6c92c13bc6a61447e3f851
Signed-off-by: Andreas Sandberg 
---
M src/python/pybind11/event.cc
1 file changed, 0 insertions(+), 11 deletions(-)



diff --git a/src/python/pybind11/event.cc b/src/python/pybind11/event.cc
index 794b6e3..aefe50a 100644
--- a/src/python/pybind11/event.cc
+++ b/src/python/pybind11/event.cc
@@ -134,18 +134,7 @@
std::unique_ptr>(
m, "GlobalSimLoopExitEvent")
 .def("getCause", ::getCause)
-#if PY_MAJOR_VERSION >= 3
 .def("getCode", ::getCode)
-#else
-// Workaround for an issue where PyBind11 converts the exit
-// code to a long. This is normally fine, but sys.exit treats
-// any non-int type as an error and exits with status 1 if it
-// is passed a long.
-.def("getCode", [](GlobalSimLoopExitEvent *e) {
-return py::reinterpret_steal(
-PyInt_FromLong(e->getCode()));
-})
-#endif
 ;

 // Event base class. These should never be returned directly to

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50407
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: I7770d0f180e826ac7e6c92c13bc6a61447e3f851
Gerrit-Change-Number: 50407
Gerrit-PatchSet: 1
Gerrit-Owner: Andreas Sandberg 
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: Fix fork for multithreaded simulations

2021-09-15 Thread Andreas Sandberg (Gerrit) via gem5-dev
Andreas Sandberg has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/50408 )



Change subject: sim: Fix fork for multithreaded simulations
..

sim: Fix fork for multithreaded simulations

It is currently not possible to call m5.fork when the simulator is
running in with multiple parallel event queues. The POSIX standard
have very weak guarantees when forking a process with multiple
threads. In order to use fork correctly, we need to ensure that all
helper threads servicing event queues have terminated before the fork
system call is invoked.

There are two ways this could be implemented: 1) Always terminate
helper threads when taking a global simulator exit event, or 2)
terminate helper threads just before fork is called from Python.

This change implements the second strategy since the KVM-based CPUs
currently assume that TIDs don't change unless there is a fork event.

Change-Id: I22feaecd49f7f81689b43185d63a8f14428bed63
Signed-off-by: Andreas Sandberg 
---
M src/python/m5/simulate.py
M src/python/pybind11/event.cc
M src/sim/simulate.cc
M src/sim/simulate.hh
4 files changed, 90 insertions(+), 14 deletions(-)



diff --git a/src/python/m5/simulate.py b/src/python/m5/simulate.py
index 66e6a08..b5b8c78 100644
--- a/src/python/m5/simulate.py
+++ b/src/python/m5/simulate.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2012,2019 ARM Limited
+# Copyright (c) 2012, 2019, 2021 Arm Limited
 # All rights reserved.
 #
 # The license below extends only to copyright in the software and shall
@@ -348,6 +348,9 @@

 drain()

+# Terminate helper threads that service parallel event queues.
+_m5.event.terminateEventQueueThreads()
+
 try:
 pid = os.fork()
 except OSError as e:
diff --git a/src/python/pybind11/event.cc b/src/python/pybind11/event.cc
index aefe50a..7a02221 100644
--- a/src/python/pybind11/event.cc
+++ b/src/python/pybind11/event.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017 ARM Limited
+ * Copyright (c) 2017, 2021 Arm Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -107,6 +107,7 @@

 m.def("simulate", ,
   py::arg("ticks") = MaxTick);
+m.def("terminateEventQueueThreads", );
 m.def("exitSimLoop", );
 m.def("getEventQueue", []() { return curEventQueue(); },
   py::return_value_policy::reference);
diff --git a/src/sim/simulate.cc b/src/sim/simulate.cc
index 4a00869..a87bd04 100644
--- a/src/sim/simulate.cc
+++ b/src/sim/simulate.cc
@@ -1,4 +1,16 @@
 /*
+ * Copyright (c) 2021 Arm Limited
+ * All rights reserved
+ *
+ * The license below extends only to copyright in the software and shall
+ * not be construed as granting a license to any other intellectual
+ * property including but not limited to intellectual property relating
+ * to a hardware implementation of the functionality of the software
+ * licensed hereunder.  You may use the software subject to the license
+ * terms below provided that you ensure that this notice is replicated
+ * unmodified and in its entirety in all distributions of the software,
+ * modified or unmodified, in source code or in binary form.
+ *
  * Copyright (c) 2006 The Regents of The University of Michigan
  * Copyright (c) 2013 Advanced Micro Devices, Inc.
  * Copyright (c) 2013 Mark D. Hill and David A. Wood
@@ -30,6 +42,7 @@

 #include "sim/simulate.hh"

+#include 
 #include 
 #include 

@@ -46,11 +59,15 @@
 {

 //! Mutex for handling async events.
-std::mutex asyncEventMutex;
+static std::mutex asyncEventMutex;

 //! Global barrier for synchronizing threads entering/exiting the
 //! simulation loop.
-Barrier *threadBarrier;
+static Barrier *threadBarrier;
+
+static std::atomic terminateEqThreads = false;
+
+static std::vector eventThreads;

 //! forward declaration
 Event *doSimLoop(EventQueue *);
@@ -66,9 +83,12 @@
 static void
 thread_loop(EventQueue *queue)
 {
-while (true) {
-threadBarrier->wait();
+/* Wait for all initialisation to complete */
+threadBarrier->wait();
+
+while (!terminateEqThreads) {
 doSimLoop(queue);
+threadBarrier->wait();
 }
 }

@@ -86,18 +106,14 @@
 // create a thread for each of event queues referenced by the
 // instantiated sim objects.
 static bool threads_initialized = false;
-static std::vector threads;
+
+/* terminateEqThreads is initialised to false and should only be
+ * set to true temporarily in terminateEventQueueThreads. */
+assert(!terminateEqThreads);

 if (!threads_initialized) {
 threadBarrier = new Barrier(numMainEventQueues);

-// the main thread (the one we're currently running on)
-// handles queue 0, so we only need to allocate new threads
-// for queues 1..N-1.  We'll call these the "subordinate" threads.
-for (uint32_t i = 1; i < numMainEventQueues; i++) {
-threads.push_back(new 

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Refactor AArch64 MMU permission check

2021-09-15 Thread Giacomo Travaglini (Gerrit) via gem5-dev
Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/50387 )



Change subject: arch-arm: Refactor AArch64 MMU permission check
..

arch-arm: Refactor AArch64 MMU permission check

This refactor of the MMU::checkPermissions64 method is moving
the TLB entry access permission bits (AP,XN...) checking into
new separate stage1 and stage2 helpers

Change-Id: If7d42538f5ea1ec21e918cccaf469fcb6a36d82b
Signed-off-by: Giacomo Travaglini 
---
M src/arch/arm/mmu.cc
M src/arch/arm/mmu.hh
2 files changed, 206 insertions(+), 143 deletions(-)



diff --git a/src/arch/arm/mmu.cc b/src/arch/arm/mmu.cc
index d2fc706..9479f9d 100644
--- a/src/arch/arm/mmu.cc
+++ b/src/arch/arm/mmu.cc
@@ -453,7 +453,6 @@
 // Cache clean operations require read permissions to the specified VA
 bool is_write = !req->isCacheClean() && mode == Write;
 bool is_atomic = req->isAtomic();
-[[maybe_unused]] bool is_priv = state.isPriv && !(flags & UserMode);

 updateMiscReg(tc, state.curTranType, state.isStage2);

@@ -497,161 +496,27 @@
 }
 }

-uint8_t ap  = 0x3 & (te->ap);  // 2-bit access protection field
 bool grant = false;
-
-bool wxn = state.sctlr.wxn;
-uint8_t xn =  te->xn;
-uint8_t pxn = te->pxn;
-bool r = (!is_write && !is_fetch);
-bool w = is_write;
-bool x = is_fetch;
-
-if (ArmSystem::haveEL(tc, EL3) && state.isSecure &&
-te->ns && state.scr.sif) {
-xn = true;
-}
-
 // grant_read is used for faults from an atomic instruction that
 // both reads and writes from a memory location. From a ISS point
 // of view they count as read if a read to that address would have
 // generated the fault; they count as writes otherwise
 bool grant_read = true;
-DPRINTF(TLBVerbose, "Checking permissions: ap:%d, xn:%d, pxn:%d,  
r:%d, "

-"w:%d, x:%d, is_priv: %d, wxn: %d\n", ap, xn,
-pxn, r, w, x, is_priv, wxn);

 if (state.isStage2) {
-assert(ArmSystem::haveVirtualization(tc) && state.aarch64EL !=  
EL2);

-// In stage 2 we use the hypervisor access permission bits.
-// The following permissions are described in ARM DDI 0487A.f
-// D4-1802
-uint8_t hap = 0x3 & te->hap;
-grant_read = hap & 0x1;
-if (is_fetch) {
-// sctlr.wxn overrides the xn bit
-grant = !wxn && !xn;
-} else if (is_atomic) {
-grant = hap;
-} else if (is_write) {
-grant = hap & 0x2;
-} else { // is_read
-grant = grant_read;
-}
+std::tie(grant, grant_read) = s2PermBits64(te, req, mode, tc,  
state,

+(!is_write && !is_fetch), is_write, is_fetch);
 } else {
-switch (state.aarch64EL) {
-  case EL0:
-{
-grant_read = ap & 0x1;
-uint8_t perm = (ap << 2)  | (xn << 1) | pxn;
-switch (perm) {
-  case 0:
-  case 1:
-  case 8:
-  case 9:
-grant = x;
-break;
-  case 4:
-  case 5:
-grant = r || w || (x && !wxn);
-break;
-  case 6:
-  case 7:
-grant = r || w;
-break;
-  case 12:
-  case 13:
-grant = r || x;
-break;
-  case 14:
-  case 15:
-grant = r;
-break;
-  default:
-grant = false;
-}
-}
-break;
-  case EL1:
-{
-if (checkPAN(tc, ap, req, mode, is_priv, state)) {
-grant = false;
-grant_read = false;
-break;
-}
-
-uint8_t perm = (ap << 2)  | (xn << 1) | pxn;
-switch (perm) {
-  case 0:
-  case 2:
-grant = r || w || (x && !wxn);
-break;
-  case 1:
-  case 3:
-  case 4:
-  case 5:
-  case 6:
-  case 7:
-// regions that are writeable at EL0 should not be
-// executable at EL1
-grant = r || w;
-break;
-  case 8:
-  case 10:
-  case 12:
-  case 14:
-grant = r || x;
-break;
-  case 9:
-  case 11:
-  case 13:
-  case 15:
-grant = r;
-

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Remove SPSR write mask

2021-09-15 Thread Giacomo Travaglini (Gerrit) via gem5-dev
Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/50389 )



Change subject: arch-arm: Remove SPSR write mask
..

arch-arm: Remove SPSR write mask

We are currently masking out the PAN and UAO field when writing
to the SPSR_ELx register.
This is not needed and we should treat them as RES0 instead if
FEAT_PAN and FEAT_UAO are not implemented

Change-Id: Ib65e3744f365825d2414b8092b3a79324be461b4
Signed-off-by: Giacomo Travaglini 
---
M src/arch/arm/isa.cc
1 file changed, 0 insertions(+), 10 deletions(-)



diff --git a/src/arch/arm/isa.cc b/src/arch/arm/isa.cc
index 51856ca..52042e2 100644
--- a/src/arch/arm/isa.cc
+++ b/src/arch/arm/isa.cc
@@ -2302,16 +2302,6 @@
   case MISCREG_AT_S1E3W_Xt:
 addressTranslation64(MMU::S1E3Tran, BaseMMU::Write, 0, val);
 return;
-  case MISCREG_SPSR_EL3:
-  case MISCREG_SPSR_EL2:
-  case MISCREG_SPSR_EL1:
-{
-RegVal spsr_mask = havePAN ?
-~(0x2 << 22) : ~(0x3 << 22);
-
-newVal = val & spsr_mask;
-break;
-}
   case MISCREG_L2CTLR:
 warn("miscreg L2CTLR (%s) written with %#x. ignored...\n",
  miscRegName[misc_reg], uint32_t(val));

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50389
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: Ib65e3744f365825d2414b8092b3a79324be461b4
Gerrit-Change-Number: 50389
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini 
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]: arch-arm: Implement Armv8.2 FEAT_UAO

2021-09-15 Thread Giacomo Travaglini (Gerrit) via gem5-dev
Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/50390 )



Change subject: arch-arm: Implement Armv8.2 FEAT_UAO
..

arch-arm: Implement Armv8.2 FEAT_UAO

Change-Id: I87b25a65e706ed6486347806a540b1dbf25231cb
Signed-off-by: Giacomo Travaglini 
---
M src/arch/arm/ArmISA.py
M src/arch/arm/faults.cc
M src/arch/arm/insts/misc64.cc
M src/arch/arm/insts/static_inst.cc
M src/arch/arm/isa.cc
M src/arch/arm/isa/formats/aarch64.isa
M src/arch/arm/regs/misc.cc
M src/arch/arm/regs/misc.hh
M src/arch/arm/regs/misc_types.hh
M src/arch/arm/utility.cc
10 files changed, 40 insertions(+), 6 deletions(-)



diff --git a/src/arch/arm/ArmISA.py b/src/arch/arm/ArmISA.py
index 55dcdf4..23338d1 100644
--- a/src/arch/arm/ArmISA.py
+++ b/src/arch/arm/ArmISA.py
@@ -116,8 +116,8 @@
 # PAN | HPDS | !VHE | VMIDBits
 id_aa64mmfr1_el1 = Param.UInt64(0x00101020,
 "AArch64 Memory Model Feature Register 1")
-# |VARANGE
-id_aa64mmfr2_el1 = Param.UInt64(0x0001,
+# |VARANGE | UAO
+id_aa64mmfr2_el1 = Param.UInt64(0x00010010,
 "AArch64 Memory Model Feature Register 2")

 # Any access (read/write) to an unimplemented
diff --git a/src/arch/arm/faults.cc b/src/arch/arm/faults.cc
index 102ce84..9f82e50 100644
--- a/src/arch/arm/faults.cc
+++ b/src/arch/arm/faults.cc
@@ -677,6 +677,7 @@
 ITSTATE it = tc->pcState().itstate();
 spsr.it2 = it.top6;
 spsr.it1 = it.bottom2;
+spsr.uao = 0;
 }
 tc->setMiscReg(spsr_idx, spsr);

@@ -701,6 +702,7 @@
 cpsr.il = 0;
 cpsr.ss = 0;
 cpsr.pan = span ? 1 : spsr.pan;
+cpsr.uao = 0;
 tc->setMiscReg(MISCREG_CPSR, cpsr);

 // If we have a valid instruction then use it to annotate this fault  
with

diff --git a/src/arch/arm/insts/misc64.cc b/src/arch/arm/insts/misc64.cc
index 0f301b0..4cdd3dd 100644
--- a/src/arch/arm/insts/misc64.cc
+++ b/src/arch/arm/insts/misc64.cc
@@ -800,11 +800,14 @@
 RegVal
 MiscRegImmOp64::miscRegImm() const
 {
-if (dest == MISCREG_SPSEL) {
+switch (dest) {
+  case MISCREG_SPSEL:
 return imm & 0x1;
-} else if (dest == MISCREG_PAN) {
+  case MISCREG_PAN:
 return (imm & 0x1) << 22;
-} else {
+  case MISCREG_UAO:
+return (imm & 0x1) << 23;
+  default:
 panic("Not a valid PSTATE field register\n");
 }
 }
diff --git a/src/arch/arm/insts/static_inst.cc  
b/src/arch/arm/insts/static_inst.cc

index d6ca535..b89acbc 100644
--- a/src/arch/arm/insts/static_inst.cc
+++ b/src/arch/arm/insts/static_inst.cc
@@ -1192,6 +1192,7 @@
 } else {
 // aarch64
 new_cpsr.daif = spsr.daif;
+new_cpsr.uao = spsr.uao;
 }

 SelfDebug *sd = ArmISA::ISA::getSelfDebug(tc);
diff --git a/src/arch/arm/isa.cc b/src/arch/arm/isa.cc
index 52042e2..f9ac845 100644
--- a/src/arch/arm/isa.cc
+++ b/src/arch/arm/isa.cc
@@ -773,6 +773,10 @@
 {
 return miscRegs[MISCREG_CPSR] & 0x40;
 }
+  case MISCREG_UAO:
+{
+return miscRegs[MISCREG_CPSR] & 0x80;
+}
   case MISCREG_L2CTLR:
 {
 // mostly unimplemented, just set NumCPUs field from sim and  
return

@@ -2262,6 +2266,17 @@
 misc_reg = MISCREG_CPSR;
 }
 break;
+  case MISCREG_UAO:
+{
+// UAO is affecting data accesses
+getMMUPtr(tc)->invalidateMiscReg(MMU::D_TLBS);
+
+CPSR cpsr = miscRegs[MISCREG_CPSR];
+cpsr.uao = (uint8_t) ((CPSR) newVal).uao;
+newVal = cpsr;
+misc_reg = MISCREG_CPSR;
+}
+break;
   case MISCREG_AT_S1E1R_Xt:
 addressTranslation64(MMU::S1E1Tran, BaseMMU::Read, 0, val);
 return;
diff --git a/src/arch/arm/isa/formats/aarch64.isa  
b/src/arch/arm/isa/formats/aarch64.isa

index a508b30..0c67645 100644
--- a/src/arch/arm/isa/formats/aarch64.isa
+++ b/src/arch/arm/isa/formats/aarch64.isa
@@ -413,6 +413,10 @@
 // MSR immediate: moving immediate value to  
selected

 // bits of the PSTATE
 switch (op1 << 3 | op2) {
+  case 0x3:
+// UAO
+return new MsrImm64(
+machInst, MISCREG_UAO, crm);
   case 0x4:
 // PAN
 return new MsrImm64(
diff --git a/src/arch/arm/regs/misc.cc b/src/arch/arm/regs/misc.cc
index 3d679bb..4a94a3e 100644
--- a/src/arch/arm/regs/misc.cc
+++ b/src/arch/arm/regs/misc.cc
@@ -2451,6 +2451,8 @@
 return MISCREG_CURRENTEL;
   case 3:
 return MISCREG_PAN;
+  case 4:
+   

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Use EL0 permission logic when checking unpriv reference

2021-09-15 Thread Giacomo Travaglini (Gerrit) via gem5-dev
Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/50388 )



Change subject: arch-arm: Use EL0 permission logic when checking unpriv  
reference

..

arch-arm: Use EL0 permission logic when checking unpriv reference

An unprivileged memory reference (LDTR and STTR) executed at EL1
should be regarded as an EL0 memory access

Change-Id: Iae5e6e582f9c5a57340f27a84db463bcb8996922
Signed-off-by: Giacomo Travaglini 
---
M src/arch/arm/mmu.cc
1 file changed, 2 insertions(+), 1 deletion(-)



diff --git a/src/arch/arm/mmu.cc b/src/arch/arm/mmu.cc
index 9479f9d..51780f3 100644
--- a/src/arch/arm/mmu.cc
+++ b/src/arch/arm/mmu.cc
@@ -604,7 +604,8 @@
 return std::make_pair(false, false);
 }

-switch (state.aarch64EL) {
+ExceptionLevel regime = !is_priv ? EL0 : state.aarch64EL;
+switch (regime) {
   case EL0:
 {
 grant_read = ap & 0x1;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50388
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: Iae5e6e582f9c5a57340f27a84db463bcb8996922
Gerrit-Change-Number: 50388
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini 
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]: mem: Make ruby AbstractController compatible with XBar

2021-09-15 Thread Giacomo Travaglini (Gerrit) via gem5-dev
Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/50367 )



Change subject: mem: Make ruby AbstractController compatible with XBar
..

mem: Make ruby AbstractController compatible with XBar

At the moment the ruby AbstractController is trying to re-send the same
memory request every clock cycle until it finally succeeds [1]
(in other words it is not waiting for a recvReqRetry from the peer
port)

This polling behaviour is not compatible with the gem5 XBar, which is
panicking if it receives two consecutive requests to the same BUSY
layer [2]

This patch is fixing the incompatibility by inhibiting the
AbstractController retry until it gets a notification from the peer
response port

[1]: https://github.com/gem5/gem5/blob/v21.1.0.1/\
src/mem/ruby/slicc_interface/AbstractController.cc#L303
[2]: https://github.com/gem5/gem5/blob/v21.1.0.1/src/mem/xbar.cc#L196

Change-Id: I0ac38ce286051fb714844de569c2ebf85e71a523
Signed-off-by: Giacomo Travaglini 
---
M src/mem/ruby/slicc_interface/AbstractController.cc
M src/mem/ruby/slicc_interface/AbstractController.hh
2 files changed, 5 insertions(+), 1 deletion(-)



diff --git a/src/mem/ruby/slicc_interface/AbstractController.cc  
b/src/mem/ruby/slicc_interface/AbstractController.cc

index c7f22a6..396b128 100644
--- a/src/mem/ruby/slicc_interface/AbstractController.cc
+++ b/src/mem/ruby/slicc_interface/AbstractController.cc
@@ -61,6 +61,7 @@
   m_transitions_per_cycle(p.transitions_per_cycle),
   m_buffer_size(p.buffer_size), m_recycle_latency(p.recycle_latency),
   m_mandatory_queue_latency(p.mandatory_queue_latency),
+  m_waiting_mem_retry(false),
   memoryPort(csprintf("%s.memory", name()), this),
   addrRanges(p.addr_ranges.begin(), p.addr_ranges.end()),
   stats(this)
@@ -255,7 +256,7 @@
 {
 auto mem_queue = getMemReqQueue();
 assert(mem_queue);
-if (!mem_queue->isReady(clockEdge())) {
+if (m_waiting_mem_retry || !mem_queue->isReady(clockEdge())) {
 return false;
 }

@@ -301,6 +302,7 @@
 scheduleEvent(Cycles(1));
 } else {
 scheduleEvent(Cycles(1));
+m_waiting_mem_retry = true;
 delete pkt;
 delete s;
 }
@@ -441,6 +443,7 @@
 void
 AbstractController::MemoryPort::recvReqRetry()
 {
+controller->m_waiting_mem_retry = false;
 controller->serviceMemoryQueue();
 }

diff --git a/src/mem/ruby/slicc_interface/AbstractController.hh  
b/src/mem/ruby/slicc_interface/AbstractController.hh

index 3fe2205..56c164f 100644
--- a/src/mem/ruby/slicc_interface/AbstractController.hh
+++ b/src/mem/ruby/slicc_interface/AbstractController.hh
@@ -328,6 +328,7 @@
 const unsigned int m_buffer_size;
 Cycles m_recycle_latency;
 const Cycles m_mandatory_queue_latency;
+bool m_waiting_mem_retry;

 /**
  * Port that forwards requests and receives responses from the

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50367
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: I0ac38ce286051fb714844de569c2ebf85e71a523
Gerrit-Change-Number: 50367
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini 
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