[gem5-dev] Change in gem5/gem5[develop]: cpu-o3: Avoid passing ReExec 'faults' on CPU tracing interface

[Giacomo Travaglini (Gerrit) via gem5-dev]

Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/30554 )



Change subject: cpu-o3: Avoid passing ReExec 'faults' on CPU tracing  
interface

..

cpu-o3: Avoid passing ReExec 'faults' on CPU tracing interface

The O3 model uses ReExec faults to flush the pipeline and restart
after a memory ordering violation, e.g. due to an incoming snoop.

These, just like branch mispredict flushes, are not architectural
faults but micro-architectural events, and should therefore not
show up on the instruction tracing interface.

This adds a check on faulting instructions in commit, to verify
if the instruction faulted due to ReExec, to avoid tracing it.

Change-Id: I1d3eaffb0ff22411e0e16a69ef07961924c88c10
---
M src/cpu/o3/commit_impl.hh
1 file changed, 5 insertions(+), 1 deletion(-)



diff --git a/src/cpu/o3/commit_impl.hh b/src/cpu/o3/commit_impl.hh
index 4f467e9..49b40e3 100644
--- a/src/cpu/o3/commit_impl.hh
+++ b/src/cpu/o3/commit_impl.hh
@@ -1259,7 +1259,11 @@
 "[tid:%i] [sn:%llu] Committing instruction with fault\n",
 tid, head_inst->seqNum);
 if (head_inst->traceData) {
-if (DTRACE(ExecFaulting)) {
+// We ignore ReExecution "faults" here as they are not real
+// (architectural) faults but signal flush/replays.
+if (DTRACE(ExecFaulting)
+&& dynamic_cast(inst_fault.get()) == nullptr) {
+
 head_inst->traceData->setFaulting(true);
 head_inst->traceData->setFetchSeq(head_inst->seqNum);
 head_inst->traceData->setCPSeq(thread[tid]->numOp);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30554
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: I1d3eaffb0ff22411e0e16a69ef07961924c88c10
Gerrit-Change-Number: 30554
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] Re: bug squashing renamed pinned registers in o3?

[Giacomo Travaglini via gem5-dev]

Hi Gabe,

We are encountering the same problem on top of develop but it’s still worth 
asking:


  1.  Do you have https://gem5-review.googlesource.com/c/public/gem5/+/25743 ?
  2.  Are you encountering this in a simulation which is using a CPU switch or 
checkpoint save/restore

Kind Regards

Giacomo

From: Gabe Black via gem5-dev 
Sent: 23 June 2020 06:24
To: gem5 Developer List 
Cc: Gabe Black 
Subject: [gem5-dev] bug squashing renamed pinned registers in o3?

Hi folks, specifically ARM folks. We've been seeing a problem with O3 where 
when switching vector register renaming modes (full vectors vs vector 
elements), the CPU checks its bookkeeping and finds that a vector register is 
missing, ie with no instructions in flight, the free list has one fewer 
register in it than the difference between the total number of physical vector 
registers, and the number that should be taken up with architectural state.

This problem has been somewhat difficult to reproduce, although we can get it 
to happen, and it does happen often enough that it's been a real pain for us. 
Given that it's not very easy to get it to happen which makes it hard to 
observe, I've been digging around in the code trying to understand what all the 
pieces do and why the bookkeeping might be wrong.

The most promising thing I've found so far is that when squashing, the rename 
stage looks at its history and rolls back renames for squashed instructions. 
Some registers are fixed and not renamed, so rolling back those would be 
pointless. Also those registers should not go on the free list.

The way O3 detects those special registers is that they have the same index 
before and after renaming. If that is the case, O3 ignores those entries, and 
does not roll them back or mark their target as free.

This check is slightly out of date though, since with the recently added pinned 
register writes, a register will be renamed to the same thing several times in 
a row. When these entries are checked, they will not be rolled back (I think 
this part is still fine), but they will also not be marked as free.

This isn't exactly a smoking gun though, since the more I think about it, the 
more I think this may actually be ok. If one of the later writes is squashed, 
the register isn't "free" since it still holds the (partially written) 
architectural state. If everything gets squashed all the way back to the first 
entry which did change what register to use, then the slightly outdated check 
won't trigger and things should be freed up correctly (I think).

This code is mostly new to me though, so I'm not super confident making any 
grand declarations about what's going on. All the pieces seem to be there 
though, which makes me very suspicious.

Maybe something goes wrong if the right number of writes never happens because 
later writers get squashed?

Gabe
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
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: TestStdout and TestStderr

[Giacomo Travaglini (Gerrit) via gem5-dev]

Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/30574 )



Change subject: ext: TestStdout and TestStderr
..

ext: TestStdout and TestStderr

This is actually not used

JIRA: https://gem5.atlassian.net/projects/GEM5/issues/GEM5-533

Change-Id: Ic2be44daf26fed4236647bca3f2e82ca950d7656
Signed-off-by: Giacomo Travaglini 
---
M ext/testlib/handlers.py
M ext/testlib/log.py
2 files changed, 0 insertions(+), 65 deletions(-)



diff --git a/ext/testlib/handlers.py b/ext/testlib/handlers.py
index 723a855..c8eab4c 100644
--- a/ext/testlib/handlers.py
+++ b/ext/testlib/handlers.py
@@ -49,43 +49,6 @@
 from six.moves import queue as Queue
 from testlib.configuration import constants

-
-class _TestStreamManager(object):
-def __init__(self):
-self._writers = {}
-
-def open_writer(self, test_result):
-if test_result in self._writers:
-raise ValueError('Cannot have multiple writters on a single  
test.')

-self._writers[test_result] = _TestStreams(test_result.stdout,
-test_result.stderr)
-
-def get_writer(self, test_result):
-if test_result not in self._writers:
-self.open_writer(test_result)
-return self._writers[test_result]
-
-def close_writer(self, test_result):
-if test_result in self._writers:
-writer = self._writers.pop(test_result)
-writer.close()
-
-def close(self):
-for writer in self._writers.values():
-writer.close()
-self._writers.clear()
-
-class _TestStreams(object):
-def __init__(self, stdout, stderr):
-helper.mkdir_p(os.path.dirname(stdout))
-helper.mkdir_p(os.path.dirname(stderr))
-self.stdout = open(stdout, 'w')
-self.stderr = open(stderr, 'w')
-
-def close(self):
-self.stdout.close()
-self.stderr.close()
-
 class ResultHandler(object):
 '''
 Log handler which listens for test results and output saving data as
@@ -113,9 +76,6 @@

 log.SuiteResult.type_id: self.handle_suite_result,
 log.TestResult.type_id: self.handle_test_result,
-
-log.TestStderr.type_id: self.handle_stderr,
-log.TestStdout.type_id: self.handle_stdout,
 }

 def handle(self, record):
@@ -135,16 +95,6 @@
 test_result = self._get_test_result(record)
 test_result.result = record['result']

-def handle_stderr(self, record):
-self.test_stream_manager.get_writer(
-self._get_test_result(record)
-).stderr.write(record['buffer'])
-
-def handle_stdout(self, record):
-self.test_stream_manager.get_writer(
-self._get_test_result(record)
-).stdout.write(record['buffer'])
-
 def _get_test_result(self, test_record):
 return self.internal_results.get_test_result(
 test_record['metadata'].uid,
@@ -263,8 +213,6 @@
 log.TestResult.type_id: self.handle_testresult,
 log.SuiteStatus.type_id: self.handle_suitestatus,
 log.TestStatus.type_id: self.handle_teststatus,
-log.TestStderr.type_id: self.handle_stderr,
-log.TestStdout.type_id: self.handle_stdout,
 log.TestMessage.type_id: self.handle_testmessage,
 log.LibraryMessage.type_id: self.handle_librarymessage,
 }
@@ -298,14 +246,6 @@
   log.test_log.debug('Starting Test Suite: %s ' %\
 record['metadata'].name)

-def handle_stderr(self, record):
-if self.stream:
-print(record.data['buffer'], file=sys.stderr, end='')
-
-def handle_stdout(self, record):
-if self.stream:
-print(record.data['buffer'], file=sys.stdout, end='')
-
 def handle_testmessage(self, record):
 if self.stream:
 print(self._colorize(record['message'], record['level']))
diff --git a/ext/testlib/log.py b/ext/testlib/log.py
index 1bdb373..0c19928 100644
--- a/ext/testlib/log.py
+++ b/ext/testlib/log.py
@@ -97,11 +97,6 @@
 pass
 class LibraryResult(ResultRecord):
 pass
-# Test Output Types
-class TestStderr(Record):
-pass
-class TestStdout(Record):
-pass
 # Message (Raw String) Types
 class TestMessage(Record):
 pass

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30574
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: Ic2be44daf26fed4236647bca3f2e82ca950d7656
Gerrit-Change-Number: 30574
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]: dev-arm: Verify number of CPUs when restoring Generic Timer Cpts.

[Richard Cooper (Gerrit) via gem5-dev]

Hello Giacomo Travaglini,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: dev-arm: Verify number of CPUs when restoring Generic Timer  
Cpts.

..

dev-arm: Verify number of CPUs when restoring Generic Timer Cpts.

When restoring a checkpoint containing a generic timer, the checkpoint
expects to connect the timer to the same number of CPUs that were
present when the checkpoint was taken. If the number of CPUs in the
new simulation is different, deserialization will fail. In the case
that the number of CPUs expected by the checkpoint is greater than the
number of CPUs present, this will cause a segmentation fault caused by
reading off the end of the list of Thread Contexts.

This commit fixes the problem by checking the number of CPUs present
in the simulation matches the number of CPUs expected by the generic
timer checkpoint. If there is a mismatch, a fatal error is triggered
with an informative message to the user.

Change-Id: Iff9ad68d64e67b3df51682b7e4e272e5f355bcd6
Reviewed-by: Giacomo Travaglini 
---
M src/dev/arm/generic_timer.cc
1 file changed, 7 insertions(+), 0 deletions(-)



diff --git a/src/dev/arm/generic_timer.cc b/src/dev/arm/generic_timer.cc
index 4f92dac..1701ccd 100644
--- a/src/dev/arm/generic_timer.cc
+++ b/src/dev/arm/generic_timer.cc
@@ -415,6 +415,13 @@
 cpu_count = OLD_CPU_MAX;
 }

+if (cpu_count != system.numContexts()) {
+fatal("The simulated system has been initialized with %d CPUs, "
+  "but the Generic Timer checkpoint expects %d CPUs. Consider "
+  "restoring the checkpoint specifying %d CPUs.",
+  system.numContexts(), cpu_count, cpu_count);
+}
+
 for (int i = 0; i < cpu_count; ++i) {
 CoreTimers &core(getTimers(i));
 core.unserializeSection(cp, csprintf("pe_implementation%d", i));

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30576
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: Iff9ad68d64e67b3df51682b7e4e272e5f355bcd6
Gerrit-Change-Number: 30576
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Cooper 
Gerrit-Reviewer: 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]: configs: Updated DTB warnings in fs.py for Arm platforms.

[Richard Cooper (Gerrit) via gem5-dev]

Hello Giacomo Travaglini,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: configs: Updated DTB warnings in fs.py for Arm platforms.
..

configs: Updated DTB warnings in fs.py for Arm platforms.

fs.py warns when an Arm platform is being created without a DTB file,
if the platform does not support the automatic creation of a DTB.

Updated the list of supported platforms with recent additions in order
to remove incorrect and potentially confusing warnings.

Change-Id: I549124a1afbc36e313f614dccab17973582bc3f7
Reviewed-by: Giacomo Travaglini 
---
M configs/example/fs.py
1 file changed, 6 insertions(+), 3 deletions(-)



diff --git a/configs/example/fs.py b/configs/example/fs.py
index 6643d35..d39feee 100644
--- a/configs/example/fs.py
+++ b/configs/example/fs.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2010-2013, 2016, 2019 ARM Limited
+# Copyright (c) 2010-2013, 2016, 2019-2020 ARM Limited
 # Copyright (c) 2020 Barkhausen Institut
 # All rights reserved.
 #
@@ -366,8 +366,11 @@

 if buildEnv['TARGET_ISA'] == "arm" and not options.bare_metal \
 and not options.dtb_filename:
-if options.machine_type not in ["VExpress_GEM5", "VExpress_GEM5_V1"]:
-warn("Can only correctly generate a dtb for VExpress_GEM5_V1 " \
+if options.machine_type not in ["VExpress_GEM5",
+"VExpress_GEM5_V1",
+"VExpress_GEM5_V2",
+"VExpress_GEM5_Foundation"]:
+warn("Can only correctly generate a dtb for VExpress_GEM5_* " \
  "platforms, unless custom hardware models have been  
equipped "\

  "with generation functionality.")


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30575
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: I549124a1afbc36e313f614dccab17973582bc3f7
Gerrit-Change-Number: 30575
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Cooper 
Gerrit-Reviewer: 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] Re: bug squashing renamed pinned registers in o3?

[Gabe Black via gem5-dev]

Hi Giacomo, thanks for your reply. To answer your questions, it looks like
no for 1 (unless my grep was bad), and checkpoint save/restore for 2. I
think we've been able to reproduce this problem much more easily with older
versions of gem5, likely missing that fix, although I think we may have
also seen it with newer versions. I've only been looking at it recently and
was looking for the easiest way to reproduce, so I've only directly tried
the older version.

I've cc-ed some Google folks that can hopefully share more details and
confirm if they've seen this problem on a branch which does have the CL you
mentioned in 1.

To Google folks, we should cherry-pick that CL into our branch to at least
make the problem less common. We should have it already in our rebase
branch, since it looks like it went in upstream in early March.

Gabe

On Wed, Jun 24, 2020 at 2:18 AM Giacomo Travaglini <
giacomo.travagl...@arm.com> wrote:

> Hi Gabe,
>
>
>
> We are encountering the same problem on top of develop but it’s still
> worth asking:
>
>
>
>1. Do you have
>https://gem5-review.googlesource.com/c/public/gem5/+/25743 ?
>2. Are you encountering this in a simulation which is using a CPU
>switch or checkpoint save/restore
>
>
>
> Kind Regards
>
>
>
> Giacomo
>
>
>
> *From:* Gabe Black via gem5-dev 
> *Sent:* 23 June 2020 06:24
> *To:* gem5 Developer List 
> *Cc:* Gabe Black 
> *Subject:* [gem5-dev] bug squashing renamed pinned registers in o3?
>
>
>
> Hi folks, specifically ARM folks. We've been seeing a problem with O3
> where when switching vector register renaming modes (full vectors vs vector
> elements), the CPU checks its bookkeeping and finds that a vector register
> is missing, ie with no instructions in flight, the free list has one fewer
> register in it than the difference between the total number of physical
> vector registers, and the number that should be taken up with architectural
> state.
>
>
>
> This problem has been somewhat difficult to reproduce, although we can get
> it to happen, and it does happen often enough that it's been a real pain
> for us. Given that it's not very easy to get it to happen which makes it
> hard to observe, I've been digging around in the code trying to understand
> what all the pieces do and why the bookkeeping might be wrong.
>
>
>
> The most promising thing I've found so far is that when squashing, the
> rename stage looks at its history and rolls back renames for squashed
> instructions. Some registers are fixed and not renamed, so rolling back
> those would be pointless. Also those registers should not go on the free
> list.
>
>
>
> The way O3 detects those special registers is that they have the same
> index before and after renaming. If that is the case, O3 ignores those
> entries, and does not roll them back or mark their target as free.
>
>
>
> This check is slightly out of date though, since with the recently added
> pinned register writes, a register will be renamed to the same thing
> several times in a row. When these entries are checked, they will not be
> rolled back (I think this part is still fine), but they will also not be
> marked as free.
>
>
>
> This isn't exactly a smoking gun though, since the more I think about it,
> the more I think this may actually be ok. If one of the later writes is
> squashed, the register isn't "free" since it still holds the (partially
> written) architectural state. If everything gets squashed all the way back
> to the first entry which did change what register to use, then the slightly
> outdated check won't trigger and things should be freed up correctly (I
> think).
>
>
>
> This code is mostly new to me though, so I'm not super confident making
> any grand declarations about what's going on. All the pieces seem to be
> there though, which makes me very suspicious.
>
>
>
> Maybe something goes wrong if the right number of writes never happens
> because later writers get squashed?
>
>
>
> Gabe
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>
___
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