[gem5-dev] Re: Testlib six dependency

[Andreas Sandberg via gem5-dev]

Hi Jason,

Thanks for confirming that. I have posted an update here: 
https://gem5-review.googlesource.com/c/public/gem5/+/39759

Since there is no upstream for testlib, should we move it into tests/ somewhere 
instead of keeping it in ext/?

Cheers,
Andreas

On 26/01/2021 16:21, Jason Lowe-Power wrote:
Hi Andreas,

There is no upstream for testlib. It's a purely gem5 project. We should fix it 
in tree.

Jason

On Tue, Jan 26, 2021 at 4:56 AM Andreas Sandberg via gem5-dev 
mailto:gem5-dev@gem5.org>> wrote:
Hi Everyone,

I have just posted a series of patches [1] that get rid of 'six' as a
dependency in gem5. However, there is still a dependency on six coming
from testlib. What's the process there? Should we fix it upstream and
backport it or is testlib now effectively a gem5 project?

Cheers,
Abdreas

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

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<mailto:gem5-dev@gem5.org>
To unsubscribe send an email to 
gem5-dev-le...@gem5.org<mailto:gem5-dev-le...@gem5.org>
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
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] Testlib six dependency

[Andreas Sandberg via gem5-dev]

Hi Everyone,

I have just posted a series of patches [1] that get rid of 'six' as a
dependency in gem5. However, there is still a dependency on six coming
from testlib. What's the process there? Should we fix it upstream and
backport it or is testlib now effectively a gem5 project?

Cheers,
Abdreas

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

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] Re: ARM build failures

[Andreas Sandberg via gem5-dev]

+ Ciro, Richard

Hi Everyone,

Thanks for pointing this out and submitting a fix.

Richard/Ciro/Giacomo: Could one of you review this so we can merge the fix?

Thanks,
Andreas

On 31/08/2020 05:41, Bobby Bruce wrote:
Hey Gabe,

Iru Cai made a fix for this a week or so ago: 
https://gem5-review.googlesource.com/c/public/gem5/+/33154. Not sure if this 
addresses all concerns, but their change is mostly basic reduction due to `imm` 
and `ecount` being unsigned. I also find if you take on board these 
observations, there's at least one unreachable branch, and one condition that's 
always true (see my comments in the Gerrit PatchSet).

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

web: https://www.bobbybruce.net


On Sun, Aug 30, 2020 at 3:52 AM Gabe Black via gem5-dev 
mailto:gem5-dev@gem5.org>> wrote:
Hi folks. I'm seeing a few build failures for ARM with gcc version 10.2. Since 
these look like they may be real bugs and I don't want to make a mess fixing 
them or do a bunch of research, I'll mention them here so we can collectively 
find the right fix. There are a lot of instances of these two:

build/ARM/arch/arm/generated/exec-ns.cc.inc:278501:40: error: comparison of 
unsigned express
ion in '>= 0' is always true [-Werror=type-limits]
278501 | bool posCount = ((count * imm) >= 0);


build/ARM/arch/arm/generated/exec-ns.cc.inc:278970:40: error: comparison of 
unsigned express
ion in '< 0' is always false [-Werror=type-limits]
278970 | bool negCount = ((count * imm) < 0);

I'm not sure what's going on with these. Maybe applying the same template with 
both signed and unsigned imm and count fields? As far as I can tell with a 
little digging around, imm is usually unsigned. I'm not sure where count comes 
from, but I'm guessing also unsigned?

build/ARM/arch/arm/generated/exec-ns.cc.inc:169243:29: error: 
'destReg.ArmISAInst::VqdmulhsQ
<_Element>::execute::RegVect::regs[0]' may be used uninitialized 
in this functi
on [-Werror=maybe-uninitialized]
169243 | FpDestP0 = letoh(destReg.regs[0]);

I haven't looked into these at all.

build/ARM/arch/arm/generated/exec-ns.cc.inc:169491:17: error: comparison of 
unsigned express
ion in '< 0' is always false [-Werror=type-limits]
169491 | if (imm < 0 && imm >= eCount) {

This one looks really fishy. How would imm be both less than 0 and also greater 
than eCount? Is eCount negative? Is it ok for it to be just a little negative? 
Is this supposed to be an ||? Apparently imm is unsigned anyway, so comparing 
it with 0 is pointless.

Gabe
___
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


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] gem5 documentation repo

[Andreas Sandberg via gem5-dev]

Hi All,

I just had a quick look at the excellent new documentation section on
the gem5 website. A big thanks to everyone who has worked on making that
a reality!

One thing that I noticed when browsing the documentation is hosted in
the website repo and not the code repo. Would it make sense to move it
across to the code repo make sure it tracks the source code?

In practice, we probably want to present (at least) two different views
of the documentation, one for the latest release and one for develop.
Another option would be to deal with versioning in the website repo, but
that will make it hard to keep the documentation up-to-date when
submitting code changes.

Cheers,
Andreas

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] Re: [Suggestion] Replace gem5-users mailing-list with Discourse

[Andreas Sandberg via gem5-dev]

On 06/07/2020 19:37, Jason Lowe-Power wrote:
On Mon, Jul 6, 2020 at 11:22 AM Andreas Sandberg via gem5-dev 
mailto:gem5-dev@gem5.org>> wrote:
Hi Bobby,

Can't we solve some of these issues by just moving the mailinglist to a better system 
with good archiving? That should solve both the spam issue and some of the usability 
issues. I have looked at bit at groups.io<http://groups.io> since it is used in 
a project I'm contributing to in my spare time and it seems like a good hybrid 
between a mailinglist and a forum. They seem to have good support for grouping by 
topic, hash tag filtering, RSS feeds, and plenty of integrations.

The maintenance requirement of gem5-us...@gem5.org<mailto:gem5-us...@gem5.org> 
has been extraordinary. I really don't see how we can keep using that mailing list. 
Bobby has spent 50+ hours fighting with it in the past 6 months. From what I can 
tell, the way we have many people posting to a single list that ~1000 people 
subscribe to just isn't a normal use case for email anymore. We're getting blocked by 
spam filters, having to answer lots of questions about how to subscribe, etc.


The number of subscribers shouldn't be a problem. The Zephyr mailinglists have 
close to 2000 subscribers and several other open source projects have 
mailinglists with thousands of subscribers.

I think part of the problem here is that the mail servers for gem5.org are 
misconfigured or at least lack the configuration required for modern email 
systems.  According to the SMTP headers, our email servers have flagged a 
recent email from the dev list as failing SPF checks. I suspect the domain 
lacks (correct) SPF, DKIM, and DMARC records. Missing records will cause issues 
(higher likelihood of being flagged as spam), while incorrect ones will likely 
flag emails as spam right away.


If someone else is willing to step up and take ownership of the mailing list 
that would be great. It's just not something that we currently have resources 
for right now. We thought that moving to a managed system would help, but it 
hasn't solved the main problems: mail is still getting hijacked by spam 
filters, and people are still having problems signing up.


I really can't see what a Discourse-style forum would give us that you can't 
get from an email list with a good archive. I generally find forums at least as 
annoying as email archives when going back to look for information about a 
topic. The lack of threading within topics tends to make discussion really hard 
to follow, which usually isn't an issue in a well-behaved email list.

I'm not sure I understand the difference between Discourse and 
groups.io<http://groups.io> other than the interface. Could you describe why you 
think that groups.io<http://groups.io> would be better than discourse?

Also, from my experience, we have a number of people who try to contribute that *don't* 
have a well behaved email client. We see a large number of messages that are "off 
thread" (e.g., replying to a digest, changing the subject line accidentally, or just 
replying to the wrong message).


My impression of groups.io is that it is primarily an email distribution 
service with a fancy web frontend while Discourse is primarily a web system 
with email notifications.



Is the barrier of entry that people feel like the list is to "formal" or think 
that their questions are stupid? I'm not convinced that the latter would be solved by 
switching to a forum-style system like Discourse. A less formal chat system in addition 
to the list might be a better way to lower the barrier of entry.

I disagree somewhat with this. I think that if we had a discourse section titled "any 
questions here" or "new user questions" that it *would* lower the barrier to entry 
and make people feel more comfortable.


Hmm, yes, that is a good point.


I have found the Slack (despite the poor threading) system used by Zephyr very useful 
when debugging/developing drivers. It has been a convenient low-latency channel when 
working on the same subsystem as other people in the project and a general "I have 
seen this weird issue, has anyone else seen anything like it?". It's not a complete 
substitute for email lists though.

I feel that while slack might be useful, it's fundamentally different from the 
current users list. While it's gotten much better over the past several years, 
we still frequently answer the same questions over and over again on the 
mailing list because 1) we need to improve our documentation and 2) the mailing 
list isn't easy for most people to search.

I (personally) just can't imagine answering gem5 questions on slack. There's 
too much in my life that demands immediate attention already! But maybe that's 
just me.

I see them as serving different purposes. I wouldn't expect senior community members to hangout in 
an ask me anything channel all the time, maybe during "office hours

[gem5-dev] Re: [Suggestion] Replace gem5-users mailing-list with Discourse

[Andreas Sandberg via gem5-dev]

Hi Bobby,

Can't we solve some of these issues by just moving the mailinglist to a better 
system with good archiving? That should solve both the spam issue and some of 
the usability issues. I have looked at bit at groups.io since it is used in a 
project I'm contributing to in my spare time and it seems like a good hybrid 
between a mailinglist and a forum. They seem to have good support for grouping 
by topic, hash tag filtering, RSS feeds, and plenty of integrations.

I really can't see what a Discourse-style forum would give us that you can't 
get from an email list with a good archive. I generally find forums at least as 
annoying as email archives when going back to look for information about a 
topic. The lack of threading within topics tends to make discussion really hard 
to follow, which usually isn't an issue in a well-behaved email list.

Is the barrier of entry that people feel like the list is to "formal" or think 
that their questions are stupid? I'm not convinced that the latter would be solved by 
switching to a forum-style system like Discourse. A less formal chat system in addition 
to the list might be a better way to lower the barrier of entry.

I have found the Slack (despite the poor threading) system used by Zephyr very useful 
when debugging/developing drivers. It has been a convenient low-latency channel when 
working on the same subsystem as other people in the project and a general "I have 
seen this weird issue, has anyone else seen anything like it?". It's not a complete 
substitute for email lists though.

Cheers,
Andreas

On 06/07/2020 03:34, Bobby Bruce via gem5-dev wrote:
I personally see the problem of people not answering questions to be a fixed constant 
regardless as to what medium we choose. It's a shame, but it's a "people 
problem" which I agree won't be solved by deploying new platforms.

The reason for moving from the mailing-list is the mailing-list just doesn't 
appear to be an appropriate technology for tech support. I get emails every 
other week from someone who struggles to join gem5-users, and it normally turns 
out their gem5-user emails are ending up in spam. It's difficult to search 
through the mail archive to see if your question has been asked previously, 
it's hard to format your messages correctly, impossible to tag or categorize, 
and, I've already had students tell me they feel like reaching out over the 
gem5-users mailing list is awkward and embarrassing for the type of questions 
they want to ask. As a result, they just avoid doing so.

My issue with slack is it's got poor threading, and I'd quite like a good 
archive of answered questions for people to search. Though I'm not opposed to 
it as it has the plus of being popular (as shallow as it may be, I find going 
with the most popular solution to a problem is often the best course-of-action).

--
Dr. Bobby R. Bruce
Room 2235,
Kemper Hall, UC Davis
Davis,
CA, 95616

web: https://www.bobbybruce.net


On Thu, Jul 2, 2020 at 3:45 PM Gabe Black via gem5-dev 
mailto:gem5-dev@gem5.org>> wrote:
I haven't used Slack before (yeah, I know, behind the times :-), but I 100% 
agree with that last part. Having the perfect medium won't help if there aren't 
enough people around to actually use it to answer questions.

Gabe

On Thu, Jul 2, 2020 at 9:45 AM Andreas Sandberg via gem5-dev 
mailto:gem5-dev@gem5.org>> wrote:
I would probably be more in favour of a split email+Slack/Teams
approach. Email works well for most discussion, but I like the quick
more informal communication in a chat system. I have generally been very
happy with the way Slack has worked when I have contributed to Zephyr in
my spare time. As long as you have a threading email client, I can't see
any benefits of a forum other than archiving (services like 
groups.io<http://groups.io>
seems to solve that).

I think we are fooling ourselves if we think switching from email to a
different medium is going to solve underlying problem the there is a
small number of experienced users that answer most of the questions on
the lists.

Cheers,
Andreas

On 10/06/2020 16:32, Daniel Gerzhoy via gem5-dev wrote:

I think this is a great idea! Emails threads aren't a great way to do this
just because there's no mechanism for well formatted responses to
particular points in someone's questions, posting code, or things like
"upvoting" responses.

I see Daniel's point about less engagement if we move it to a forum, but I
think that could be alleviated by encouraging people to keep email
notifications up.

Cheers,

Dan

On Wed, Jun 10, 2020 at 5:23 AM Giacomo Travaglini via gem5-dev <
gem5-dev@gem5.org<mailto:gem5-dev@gem5.org>> wrote:


I agree with Daniel and Ciro; it's difficult/annoying to navigate through
old unanswered emails and I presume nobody does that at the moment.
Most of the time if your email doesn't get a quick response as soon as it
gets posted, you can forget about getting

[gem5-dev] Re: mercurial support?

[Andreas Sandberg via gem5-dev]

Hi Gabe,

As far as I know, we aren't keeping the Mercurial server in sync with the git 
repo any more. I can't see any reason to keep Mercurial-related cruft in the 
new repo.

Cheers,
Andreas

On 05/07/2020 02:08, Gabe Black via gem5-dev wrote:
Hi folks. Have we officially dropped support for checking out gem5 through 
mercurial? If so, we should probably delete the various bits and pieces lying 
around which were for that. If not then never mind...

Gabe



___
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


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] Re: [Suggestion] Replace gem5-users mailing-list with Discourse

[Andreas Sandberg via gem5-dev]

I would probably be more in favour of a split email+Slack/Teams
approach. Email works well for most discussion, but I like the quick
more informal communication in a chat system. I have generally been very
happy with the way Slack has worked when I have contributed to Zephyr in
my spare time. As long as you have a threading email client, I can't see
any benefits of a forum other than archiving (services like groups.io
seems to solve that).

I think we are fooling ourselves if we think switching from email to a
different medium is going to solve underlying problem the there is a
small number of experienced users that answer most of the questions on
the lists.

Cheers,
Andreas

On 10/06/2020 16:32, Daniel Gerzhoy via gem5-dev wrote:

I think this is a great idea! Emails threads aren't a great way to do this
just because there's no mechanism for well formatted responses to
particular points in someone's questions, posting code, or things like
"upvoting" responses.

I see Daniel's point about less engagement if we move it to a forum, but I
think that could be alleviated by encouraging people to keep email
notifications up.

Cheers,

Dan

On Wed, Jun 10, 2020 at 5:23 AM Giacomo Travaglini via gem5-dev <
gem5-dev@gem5.org> wrote:


I agree with Daniel and Ciro; it's difficult/annoying to navigate through
old unanswered emails and I presume nobody does that at the moment.
Most of the time if your email doesn't get a quick response as soon as it
gets posted, you can forget about getting some help as time passes; there
is a short window and you really have to hope someone flagged your email or
got some time to address your problem.

I wouldn't use JIRA to be honest for asking questions; that's beyond the
scope of JIRA and it would be chaotic to mix Bug Reports, Improvement tasks
with normal questions (like: I cannot build gem5 on my machine, or does
anyone know how this works)

Giacomo


-Original Message-
From: Ciro Santilli via gem5-dev 
Sent: 10 June 2020 09:21
To: gem5 Developer List 
Cc: Ciro Santilli 
Subject: [gem5-dev] Re: [Suggestion] Replace gem5-users mailing-list with
Discourse

I would just use JIRA. But after that, Discourse is the second best. And
anything is better than a mailing list :-) 
From: Jason Lowe-Power via gem5-dev 
Sent: Tuesday, June 9, 2020 11:40 PM
To: gem5 Developer List 
Cc: gem5 users mailing list ; Jason Lowe-Power <
ja...@lowepower.com>
Subject: [gem5-dev] Re: [Suggestion] Replace gem5-users mailing-list with
Discourse

+1 for Discourse :).

Just to give a bit more context: I'm also trying to find a good forum for
community engagement during my online Learning gem5 class this summer. I
would like to find a platform that could be used generally for my class
this summer, future iterations of the class, and general gem5 questions, as
I believe there will be significant overlap between these groups.

Other potential options that IMO have more cons than pros when compared to
Discourse:
- Slack/Teams/etc.
- gitter.im
- stackoverflow

That said, we're open to suggestions :). Our goal is to create the most
welcoming and inclusive environment possible. We'll go where our users are!

Cheers,
Jason

On Tue, Jun 9, 2020 at 2:45 PM Bobby Bruce via gem5-dev 
Dear all,

In an effort to better support the gem5 community, there has been a
suggestion that we drop the gem5-users mailing list and replace it
with Discourse, https://www.discourse.org/about, a web-based
discussion platform. I'm writing this email to propose this to the
community and ask for feedback on the matter.

We have noticed that using mailing lists as our primary communication
platform is problematic. Sending an email to a list can be daunting
experience, requiring an etiquette many are not accustom to. I'm sure
I'm not the only one who feels like they are unduly bothering a large
number of people when posting to a list (like I'm doing right now :)
). This is, of course, an unfortunate hurdle for many to get over when
they encounter problems using gem5, particularly those new to the
project. I've come to believe mailing lists are simply not a very good
technology for fostering community engagement and helping those who are

running into difficulties.

Mailing lists are also difficult to search, and lack proper formatting
mechanisms to neatly display attributes such as code and output logs.

Looking around at alternative technologies available, Discourse
appears to be a suitable replacement. For those unaware, Discourse is
(essentially) a revamp of messaging forums. It is an increasingly
popular platform for users and developers in open source projects to
communicate with one another (see LLVM's discourse as an example:
https://llvm.discourse.group ).
All-in-all, I think it's a well-designed product and contains all the
features we'd expect and need to get our work done. I can see no
immediate downsides to using it, though feedback from the community on
the matter would be greatly 

[gem5-dev] changeset in gem5: dev, arm: Clean up PL011 and rewrite interrup...

[Andreas Sandberg via gem5-dev]

changeset 4ed87af2930f in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=4ed87af2930f
description:
dev, arm: Clean up PL011 and rewrite interrupt handling

The ARM PL011 UART model didn't clear and raise interrupts
correctly. This changeset rewrites the whole interrupt handling and
makes it both simpler and fixes several cases where the correct
interrupts weren't raised or cleared. Additionally, it cleans up many
other aspects of the code.

diffstat:

 src/dev/arm/pl011.cc |  144 
 src/dev/arm/pl011.hh |  152 --
 2 files changed, 144 insertions(+), 152 deletions(-)

diffs (truncated from 442 to 300 lines):

diff -r 4f8c1bd6fdb8 -r 4ed87af2930f src/dev/arm/pl011.cc
--- a/src/dev/arm/pl011.cc  Mon Mar 02 04:00:42 2015 -0500
+++ b/src/dev/arm/pl011.cc  Mon Mar 02 04:00:44 2015 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010 ARM Limited
+ * Copyright (c) 2010, 2015 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -38,23 +38,29 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
  * Authors: Ali Saidi
+ *  Andreas Sandberg
  */
 
+#include dev/arm/pl011.hh
+
 #include base/trace.hh
 #include debug/Checkpoint.hh
 #include debug/Uart.hh
 #include dev/arm/amba_device.hh
 #include dev/arm/base_gic.hh
-#include dev/arm/pl011.hh
 #include dev/terminal.hh
 #include mem/packet.hh
 #include mem/packet_access.hh
 #include sim/sim_exit.hh
+#include params/Pl011.hh
 
-Pl011::Pl011(const Params *p)
-: Uart(p, 0xfff), control(0x300), fbrd(0), ibrd(0), lcrh(0), ifls(0x12),
-  imsc(0), rawInt(0), maskInt(0), intNum(p-int_num), gic(p-gic),
-  endOnEOT(p-end_on_eot), intDelay(p-int_delay), intEvent(this)
+Pl011::Pl011(const Pl011Params *p)
+: Uart(p, 0xfff),
+  intEvent(this),
+  control(0x300), fbrd(0), ibrd(0), lcrh(0), ifls(0x12),
+  imsc(0), rawInt(0),
+  gic(p-gic), endOnEOT(p-end_on_eot), intNum(p-int_num),
+  intDelay(p-int_delay)
 {
 }
 
@@ -75,17 +81,23 @@
 switch(daddr) {
   case UART_DR:
 data = 0;
-if (term-dataAvailable())
+if (term-dataAvailable()) {
 data = term-in();
+// Since we don't simulate a FIFO for incoming data, we
+// assume it's empty and clear RXINTR and RTINTR.
+clearInterrupts(UART_RXINTR | UART_RTINTR);
+}
 break;
   case UART_FR:
-// For now we're infintely fast, so TX is never full, always empty,
-// always clear to send
-data = UART_FR_TXFE | UART_FR_CTS;
-if (!term-dataAvailable())
-data |= UART_FR_RXFE;
-DPRINTF(Uart, Reading FR register as %#x rawInt=0x%x imsc=0x%x 
maskInt=0x%x\n,
-data, rawInt, imsc, maskInt);
+data =
+UART_FR_CTS | // Clear To Send
+(!term-dataAvailable() ? UART_FR_RXFE : 0) | // RX FIFO Empty
+UART_FR_TXFE; // TX FIFO empty
+
+DPRINTF(Uart,
+Reading FR register as %#x rawInt=0x%x 
+imsc=0x%x maskInt=0x%x\n,
+data, rawInt, imsc, maskInt());
 break;
   case UART_CR:
 data = control;
@@ -110,8 +122,8 @@
 DPRINTF(Uart, Reading Raw Int status as 0x%x\n, rawInt);
 break;
   case UART_MIS:
-DPRINTF(Uart, Reading Masked Int status as 0x%x\n, rawInt);
-data = maskInt;
+DPRINTF(Uart, Reading Masked Int status as 0x%x\n, maskInt());
+data = maskInt();
 break;
   default:
 if (readId(pkt, AMBA_ID, pioAddr)) {
@@ -182,15 +194,11 @@
 exitSimLoop(UART received EOT, 0);
 
 term-out(data  0xFF);
-
-//raw interrupt is set regardless of imsc.txim
-rawInt.txim = 1;
-if (imsc.txim) {
-DPRINTF(Uart, TX int enabled, scheduling interruptt\n);
-if (!intEvent.scheduled())
-schedule(intEvent, curTick() + intDelay);
-}
-
+// We're supposed to clear TXINTR when this register is
+// written to, however. since we're also infinitely fast, we
+// need to immediately raise it again.
+clearInterrupts(UART_TXINTR);
+raiseInterrupts(UART_TXINTR);
 break;
   case UART_CR:
 control = data;
@@ -208,35 +216,13 @@
 ifls = data;
 break;
   case UART_IMSC:
-imsc = data;
-
-if (imsc.feim || imsc.peim || imsc.beim || imsc.oeim || imsc.rsvd)
-panic(Unknown interrupt enabled\n);
-
-// rimim, ctsmim, dcdmim, dsrmim can be enabled but are ignored
-// they are supposed to interrupt on a change of status in the line
-// which we should never have since our terminal is happy to always
-// receive bytes.
-
-if (imsc.txim) {
- 

[gem5-dev] changeset in gem5: arm: Correctly access the stack pointer in GDB

[Andreas Sandberg via gem5-dev]

changeset fe09d1bc6721 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=fe09d1bc6721
description:
arm: Correctly access the stack pointer in GDB

We curently use INTREG_X31 instead of INTREG_SPX when accessing the
stack pointer in GDB. gem5 normally uses INTREG_SPX to access the
stack pointer, which gets mapped to the stack pointer corresponding
(INTREG_SPn) to the current exception level. This changeset updates
the GDB interface to use SPX instead of X31 (which is always zero)
when transfering CPU state to gdb.

diffstat:

 src/arch/arm/remote_gdb.cc |  13 +
 src/arch/arm/remote_gdb.hh |   1 +
 2 files changed, 10 insertions(+), 4 deletions(-)

diffs (47 lines):

diff -r f7d17d8a854c -r fe09d1bc6721 src/arch/arm/remote_gdb.cc
--- a/src/arch/arm/remote_gdb.ccMon Mar 02 04:00:27 2015 -0500
+++ b/src/arch/arm/remote_gdb.ccMon Mar 02 04:00:27 2015 -0500
@@ -204,9 +204,10 @@
 memset(gdbregs.regs, 0, gdbregs.bytes());
 
 if (inAArch64(context)) {  // AArch64
-// x0-x31
-for (int i = 0; i  32; ++i)
+// x0-x30
+for (int i = 0; i  31; ++i)
 gdbregs.regs64[GDB64_X0 + i] = context-readIntReg(INTREG_X0 + i);
+gdbregs.regs64[GDB64_SPX] = context-readIntReg(INTREG_SPX);
 // pc
 gdbregs.regs64[GDB64_PC] = context-pcState().pc();
 // cpsr
@@ -262,13 +263,17 @@
 
 DPRINTF(GDBAcc, setregs in remotegdb \n);
 if (inAArch64(context)) {  // AArch64
-// x0-x31
-for (int i = 0; i  32; ++i)
+// x0-x30
+for (int i = 0; i  31; ++i)
 context-setIntReg(INTREG_X0 + i, gdbregs.regs64[GDB64_X0 + i]);
 // pc
 context-pcState(gdbregs.regs64[GDB64_PC]);
 // cpsr
 context-setMiscRegNoEffect(MISCREG_CPSR, gdbregs.regs64[GDB64_CPSR]);
+// Update the stack pointer. This should be done after
+// updating CPSR/PSTATE since that might affect how SPX gets
+// mapped.
+context-setIntReg(INTREG_SPX, gdbregs.regs64[GDB64_SPX]);
 // v0-v31
 for (int i = 0; i  128; i += 4) {
 int gdboff = GDB64_V0_32 + i;
diff -r f7d17d8a854c -r fe09d1bc6721 src/arch/arm/remote_gdb.hh
--- a/src/arch/arm/remote_gdb.hhMon Mar 02 04:00:27 2015 -0500
+++ b/src/arch/arm/remote_gdb.hhMon Mar 02 04:00:27 2015 -0500
@@ -68,6 +68,7 @@
 // AArch64 registers
 enum {
 GDB64_X0 = 0,
+GDB64_SPX = 31,
 GDB64_PC = 32,
 GDB64_CPSR = 33,
 GDB64_V0 = 34,
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: arm: Fix broken page table permissions checks...

[Andreas Sandberg via gem5-dev]

changeset f7d17d8a854c in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=f7d17d8a854c
description:
arm: Fix broken page table permissions checks in remote GDB

The remote GDB interface currently doesn't check if translations are
valid before reading memory. This causes a panic when GDB tries to
access unmapped memory (e.g., when getting a stack trace). There are
two reasons for this: 1) The function used to check for valid
translations (virtvalid()) doesn't work and panics on invalid
translations. 2) The method in the GDB interface used to test if a
translation is valid (RemoteGDB::acc) always returns true regardless
of the return from virtvalid().

This changeset fixes both of these issues.

diffstat:

 src/arch/arm/remote_gdb.cc |  15 ++-
 src/arch/arm/vtophys.cc|  27 +++
 2 files changed, 25 insertions(+), 17 deletions(-)

diffs (87 lines):

diff -r 4206946d60fe -r f7d17d8a854c src/arch/arm/remote_gdb.cc
--- a/src/arch/arm/remote_gdb.ccThu Feb 26 09:58:26 2015 -0600
+++ b/src/arch/arm/remote_gdb.ccMon Mar 02 04:00:27 2015 -0500
@@ -142,6 +142,7 @@
 #include arch/arm/system.hh
 #include arch/arm/utility.hh
 #include arch/arm/vtophys.hh
+#include base/chunk_generator.hh
 #include base/intmath.hh
 #include base/remote_gdb.hh
 #include base/socket.hh
@@ -172,16 +173,12 @@
 RemoteGDB::acc(Addr va, size_t len)
 {
 if (FullSystem) {
-Addr last_va;
-va   = truncPage(va);
-last_va  = roundPage(va + len);
-
-do  {
-if (virtvalid(context, va)) {
-return true;
+for (ChunkGenerator gen(va, len, PageBytes); !gen.done(); gen.next()) {
+if (!virtvalid(context, gen.addr())) {
+DPRINTF(GDBAcc, acc:   %#x mapping is invalid\n, va);
+return false;
 }
-va += PageBytes;
-} while (va  last_va);
+}
 
 DPRINTF(GDBAcc, acc:   %#x mapping is valid\n, va);
 return true;
diff -r 4206946d60fe -r f7d17d8a854c src/arch/arm/vtophys.cc
--- a/src/arch/arm/vtophys.cc   Thu Feb 26 09:58:26 2015 -0600
+++ b/src/arch/arm/vtophys.cc   Mon Mar 02 04:00:27 2015 -0500
@@ -63,8 +63,8 @@
 fatal(VTOPHYS: Can't convert vaddr to paddr on ARM without a thread 
context);
 }
 
-Addr
-ArmISA::vtophys(ThreadContext *tc, Addr addr)
+static std::pairbool, Addr
+try_translate(ThreadContext *tc, Addr addr)
 {
 Fault fault;
 // Set up a functional memory Request to pass to the TLB
@@ -82,22 +82,33 @@
 tlb = static_castArmISA::TLB*(tc-getDTBPtr());
 fault = tlb-translateFunctional(req, tc, BaseTLB::Read, TLB::NormalTran);
 if (fault == NoFault)
-return req.getPaddr();
+return std::make_pair(true, req.getPaddr());
 
 tlb = static_castArmISA::TLB*(tc-getITBPtr());
 fault = tlb-translateFunctional(req, tc, BaseTLB::Read, TLB::NormalTran);
 if (fault == NoFault)
-return req.getPaddr();
+return std::make_pair(true, req.getPaddr());
 
-panic(Table walkers support functional accesses. We should never get 
here\n);
+return std::make_pair(false, 0);
+}
+
+Addr
+ArmISA::vtophys(ThreadContext *tc, Addr addr)
+{
+const std::pairbool, Addr translation(try_translate(tc, addr));
+
+if (translation.first)
+return translation.second;
+else
+panic(Table walkers support functional accesses. We should never get 
here\n);
 }
 
 bool
 ArmISA::virtvalid(ThreadContext *tc, Addr vaddr)
 {
-if (vtophys(tc, vaddr) != -1)
-return true;
-return false;
+const std::pairbool, Addr translation(try_translate(tc, vaddr));
+
+return translation.first;
 }
 
 
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: arm: Don't truncate 16-bit ASIDs to 8 bits

[Andreas Sandberg via gem5-dev]

changeset 890269a13188 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=890269a13188
description:
arm: Don't truncate 16-bit ASIDs to 8 bits

The ISA code sometimes stores 16-bit ASIDs as 8-bit unsigned integers
and has a couple of inverted checks that mask out the high 8 bits of
an ASID if 16-bit ASIDs have been /enabled/. This changeset fixes both
of those issues.

diffstat:

 src/arch/arm/isa.cc |  8 
 src/arch/arm/isa.hh |  2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diffs (38 lines):

diff -r fe09d1bc6721 -r 890269a13188 src/arch/arm/isa.cc
--- a/src/arch/arm/isa.cc   Mon Mar 02 04:00:27 2015 -0500
+++ b/src/arch/arm/isa.cc   Mon Mar 02 04:00:28 2015 -0500
@@ -1386,7 +1386,7 @@
 oc = sys-getThreadContext(x);
 assert(oc-getITBPtr()  oc-getDTBPtr());
 asid = bits(newVal, 63, 48);
-if (haveLargeAsid64)
+if (!haveLargeAsid64)
 asid = mask(8);
 oc-getITBPtr()-flushAsid(asid, secure_lookup, target_el);
 oc-getDTBPtr()-flushAsid(asid, secure_lookup, target_el);
@@ -1941,10 +1941,10 @@
 }
 
 void
-ISA::tlbiVA(ThreadContext *tc, MiscReg newVal, uint8_t asid, bool 
secure_lookup,
-uint8_t target_el)
+ISA::tlbiVA(ThreadContext *tc, MiscReg newVal, uint16_t asid,
+bool secure_lookup, uint8_t target_el)
 {
-if (haveLargeAsid64)
+if (!haveLargeAsid64)
 asid = mask(8);
 Addr va = ((Addr) bits(newVal, 43, 0))  12;
 System *sys = tc-getSystemPtr();
diff -r fe09d1bc6721 -r 890269a13188 src/arch/arm/isa.hh
--- a/src/arch/arm/isa.hh   Mon Mar 02 04:00:27 2015 -0500
+++ b/src/arch/arm/isa.hh   Mon Mar 02 04:00:28 2015 -0500
@@ -221,7 +221,7 @@
 assert(!cpsr.width);
 }
 
-void tlbiVA(ThreadContext *tc, MiscReg newVal, uint8_t asid,
+void tlbiVA(ThreadContext *tc, MiscReg newVal, uint16_t asid,
 bool secure_lookup, uint8_t target_el);
 
 void tlbiALL(ThreadContext *tc, bool secure_lookup, uint8_t target_el);
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: arm: Merge ISA files with pseudo instructions

[Andreas Sandberg via gem5-dev]

changeset b5e5068fcb26 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=b5e5068fcb26
description:
arm: Merge ISA files with pseudo instructions

This changeset moves the pseudo instructions used to signal unknown
instructions and unimplemented instructions to the same source files
as the decoder fault.

diffstat:

 src/arch/arm/insts/pseudo.cc |  101 
 src/arch/arm/insts/pseudo.hh |   70 +++
 src/arch/arm/isa/formats/formats.isa |6 -
 src/arch/arm/isa/formats/pseudo.isa  |   34 +
 src/arch/arm/isa/formats/unimp.isa   |  215 ---
 src/arch/arm/isa/formats/unknown.isa |   46 ---
 6 files changed, 205 insertions(+), 267 deletions(-)

diffs (truncated from 554 to 300 lines):

diff -r ef2c71a5f02e -r b5e5068fcb26 src/arch/arm/insts/pseudo.cc
--- a/src/arch/arm/insts/pseudo.cc  Mon Feb 16 03:32:38 2015 -0500
+++ b/src/arch/arm/insts/pseudo.cc  Mon Feb 16 03:32:58 2015 -0500
@@ -11,6 +11,9 @@
  * unmodified and in its entirety in all distributions of the software,
  * modified or unmodified, in source code or in binary form.
  *
+ * Copyright (c) 2007-2008 The Florida State University
+ * 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
@@ -35,6 +38,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
  * Authors: Andreas Sandberg
+ *  Stephen Hines
  */
 
 #include arch/arm/insts/pseudo.hh
@@ -99,3 +103,100 @@
 {
 return csprintf(gem5fault %s, faultName());
 }
+
+
+
+FailUnimplemented::FailUnimplemented(const char *_mnemonic,
+ ExtMachInst _machInst)
+: ArmStaticInst(_mnemonic, _machInst, No_OpClass)
+{
+// don't call execute() (which panics) if we're on a
+// speculative path
+flags[IsNonSpeculative] = true;
+}
+
+FailUnimplemented::FailUnimplemented(const char *_mnemonic,
+ ExtMachInst _machInst,
+ const std::string _fullMnemonic)
+: ArmStaticInst(_mnemonic, _machInst, No_OpClass),
+  fullMnemonic(_fullMnemonic)
+{
+// don't call execute() (which panics) if we're on a
+// speculative path
+flags[IsNonSpeculative] = true;
+}
+
+Fault
+FailUnimplemented::execute(ExecContext *xc, Trace::InstRecord *traceData) const
+{
+return std::make_sharedUndefinedInstruction(machInst, false, mnemonic);
+}
+
+std::string
+FailUnimplemented::generateDisassembly(Addr pc, const SymbolTable *symtab) 
const
+{
+return csprintf(%-10s (unimplemented),
+fullMnemonic.size() ? fullMnemonic.c_str() : mnemonic);
+}
+
+
+
+WarnUnimplemented::WarnUnimplemented(const char *_mnemonic,
+ ExtMachInst _machInst)
+: ArmStaticInst(_mnemonic, _machInst, No_OpClass), warned(false)
+{
+// don't call execute() (which panics) if we're on a
+// speculative path
+flags[IsNonSpeculative] = true;
+}
+
+WarnUnimplemented::WarnUnimplemented(const char *_mnemonic,
+ ExtMachInst _machInst,
+ const std::string _fullMnemonic)
+: ArmStaticInst(_mnemonic, _machInst, No_OpClass), warned(false),
+  fullMnemonic(_fullMnemonic)
+{
+// don't call execute() (which panics) if we're on a
+// speculative path
+flags[IsNonSpeculative] = true;
+}
+
+Fault
+WarnUnimplemented::execute(ExecContext *xc, Trace::InstRecord *traceData) const
+{
+if (!warned) {
+warn(\tinstruction '%s' unimplemented\n,
+ fullMnemonic.size() ? fullMnemonic.c_str() : mnemonic);
+warned = true;
+}
+
+return NoFault;
+}
+
+std::string
+WarnUnimplemented::generateDisassembly(Addr pc, const SymbolTable *symtab) 
const
+{
+return csprintf(%-10s (unimplemented),
+fullMnemonic.size() ? fullMnemonic.c_str() : mnemonic);
+}
+
+
+
+FlushPipeInst::FlushPipeInst(const char *_mnemonic, ExtMachInst _machInst)
+: ArmStaticInst(_mnemonic, _machInst, No_OpClass)
+{
+flags[IsNonSpeculative] = true;
+}
+
+Fault
+FlushPipeInst::execute(ExecContext *xc, Trace::InstRecord *traceData) const
+{
+Fault fault = std::make_sharedFlushPipe();
+return fault;
+}
+
+std::string
+FlushPipeInst::generateDisassembly(Addr pc, const SymbolTable *symtab) const
+{
+return csprintf(%-10s (pipe flush), mnemonic);
+}
diff -r ef2c71a5f02e -r b5e5068fcb26 src/arch/arm/insts/pseudo.hh
--- a/src/arch/arm/insts/pseudo.hh  Mon Feb 16 03:32:38 2015 -0500
+++ b/src/arch/arm/insts/pseudo.hh  Mon Feb 16 03:32:58 2015 -0500
@@ -11,6 +11,9 @@
  * unmodified and in its entirety in all distributions of the software,
  * modified or unmodified, in source code or in binary form.
  *
+ 

[gem5-dev] changeset in gem5: dev: Remove unused system pointer in the Plat...

[Andreas Sandberg via gem5-dev]

changeset d1d95f0f4563 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=d1d95f0f4563
description:
dev: Remove unused system pointer in the Platform base class

The Platform base class contains a pointer to an instance of the
System which is never initialized. This can lead to subtle bugs since
some architecture-specific platform implementations contain their own
system pointer which is normally used. However, if the platform is
accessed through a pointer to its base class, the dangling pointer
will be used instead.

diffstat:

 src/dev/platform.hh |  3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diffs (13 lines):

diff -r 94901e131a7f -r d1d95f0f4563 src/dev/platform.hh
--- a/src/dev/platform.hh   Fri Feb 06 18:01:22 2015 -0800
+++ b/src/dev/platform.hh   Wed Feb 11 10:23:22 2015 -0500
@@ -55,9 +55,6 @@
 /** Pointer to the interrupt controller */
 IntrControl *intrctrl;
 
-/** Pointer to the system for info about the memory system. */
-System *system;
-
   public:
 typedef PlatformParams Params;
 Platform(const Params *p);
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: style: Fix broken m5format command

[Andreas Sandberg via gem5-dev]

changeset ab81a0feab55 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=ab81a0feab55
description:
style: Fix broken m5format command

The m5format command didn't actually work due to parameter handling
issues and missing language detection. This changeset fixes those
issues and cleans up some of the code to shared between the style
checker and the format checker.

diffstat:

 util/style.py |  123 +
 1 files changed, 70 insertions(+), 53 deletions(-)

diffs (195 lines):

diff -r 65da28dee7cf -r ab81a0feab55 util/style.py
--- a/util/style.py Wed Feb 11 10:23:33 2015 -0500
+++ b/util/style.py Wed Feb 11 10:23:34 2015 -0500
@@ -352,7 +352,8 @@
self.trailwhite or self.badcontrol or self.cret
 
 def validate(filename, stats, verbose, exit_code):
-if lang_type(filename) not in format_types:
+lang = lang_type(filename)
+if lang not in format_types:
 return
 
 def msg(lineno, line, message):
@@ -408,7 +409,7 @@
 bad()
 
 # for c++, exactly one space betwen if/while/for and (
-if cpp:
+if lang == 'C++':
 match = any_control.search(line)
 if match and not good_control.search(line):
 stats.badcontrol += 1
@@ -417,6 +418,40 @@
 bad()
 
 
+def _modified_regions(repo, patterns, **kwargs):
+opt_all = kwargs.get('all', False)
+opt_no_ignore = kwargs.get('no_ignore', False)
+
+# Import the match (repository file name matching helper)
+# function. Different versions of Mercurial keep it in different
+# modules and implement them differently.
+try:
+from mercurial import scmutil
+m = scmutil.match(repo[None], patterns, kwargs)
+except ImportError:
+from mercurial import cmdutil
+m = cmdutil.match(repo, patterns, kwargs)
+
+modified, added, removed, deleted, unknown, ignore, clean = \
+repo.status(match=m, clean=opt_all)
+
+if not opt_all:
+try:
+wctx = repo.workingctx()
+except:
+from mercurial import context
+wctx = context.workingctx(repo)
+
+files = [ (fn, all_regions) for fn in added ] + \
+[ (fn,  modregions(wctx, fn)) for fn in modified ]
+else:
+files = [ (fn, all_regions) for fn in added + modified + clean ]
+
+for fname, mod_regions in files:
+if opt_no_ignore or not check_ignores(fname):
+yield fname, mod_regions
+
+
 def do_check_style(hgui, repo, *pats, **opts):
 check files for proper m5 style guidelines
 
@@ -430,8 +465,6 @@
 The --all option can be specified to include clean files and check
 modified files in their entirety.
 
-from mercurial import mdiff, util
-
 opt_fix_all = opts.get('fix_all', False)
 if not opt_fix_all:
 opt_fix_white = opts.get('fix_white', False)
@@ -440,8 +473,6 @@
 opt_fix_white = True
 opt_fix_include = True
 
-opt_all = opts.get('all', False)
-opt_no_ignore = opts.get('no_ignore', False)
 ui = MercurialUI(hgui, verbose=hgui.verbose)
 
 def prompt(name, func, regions=all_regions):
@@ -460,36 +491,9 @@
 prompt_white = prompt if not opt_fix_white else no_prompt
 prompt_include = prompt if not opt_fix_include else no_prompt
 
-# Import the match (repository file name matching helper)
-# function. Different versions of Mercurial keep it in different
-# modules and implement them differently.
-try:
-from mercurial import scmutil
-m = scmutil.match(repo[None], pats, opts)
-except ImportError:
-from mercurial import cmdutil
-m = cmdutil.match(repo, pats, opts)
-
-modified, added, removed, deleted, unknown, ignore, clean = \
-repo.status(match=m, clean=opt_all)
-if not opt_all:
-try:
-wctx = repo.workingctx()
-except:
-from mercurial import context
-wctx = context.workingctx(repo)
-
-files = [ (fn, all_regions) for fn in added ] + \
-[ (fn,  modregions(wctx, fn)) for fn in modified ]
-else:
-files = [ (fn, all_regions) for fn in added + modified + clean ]
-
 whitespace = Whitespace(ui, repo)
 sorted_includes = SortedIncludes(ui, repo)
-for fname, mod_regions in files:
-if not opt_no_ignore and check_ignores(fname):
-continue
-
+for fname, mod_regions in _modified_regions(repo, pats, **opts):
 if whitespace.apply(fname, prompt_white, mod_regions):
 return True
 
@@ -498,22 +502,32 @@
 
 return False
 
-def do_check_format(hgui, repo, **args):
+def do_check_format(hgui, repo, *pats, **opts):
+check files for gem5 code formatting violations
+
+Without an argument, checks all modified and added files for gem5
+code formatting violations. A list of files can be 

[gem5-dev] changeset in gem5: sim: Move the BaseTLB to src/arch/generic/

[Andreas Sandberg via gem5-dev]

changeset 276da6265ab8 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=276da6265ab8
description:
sim: Move the BaseTLB to src/arch/generic/

The TLB-related code is generally architecture dependent and should
live in the arch directory to signify that.

diffstat:

 src/arch/alpha/tlb.hh |2 +-
 src/arch/arm/stage2_lookup.hh |1 -
 src/arch/arm/tlb.hh   |2 +-
 src/arch/generic/BaseTLB.py   |   34 +
 src/arch/generic/SConscript   |5 +
 src/arch/generic/tlb.cc   |   72 
 src/arch/generic/tlb.hh   |  150 ++
 src/arch/mips/tlb.hh  |2 +-
 src/arch/power/tlb.hh |2 +-
 src/arch/sparc/tlb.hh |2 +-
 src/arch/x86/faults.hh|2 +-
 src/arch/x86/tlb.hh   |2 +-
 src/cpu/base_dyn_inst.hh  |2 +-
 src/cpu/checker/cpu.cc|2 +-
 src/cpu/translation.hh|2 +-
 src/sim/BaseTLB.py|   34 -
 src/sim/SConscript|3 -
 src/sim/tlb.cc|   71 ---
 src/sim/tlb.hh|  150 --
 19 files changed, 271 insertions(+), 269 deletions(-)

diffs (truncated from 758 to 300 lines):

diff -r 1922f9d2ac01 -r 276da6265ab8 src/arch/alpha/tlb.hh
--- a/src/arch/alpha/tlb.hh Wed Feb 11 10:23:24 2015 -0500
+++ b/src/arch/alpha/tlb.hh Wed Feb 11 10:23:27 2015 -0500
@@ -39,10 +39,10 @@
 #include arch/alpha/pagetable.hh
 #include arch/alpha/utility.hh
 #include arch/alpha/vtophys.hh
+#include arch/generic/tlb.hh
 #include base/statistics.hh
 #include mem/request.hh
 #include params/AlphaTLB.hh
-#include sim/tlb.hh
 
 class ThreadContext;
 
diff -r 1922f9d2ac01 -r 276da6265ab8 src/arch/arm/stage2_lookup.hh
--- a/src/arch/arm/stage2_lookup.hh Wed Feb 11 10:23:24 2015 -0500
+++ b/src/arch/arm/stage2_lookup.hh Wed Feb 11 10:23:27 2015 -0500
@@ -47,7 +47,6 @@
 #include arch/arm/table_walker.hh
 #include arch/arm/tlb.hh
 #include mem/request.hh
-#include sim/tlb.hh
 
 class ThreadContext;
 
diff -r 1922f9d2ac01 -r 276da6265ab8 src/arch/arm/tlb.hh
--- a/src/arch/arm/tlb.hh   Wed Feb 11 10:23:24 2015 -0500
+++ b/src/arch/arm/tlb.hh   Wed Feb 11 10:23:27 2015 -0500
@@ -48,12 +48,12 @@
 #include arch/arm/pagetable.hh
 #include arch/arm/utility.hh
 #include arch/arm/vtophys.hh
+#include arch/generic/tlb.hh
 #include base/statistics.hh
 #include dev/dma_device.hh
 #include mem/request.hh
 #include params/ArmTLB.hh
 #include sim/probe/pmu.hh
-#include sim/tlb.hh
 
 class ThreadContext;
 
diff -r 1922f9d2ac01 -r 276da6265ab8 src/arch/generic/BaseTLB.py
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/src/arch/generic/BaseTLB.py   Wed Feb 11 10:23:27 2015 -0500
@@ -0,0 +1,34 @@
+# Copyright (c) 2008 The Hewlett-Packard Development Company
+# 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.
+#
+# Authors: Gabe Black
+
+from m5.SimObject import SimObject
+
+class BaseTLB(SimObject):
+type = 'BaseTLB'
+abstract = True
+cxx_header = arch/generic/tlb.hh
diff -r 1922f9d2ac01 -r 276da6265ab8 src/arch/generic/SConscript
--- a/src/arch/generic/SConscript   Wed Feb 11 10:23:24 2015 -0500
+++ b/src/arch/generic/SConscript   Wed Feb 11 10:23:27 2015 -0500
@@ -33,4 +33,9 @@
 
 Source('decode_cache.cc')
 Source('mmapped_ipr.cc')
+Source('tlb.cc')
+
+SimObject('BaseTLB.py')
+
+DebugFlag('TLB')
 Source('pseudo_inst.cc')
diff -r 1922f9d2ac01 -r 276da6265ab8 

[gem5-dev] changeset in gem5: style: Fix incorrect style checker option name

[Andreas Sandberg via gem5-dev]

changeset 65da28dee7cf in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=65da28dee7cf
description:
style: Fix incorrect style checker option name

The style used to support the option -w to automatically fix white
space issues. However, this option was actually wired up to fix all
styles issues the checker encountered. This changeset cleans up the
code that handles automatic fixing and adds an option to fix all
issues, and separate options for white spaces and include ordering.

diffstat:

 util/style.py |  33 ++---
 1 files changed, 22 insertions(+), 11 deletions(-)

diffs (83 lines):

diff -r 4972ada74310 -r 65da28dee7cf util/style.py
--- a/util/style.py Wed Feb 11 10:23:31 2015 -0500
+++ b/util/style.py Wed Feb 11 10:23:33 2015 -0500
@@ -126,14 +126,10 @@
 return mod_regions
 
 class UserInterface(object):
-def __init__(self, verbose=False, auto=False):
-self.auto = auto
+def __init__(self, verbose=False):
 self.verbose = verbose
 
 def prompt(self, prompt, results, default):
-if self.auto:
-return self.auto
-
 while True:
 result = self.do_prompt(prompt, results, default)
 if result in results:
@@ -436,10 +432,17 @@
 
 from mercurial import mdiff, util
 
-opt_fix_white = opts.get('fix_white', False)
+opt_fix_all = opts.get('fix_all', False)
+if not opt_fix_all:
+opt_fix_white = opts.get('fix_white', False)
+opt_fix_include = opts.get('fix_include', False)
+else:
+opt_fix_white = True
+opt_fix_include = True
+
 opt_all = opts.get('all', False)
 opt_no_ignore = opts.get('no_ignore', False)
-ui = MercurialUI(hgui, hgui.verbose, opt_fix_white)
+ui = MercurialUI(hgui, verbose=hgui.verbose)
 
 def prompt(name, func, regions=all_regions):
 result = ui.prompt((a)bort, (i)gnore, or (f)ix?, 'aif', 'a')
@@ -450,6 +453,12 @@
 
 return False
 
+def no_prompt(name, func, regions=all_regions):
+func(name, regions)
+return False
+
+prompt_white = prompt if not opt_fix_white else no_prompt
+prompt_include = prompt if not opt_fix_include else no_prompt
 
 # Import the match (repository file name matching helper)
 # function. Different versions of Mercurial keep it in different
@@ -481,16 +490,16 @@
 if not opt_no_ignore and check_ignores(fname):
 continue
 
-if whitespace.apply(fname, prompt, mod_regions):
+if whitespace.apply(fname, prompt_white, mod_regions):
 return True
 
-if sorted_includes.apply(fname, prompt, mod_regions):
+if sorted_includes.apply(fname, prompt_include, mod_regions):
 return True
 
 return False
 
 def do_check_format(hgui, repo, **args):
-ui = MercurialUI(hgui, hgui.verbose, auto)
+ui = MercurialUI(hgui, hgui.verbose)
 
 modified, added, removed, deleted, unknown, ignore, clean = repo.status()
 
@@ -544,7 +553,9 @@
 cmdtable = {
 '^m5style' : (
 do_check_style, [
-('w', 'fix-white', False, _(automatically fix whitespace)),
+('f', 'fix-all', False, _(automatically fix style issues)),
+('', 'fix-white', False, _(automatically fix white space 
issues)),
+('', 'fix-include', False, _(automatically fix include 
ordering)),
 ('a', 'all', False,
  _(include clean files and unmodified parts of modified files)),
 ('', 'no-ignore', False, _(ignore the style ignore list)),
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: base: Add compiler macros to add deprecation ...

[Andreas Sandberg via gem5-dev]

changeset 1922f9d2ac01 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=1922f9d2ac01
description:
base: Add compiler macros to add deprecation warnings

Gcc and clang both provide an attribute that can be used to flag a
function as deprecated at compile time. This changeset adds a gem5
compiler macro for that compiler feature. The macro can be used to
indicate that a legacy API within gem5 has been deprecated and provide
a graceful migration to the new API.

diffstat:

 src/SConscript   |  7 ++-
 src/base/compiler.hh |  2 ++
 2 files changed, 8 insertions(+), 1 deletions(-)

diffs (29 lines):

diff -r a24286e33318 -r 1922f9d2ac01 src/SConscript
--- a/src/SConscriptWed Feb 11 10:23:23 2015 -0500
+++ b/src/SConscriptWed Feb 11 10:23:24 2015 -0500
@@ -1061,7 +1061,12 @@
 new_env.Append(LINKFLAGS='-fsanitize=undefined')
 
 werror_env = new_env.Clone()
-werror_env.Append(CCFLAGS='-Werror')
+# Treat warnings as errors but white list some warnings that we
+# want to allow (e.g., deprecation warnings).
+werror_env.Append(CCFLAGS=['-Werror',
+   '-Wno-error=deprecated-declarations',
+   '-Wno-error=deprecated',
+   ])
 
 def make_obj(source, static, extra_deps = None):
 '''This function adds the specified source to the correct
diff -r a24286e33318 -r 1922f9d2ac01 src/base/compiler.hh
--- a/src/base/compiler.hh  Wed Feb 11 10:23:23 2015 -0500
+++ b/src/base/compiler.hh  Wed Feb 11 10:23:24 2015 -0500
@@ -80,6 +80,8 @@
 #  define M5_VAR_USED __attribute__((unused))
 #  define M5_ATTR_PACKED __attribute__ ((__packed__))
 #  define M5_NO_INLINE __attribute__ ((__noinline__))
+#  define M5_DEPRECATED __attribute__((deprecated))
+#  define M5_DEPRECATED_MSG(MSG) __attribute__((deprecated(MSG)))
 #endif
 
 #if defined(__clang__)
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2567: arm: Merge ISA files with pseudo instructions

[Andreas Sandberg via gem5-dev]

 On Dec. 12, 2014, 10:24 p.m., Gabe Black wrote:
  While it's definitely nice to get these into regular C++ instead of the ISA 
  language, my concern is that these aren't really pseudo instructions. They 
  are in the sense that they're instructions that wouldn't exist outside of 
  the simulator, but there's a well defined pseudo inst concept they don't 
  really fit in. Maybe put them in a .cc and .hh file with a different name?
 
 Andreas Sandberg wrote:
 I see your point. We could rename them to something like nonisa since 
 they aren't a part of the visible ISA. Would that be a good name? Another 
 option is to just say that everything in gem5 that isn't a proper ISA-defined 
 instruction is a pseudo instruction and that m5ops are just a subset of those.

Ping. Is renaming the files to nonisa.(cc|hh) an acceptable solution? Or should 
we submit as is?


- Andreas


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2567/#review5684
---


On Dec. 12, 2014, 5:45 p.m., Andreas Hansson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2567/
 ---
 
 (Updated Dec. 12, 2014, 5:45 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10612:2bd582d94965
 ---
 arm: Merge ISA files with pseudo instructions
 
 This changeset moves the pseudo instructions used to signal unknown
 instructions and unimplemented instructions to the same source files
 as the decoder fault.
 
 
 Diffs
 -
 
   src/arch/arm/insts/pseudo.hh PRE-CREATION 
   src/arch/arm/insts/pseudo.cc PRE-CREATION 
   src/arch/arm/isa/formats/formats.isa 8fc6e7a835d1 
   src/arch/arm/isa/formats/pseudo.isa PRE-CREATION 
   src/arch/arm/isa/formats/unimp.isa 8fc6e7a835d1 
   src/arch/arm/isa/formats/unknown.isa 8fc6e7a835d1 
 
 Diff: http://reviews.gem5.org/r/2567/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Andreas Hansson
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: style: Update the style checker to handle new...

[Andreas Sandberg via gem5-dev]

changeset e2f9644a7738 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=e2f9644a7738
description:
style: Update the style checker to handle new include order

As of August 2014, the gem5 style guide mandates that a source file's
primary header is included first in that source file. This helps to
ensure that the header file does not depend on include file ordering
and avoids surprises down the road when someone tries to reuse code.

In the new order, include files are grouped into the following blocks:
  * Primary header file (e.g., foo.hh for foo.cc)
  * Python headers
  * C system/stdlib includes
  * C++ stdlib includes
  * Include files in the gem5 source tree

Just like before, include files within a block are required to be
sorted in alphabetical order.

This changeset updates the style checker to enforce the new order.

diffstat:

 util/sort_includes.py |  251 ++---
 util/style.py |   21 +--
 2 files changed, 163 insertions(+), 109 deletions(-)

diffs (truncated from 356 to 300 lines):

diff -r a3cf30302e19 -r e2f9644a7738 util/sort_includes.py
--- a/util/sort_includes.py Tue Feb 03 14:25:48 2015 -0500
+++ b/util/sort_includes.py Tue Feb 03 14:25:50 2015 -0500
@@ -49,6 +49,64 @@
 
 return key
 
+
+def _include_matcher(keyword=#include, delim=):
+Match an include statement and return a (keyword, file, extra)
+duple, or a touple of None values if there isn't a match.
+
+rex = re.compile(r'^(%s)\s*%s(.*)%s(.*)$' % (keyword, delim[0], delim[1]))
+
+def matcher(context, line):
+m = rex.match(line)
+return m.groups() if m else (None, ) * 3
+
+return matcher
+
+def _include_matcher_fname(fname, **kwargs):
+Match an include of a specific file name. Any keyword arguments
+are forwarded to _include_matcher, which is used to match the
+actual include line.
+
+rex = re.compile(fname)
+base_matcher = _include_matcher(**kwargs)
+
+def matcher(context, line):
+(keyword, fname, extra) = base_matcher(context, line)
+if fname and rex.match(fname):
+return (keyword, fname, extra)
+else:
+return (None, ) * 3
+
+return matcher
+
+
+def _include_matcher_main():
+Match a C/C++ source file's primary header (i.e., a file with
+the same base name, but a header extension).
+
+base_matcher = _include_matcher(delim='')
+rex = re.compile(r^src/(.*)\.([^.]+)$)
+header_map = {
+c : h,
+cc : hh,
+cpp : hh,
+}
+def matcher(context, line):
+m = rex.match(context[filename])
+if not m:
+return (None, ) * 3
+base, ext = m.groups()
+(keyword, fname, extra) = base_matcher(context, line)
+try:
+if fname == %s.%s % (base, header_map[ext]):
+return (keyword, fname, extra)
+except KeyError:
+pass
+
+return (None, ) * 3
+
+return matcher
+
 class SortIncludes(object):
 # different types of includes for different sorting of headers
 # Python.h - Python header needs to be first if it exists
@@ -57,124 +115,123 @@
 # *.(hh|hxx|hpp|H) - C++ Headers (directories before files)
 # *- M5 headers (directories before files)
 includes_re = (
-('python', '', r'^(#include)[ \t]+(Python.*\.h)(.*)'),
-('c', '', r'^(#include)[ \t](.+\.h)(.*)'),
-('stl', '', r'^(#include)[ \t]+([0-9A-z_]+)(.*)'),
-('cc', '', r'^(#include)[ \t]+([0-9A-z_]+\.(hh|hxx|hpp|H))(.*)'),
-('m5cc', '', r'^(#include)[ \t](.+\.h{1,2})(.*)'),
-('swig0', '', r'^(%import)[ \t](.+)(.*)'),
-('swig1', '', r'^(%include)[ \t](.+)(.*)'),
-('swig2', '', r'^(%import)[ \t](.+)(.*)'),
-('swig3', '', r'^(%include)[ \t](.+)(.*)'),
+('main', '', _include_matcher_main()),
+('python', '', _include_matcher_fname(^Python\.h$)),
+('c', '', _include_matcher_fname(^.*\.h$)),
+('stl', '', _include_matcher_fname(^\w+$)),
+('cc', '', _include_matcher_fname(^.*\.(hh|hxx|hpp|H)$)),
+('m5header', '', _include_matcher_fname(^.*\.h{1,2}$, delim='')),
+('swig0', '', _include_matcher(keyword=%import)),
+('swig1', '', _include_matcher(keyword=%include)),
+('swig2', '', _include_matcher(keyword=%import, delim='')),
+('swig3', '', _include_matcher(keyword=%include, delim='')),
 )
 
-# compile the regexes
-includes_re = tuple((a, b, re.compile(c)) for a,b,c in includes_re)
+block_order = (
+('main', ),
+('python', ),
+('c', ),
+('stl', ),
+('cc', ),
+('m5header', ),
+('swig0', 'swig1', 'swig2', 'swig3', ),
+)
 
 def __init__(self):
-pass
+self.block_priority = {}

[gem5-dev] changeset in gem5: sim: Remove test for non-NULL this in Event

[Andreas Sandberg via gem5-dev]

changeset a3cf30302e19 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=a3cf30302e19
description:
sim: Remove test for non-NULL this in Event

The method Event::initialized() tests if this != NULL as a part of the
expression that tests if an event is initialized. The only case when
this check could be false is if the method is called on a null
pointer, which is illegal and leads to undefined behavior (such as
eating your pets) according to the C++ standard. Because of this,
modern compilers (specifically, recent versions of clang) warn about
this which we treat as an error. This changeset removes the redundant
check to fix said warning.

diffstat:

 src/sim/eventq.hh |  2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diffs (12 lines):

diff -r e2716d523716 -r a3cf30302e19 src/sim/eventq.hh
--- a/src/sim/eventq.hh Tue Feb 03 14:25:47 2015 -0500
+++ b/src/sim/eventq.hh Tue Feb 03 14:25:48 2015 -0500
@@ -239,7 +239,7 @@
 bool
 initialized() const
 {
-return this  (flags  InitMask) == Initialized;
+return (flags  InitMask) == Initialized;
 }
 
   protected:
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: dev: Correctly clear interrupts in VirtIO PCI

[Andreas Sandberg via gem5-dev]

changeset e2716d523716 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=e2716d523716
description:
dev: Correctly clear interrupts in VirtIO PCI

Correctly clear the PCI interrupt belonging to a VirtIO device when
the ISR register is read.

diffstat:

 src/dev/virtio/pci.cc |  7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diffs (17 lines):

diff -r d59e40b074c6 -r e2716d523716 src/dev/virtio/pci.cc
--- a/src/dev/virtio/pci.cc Tue Feb 03 14:25:43 2015 -0500
+++ b/src/dev/virtio/pci.cc Tue Feb 03 14:25:47 2015 -0500
@@ -123,8 +123,11 @@
   case OFF_ISR_STATUS: {
   DPRINTF(VIOPci,ISR_STATUS\n);
   assert(size == sizeof(uint8_t));
-  uint8_t isr_status(interruptDeliveryPending ? 1 : 0);
-  interruptDeliveryPending = false;
+  const uint8_t isr_status(interruptDeliveryPending ? 1 : 0);
+  if (interruptDeliveryPending) {
+  interruptDeliveryPending = false;
+  intrClear();
+  }
   pkt-setuint8_t(isr_status);
   } break;
 
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2614: style: Update the style checker to handle new include order

[Andreas Sandberg via gem5-dev]

 On Jan. 27, 2015, 11:57 p.m., Joel Hestness wrote:
  util/sort_includes.py, line 151
  http://reviews.gem5.org/r/2614/diff/1/?file=43359#file43359line151
 
  This and lines 154+155 are tricky to read also. The old code was much 
  clearer/easier to step through. Is it important for the code to be this 
  dense?

I've cleaned up this part of the code (and added a few comments), which should 
make it more readable. We'll upload new version shortly.


 On Jan. 27, 2015, 11:57 p.m., Joel Hestness wrote:
  util/sort_includes.py, line 75
  http://reviews.gem5.org/r/2614/diff/1/?file=43359#file43359line75
 
  The Python do X if Y else Z conditional formatting tends to be 
  unclear and tricky to read, as is the case here. It's also quite uncommon 
  in gem5 (in fact, I'm unable to quickly find another example in the 
  codebase), and using it in line 61 and here mixes code style with the more 
  standard use in analogous line 100. My preference would be to maintain the 
  clearer and consistent use of if-conditionals already common in gem5.

This is a pretty common Python construct that is completely analogous to the 
trinary operator in C, which we use a lot. I'm inclined to say that this is a 
non-issue.


- Andreas


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2614/#review5804
---


On Jan. 28, 2015, 11:56 a.m., Andreas Hansson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2614/
 ---
 
 (Updated Jan. 28, 2015, 11:56 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10669:e82b485546d1
 ---
 style: Update the style checker to handle new include order
 
 As of August 2014, the gem5 style guide mandates that a source file's
 primary header is included first in that source file. This helps to
 ensure that the header file does not depend on include file ordering
 and avoids surprises down the road when someone tries to reuse code.
 
 In the new order, include files are grouped into the following blocks:
   * Primary header file (e.g., foo.hh for foo.cc)
   * Python headers
   * C system/stdlib includes
   * C++ stdlib includes
   * Include files in the gem5 source tree
 
 Just like before, include files within a block are required to be
 sorted in alphabetical order.
 
 This changeset updates the style checker to enforce the new order.
 
 
 Diffs
 -
 
   util/sort_includes.py 3c42be107634 
   util/style.py 3c42be107634 
 
 Diff: http://reviews.gem5.org/r/2614/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Andreas Hansson
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2600: cpu: fix RetiredStores probe point

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2600/#review5745
---

Ship it!


Ooops, that was probably my fault.. Thanks for fixing!

- Andreas Sandberg


On Jan. 8, 2015, 3:13 p.m., Nikos Nikoleris wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2600/
 ---
 
 (Updated Jan. 8, 2015, 3:13 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10637:5b49f9b306c1
 ---
 cpu: fix RetiredStores probe point
 
 
 Diffs
 -
 
   src/cpu/base.cc 9ac724889705 
 
 Diff: http://reviews.gem5.org/r/2600/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Nikos Nikoleris
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: arm: Add support for filtering in the PMU

[Andreas Sandberg via gem5-dev]

changeset ae5582819481 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=ae5582819481
description:
arm: Add support for filtering in the PMU

This patch adds support for filtering events in the PMU. In order to
do so, it updates the ISADevice base class to forward an ISA pointer
to ISA devices. This enables such devices to access the MiscReg file
to determine the current execution level.

diffstat:

 src/arch/arm/isa.cc|   3 ++
 src/arch/arm/isa_device.cc |  13 
 src/arch/arm/isa_device.hh |   9 +++-
 src/arch/arm/pmu.cc|  49 +++--
 src/arch/arm/pmu.hh|  13 +++-
 5 files changed, 78 insertions(+), 9 deletions(-)

diffs (196 lines):

diff -r 427f988fe6e5 -r ae5582819481 src/arch/arm/isa.cc
--- a/src/arch/arm/isa.cc   Tue Dec 23 09:31:17 2014 -0500
+++ b/src/arch/arm/isa.cc   Tue Dec 23 09:31:17 2014 -0500
@@ -139,6 +139,9 @@
 if (!pmu)
 pmu = dummyDevice;
 
+// Give all ISA devices a pointer to this ISA
+pmu-setISA(this);
+
 system = dynamic_castArmSystem *(p-system);
 DPRINTFN(ISA system set to: %p %p\n, system, p-system);
 
diff -r 427f988fe6e5 -r ae5582819481 src/arch/arm/isa_device.cc
--- a/src/arch/arm/isa_device.ccTue Dec 23 09:31:17 2014 -0500
+++ b/src/arch/arm/isa_device.ccTue Dec 23 09:31:17 2014 -0500
@@ -44,6 +44,19 @@
 namespace ArmISA
 {
 
+BaseISADevice::BaseISADevice()
+: isa(nullptr)
+{
+}
+
+void
+BaseISADevice::setISA(ISA *_isa)
+{
+assert(_isa);
+
+isa = _isa;
+}
+
 void
 DummyISADevice::setMiscReg(int misc_reg, MiscReg val)
 {
diff -r 427f988fe6e5 -r ae5582819481 src/arch/arm/isa_device.hh
--- a/src/arch/arm/isa_device.hhTue Dec 23 09:31:17 2014 -0500
+++ b/src/arch/arm/isa_device.hhTue Dec 23 09:31:17 2014 -0500
@@ -46,6 +46,8 @@
 namespace ArmISA
 {
 
+class ISA;
+
 /**
  * Base class for devices that use the MiscReg interfaces.
  *
@@ -56,9 +58,11 @@
 class BaseISADevice
 {
   public:
-BaseISADevice() {}
+BaseISADevice();
 virtual ~BaseISADevice() {}
 
+virtual void setISA(ISA *isa);
+
 /**
  * Write to a system register belonging to this device.
  *
@@ -74,6 +78,9 @@
  * @return Register value.
  */
 virtual MiscReg readMiscReg(int misc_reg) = 0;
+
+  protected:
+ISA *isa;
 };
 
 /**
diff -r 427f988fe6e5 -r ae5582819481 src/arch/arm/pmu.cc
--- a/src/arch/arm/pmu.cc   Tue Dec 23 09:31:17 2014 -0500
+++ b/src/arch/arm/pmu.cc   Tue Dec 23 09:31:17 2014 -0500
@@ -41,6 +41,8 @@
 
 #include arch/arm/pmu.hh
 
+#include arch/arm/isa.hh
+#include arch/arm/utility.hh
 #include base/trace.hh
 #include cpu/base.hh
 #include debug/Checkpoint.hh
@@ -350,12 +352,44 @@
 }
 }
 
+bool
+PMU::isFiltered(const CounterState ctr) const
+{
+assert(isa);
+
+const PMEVTYPER_t filter(ctr.filter);
+const SCR scr(isa-readMiscRegNoEffect(MISCREG_SCR));
+const CPSR cpsr(isa-readMiscRegNoEffect(MISCREG_CPSR));
+const ExceptionLevel el(opModeToEL((OperatingMode)(uint8_t)cpsr.mode));
+const bool secure(inSecureState(scr, cpsr));
+
+switch (el) {
+  case EL0:
+return secure ? filter.u : (filter.u != filter.nsu);
+
+  case EL1:
+return secure ? filter.p : (filter.p != filter.nsk);
+
+  case EL2:
+return !filter.nsh;
+
+  case EL3:
+return filter.p != filter.m;
+
+  default:
+panic(Unexpected execution level in PMU::isFiltered.\n);
+}
+}
+
 void
 PMU::handleEvent(CounterId id, uint64_t delta)
 {
 CounterState ctr(getCounter(id));
 const bool overflowed(reg_pmovsr  (1  id));
 
+if (isFiltered(ctr))
+return;
+
 // Handle the count every 64 cycles mode
 if (id == PMCCNTR  reg_pmcr.d) {
 clock_remainder += delta;
@@ -434,9 +468,8 @@
 return 0;
 
 const CounterState cs(getCounter(id));
-PMEVTYPER_t type(0);
+PMEVTYPER_t type(cs.filter);
 
-// TODO: Re-create filtering settings from counter state
 type.evtCount = cs.eventId;
 
 return type;
@@ -453,12 +486,14 @@
 }
 
 CounterState ctr(getCounter(id));
-// TODO: Handle filtering (both for general purpose counters and
-// the cycle counter)
+const EventTypeId old_event_id(ctr.eventId);
 
-// If PMCCNTR Register, do not change event type. PMCCNTR can count
-// processor cycles only.
-if (id != PMCCNTR) {
+ctr.filter = val;
+
+// If PMCCNTR Register, do not change event type. PMCCNTR can
+// count processor cycles only. If we change the event type, we
+// need to update the probes the counter is using.
+if (id != PMCCNTR  old_event_id != val.evtCount) {
 ctr.eventId = val.evtCount;
 updateCounter(reg_pmselr.sel, ctr);
 }
diff -r 427f988fe6e5 -r ae5582819481 src/arch/arm/pmu.hh
--- a/src/arch/arm/pmu.hh   Tue Dec 23 09:31:17 2014 -0500
+++ b/src/arch/arm/pmu.hh   

[gem5-dev] changeset in gem5: arm: Clean up and document decoder API

[Andreas Sandberg via gem5-dev]

changeset 5fae03bd840a in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=5fae03bd840a
description:
arm: Clean up and document decoder API

This changeset adds more documentation to the ArmISA::Decoder class
and restructures it slightly to make API groups more obvious.

diffstat:

 src/arch/arm/decoder.cc |   52 +++-
 src/arch/arm/decoder.hh |  197 +++
 2 files changed, 162 insertions(+), 87 deletions(-)

diffs (truncated from 302 to 300 lines):

diff -r ae5582819481 -r 5fae03bd840a src/arch/arm/decoder.cc
--- a/src/arch/arm/decoder.cc   Tue Dec 23 09:31:17 2014 -0500
+++ b/src/arch/arm/decoder.cc   Tue Dec 23 09:31:17 2014 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2013 ARM Limited
+ * Copyright (c) 2012-2014 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -51,6 +51,23 @@
 
 GenericISA::BasicDecodeCache Decoder::defaultCache;
 
+Decoder::Decoder()
+: data(0), fpscrLen(0), fpscrStride(0)
+{
+reset();
+}
+
+void
+Decoder::reset()
+{
+bigThumb = false;
+offset = 0;
+emi = 0;
+instDone = false;
+outOfBytes = true;
+foundIt = false;
+}
+
 void
 Decoder::process()
 {
@@ -118,8 +135,15 @@
 }
 }
 
-//Use this to give data to the decoder. This should be used
-//when there is control flow.
+void
+Decoder::consumeBytes(int numBytes)
+{
+offset += numBytes;
+assert(offset = sizeof(MachInst));
+if (offset == sizeof(MachInst))
+outOfBytes = true;
+}
+
 void
 Decoder::moreBytes(const PCState pc, Addr fetchPC, MachInst inst)
 {
@@ -134,4 +158,26 @@
 process();
 }
 
+StaticInstPtr
+Decoder::decode(ArmISA::PCState pc)
+{
+if (!instDone)
+return NULL;
+
+const int inst_size((!emi.thumb || emi.bigThumb) ? 4 : 2);
+ExtMachInst this_emi(emi);
+
+pc.npc(pc.pc() + inst_size);
+if (foundIt)
+pc.nextItstate(itBits);
+this_emi.itstate = pc.itstate();
+pc.size(inst_size);
+
+emi = 0;
+instDone = false;
+foundIt = false;
+
+return decode(this_emi, pc.instAddr());
 }
+
+}
diff -r ae5582819481 -r 5fae03bd840a src/arch/arm/decoder.hh
--- a/src/arch/arm/decoder.hh   Tue Dec 23 09:31:17 2014 -0500
+++ b/src/arch/arm/decoder.hh   Tue Dec 23 09:31:17 2014 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013 ARM Limited
+ * Copyright (c) 2013-2014 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -70,100 +70,129 @@
 int fpscrLen;
 int fpscrStride;
 
-  public:
-void reset()
+/// A cache of decoded instruction objects.
+static GenericISA::BasicDecodeCache defaultCache;
+
+/**
+ * Pre-decode an instruction from the current state of the
+ * decoder.
+ */
+void process();
+
+/**
+ * Consume bytes by moving the offset into the data word and
+ * sanity check the results.
+ */
+void consumeBytes(int numBytes);
+
+  public: // Decoder API
+Decoder();
+
+/** Reset the decoders internal state. */
+void reset();
+
+/**
+ * Can the decoder accept more data?
+ *
+ * A CPU model uses this method to determine if the decoder can
+ * accept more data. Note that an instruction can be ready (see
+ * instReady() even if this method returns true.
+ */
+bool needMoreBytes() const { return outOfBytes; }
+
+/**
+ * Is an instruction ready to be decoded?
+ *
+ * CPU models call this method to determine if decode() will
+ * return a new instruction on the next call. It typically only
+ * returns false if the decoder hasn't received enough data to
+ * decode a full instruction.
+ */
+bool instReady() const { return instDone; }
+
+/**
+ * Feed data to the decoder.
+ *
+ * A CPU model uses this interface to load instruction data into
+ * the decoder. Once enough data has been loaded (check with
+ * instReady()), a decoded instruction can be retrieved using
+ * decode(ArmISA::PCState).
+ *
+ * This method is intended to support both fixed-length and
+ * variable-length instructions. Instruction data is fetch in
+ * MachInst blocks (which correspond to the size of a typical
+ * insturction). The method might need to be called multiple times
+ * if the instruction spans multiple blocks, in that case
+ * needMoreBytes() will return true and instReady() will return
+ * false.
+ *
+ * The fetchPC parameter is used to indicate where in memory the
+ * instruction was fetched from. This is should be the same
+ * address as the pc. If fetching multiple blocks, it indicates
+ * where subsequent blocks are fetched from (pc + n *
+ * sizeof(MachInst)).
+ *
+ * @param pc Instruction pointer that we are decoding.
+ * @param fetchPC The address this chunk was fetched from.
+ * @param inst Raw 

[gem5-dev] changeset in gem5: arm: Raise an alignment fault if a PC has ill...

[Andreas Sandberg via gem5-dev]

changeset 3bba9f2d0c7d in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=3bba9f2d0c7d
description:
arm: Raise an alignment fault if a PC has illegal alignment

We currently don't handle unaligned PCs correctly. There is one check
for unaligned PCs in the TLB when running in aarch64 mode, but this
check does not cover cases where the CPU does not do a TLB lookup when
decoding an instruction (e.g., a branch stays within the same cache
line). Additionally, the Decoder class sometimes throws an assertion
for unaligned PCs which breaks speculation.

This changeset introduces a decoder fault bit field in the ExtMachInst
structure. This field can be used to signal a decoder failure. If set,
the decoder generates an internal gem5fault instruction instead of a
normal instruction. This instruction in turns either panics (fault
type PANIC), returns an PCAlignmentFault (fault type UNALIGNED,
aarch64) or PrefetchAbort (fault type UNALIGNED, aarch32).

The patch causes minor changes to the realview64 regressions, and a
stats bump will follow.

diffstat:

 src/arch/arm/SConscript  |1 +
 src/arch/arm/decoder.cc  |6 +-
 src/arch/arm/insts/pseudo.cc |  101 +++
 src/arch/arm/insts/pseudo.hh |   61 +
 src/arch/arm/isa/bitfields.isa   |2 +
 src/arch/arm/isa/decoder/decoder.isa |   20 +++---
 src/arch/arm/isa/formats/formats.isa |3 +
 src/arch/arm/isa/formats/pseudo.isa  |   44 +++
 src/arch/arm/isa/includes.isa|1 +
 src/arch/arm/tlb.cc  |5 -
 src/arch/arm/types.hh|   14 -
 11 files changed, 242 insertions(+), 16 deletions(-)

diffs (truncated from 360 to 300 lines):

diff -r 5fae03bd840a -r 3bba9f2d0c7d src/arch/arm/SConscript
--- a/src/arch/arm/SConscript   Tue Dec 23 09:31:17 2014 -0500
+++ b/src/arch/arm/SConscript   Tue Dec 23 09:31:17 2014 -0500
@@ -57,6 +57,7 @@
 Source('insts/misc.cc')
 Source('insts/misc64.cc')
 Source('insts/pred_inst.cc')
+Source('insts/pseudo.cc')
 Source('insts/static_inst.cc')
 Source('insts/vfp.cc')
 Source('insts/fplib.cc')
diff -r 5fae03bd840a -r 3bba9f2d0c7d src/arch/arm/decoder.cc
--- a/src/arch/arm/decoder.cc   Tue Dec 23 09:31:17 2014 -0500
+++ b/src/arch/arm/decoder.cc   Tue Dec 23 09:31:17 2014 -0500
@@ -139,7 +139,7 @@
 Decoder::consumeBytes(int numBytes)
 {
 offset += numBytes;
-assert(offset = sizeof(MachInst));
+assert(offset = sizeof(MachInst) || emi.decoderFault);
 if (offset == sizeof(MachInst))
 outOfBytes = true;
 }
@@ -154,6 +154,10 @@
 emi.fpscrLen = fpscrLen;
 emi.fpscrStride = fpscrStride;
 
+const Addr alignment(pc.thumb() ? 0x1 : 0x3);
+emi.decoderFault = static_castuint8_t(
+pc.instAddr()  alignment ? DecoderFault::UNALIGNED : 
DecoderFault::OK);
+
 outOfBytes = false;
 process();
 }
diff -r 5fae03bd840a -r 3bba9f2d0c7d src/arch/arm/insts/pseudo.cc
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/src/arch/arm/insts/pseudo.cc  Tue Dec 23 09:31:17 2014 -0500
@@ -0,0 +1,101 @@
+/*
+ * Copyright (c) 2014 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.
+ *
+ * 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 

[gem5-dev] changeset in gem5: dev: Add response sanity checks in PioPort

[Andreas Sandberg via gem5-dev]

changeset 6099331da328 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=6099331da328
description:
dev: Add response sanity checks in PioPort

Add an assert in the PioPort that checks if a response packet from a
device has the right flags set before passing it to them rest of the
memory system.

diffstat:

 src/dev/io_device.cc |  4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diffs (14 lines):

diff -r 3499de20ab3a -r 6099331da328 src/dev/io_device.cc
--- a/src/dev/io_device.cc  Mon Dec 08 04:49:51 2014 -0500
+++ b/src/dev/io_device.cc  Mon Dec 08 04:49:52 2014 -0500
@@ -57,7 +57,9 @@
 // @todo: We need to pay for this and not just zero it out
 pkt-firstWordDelay = pkt-lastWordDelay = 0;
 
-return pkt-isRead() ? device-read(pkt) : device-write(pkt);
+const Tick delay(pkt-isRead() ? device-read(pkt) : device-write(pkt));
+assert(pkt-isResponse() || pkt-isError());
+return delay;
 }
 
 AddrRangeList
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: arm: Fix decoding of PMXEVTYPER_EL0 and PMCCF...

[Andreas Sandberg via gem5-dev]

changeset 4e09ae443c96 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=4e09ae443c96
description:
arm: Fix decoding of PMXEVTYPER_EL0 and PMCCFILTR_EL0

The aarch64 system register decoder is currently not decoding
PMXEVTYPER_EL0 and PMCCFILTR_EL0 correctly. This changeset updates the
decoder so that they are decoded using the values in table C5-6 in ARM
DDI 0478A.c.

diffstat:

 src/arch/arm/miscregs.cc |  7 ++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diffs (24 lines):

diff -r 6099331da328 -r 4e09ae443c96 src/arch/arm/miscregs.cc
--- a/src/arch/arm/miscregs.cc  Mon Dec 08 04:49:52 2014 -0500
+++ b/src/arch/arm/miscregs.cc  Mon Dec 08 04:49:53 2014 -0500
@@ -3177,7 +3177,7 @@
   case 0:
 return MISCREG_PMCCNTR_EL0;
   case 1:
-return MISCREG_PMCCFILTR_EL0;
+return MISCREG_PMXEVTYPER_EL0;
   case 2:
 return MISCREG_PMXEVCNTR_EL0;
 }
@@ -3434,6 +3434,11 @@
 return MISCREG_PMEVTYPER5_EL0;
 }
 break;
+  case 15:
+switch (op2) {
+  case 7:
+return MISCREG_PMCCFILTR_EL0;
+}
 }
 break;
   case 4:
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: dev: Correctly transform packets into responses

[Andreas Sandberg via gem5-dev]

changeset 3499de20ab3a in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=3499de20ab3a
description:
dev: Correctly transform packets into responses

The VirtIO devices didn't correctly set the response flags in memory
packets. This changeset adds the required Packet::makeResponse()
calls.

diffstat:

 src/dev/virtio/base.cc |  2 ++
 src/dev/virtio/pci.cc  |  4 
 2 files changed, 6 insertions(+), 0 deletions(-)

diffs (40 lines):

diff -r 6efb37480d87 -r 3499de20ab3a src/dev/virtio/base.cc
--- a/src/dev/virtio/base.ccFri Dec 05 22:37:03 2014 -0800
+++ b/src/dev/virtio/base.ccMon Dec 08 04:49:51 2014 -0500
@@ -426,6 +426,7 @@
 if (cfgOffset + size  configSize)
 panic(Config read out of bounds.\n);
 
+pkt-makeResponse();
 pkt-setData(const_castuint8_t *(cfg) + cfgOffset);
 }
 
@@ -437,6 +438,7 @@
 if (cfgOffset + size  configSize)
 panic(Config write out of bounds.\n);
 
+pkt-makeResponse();
 pkt-writeData((uint8_t *)cfg + cfgOffset);
 }
 
diff -r 6efb37480d87 -r 3499de20ab3a src/dev/virtio/pci.cc
--- a/src/dev/virtio/pci.cc Fri Dec 05 22:37:03 2014 -0800
+++ b/src/dev/virtio/pci.cc Mon Dec 08 04:49:51 2014 -0500
@@ -75,6 +75,8 @@
 return 0;
 }
 
+pkt-makeResponse();
+
 switch(offset) {
   case OFF_DEVICE_FEATURES:
 DPRINTF(VIOPci,DEVICE_FEATURES request\n);
@@ -151,6 +153,8 @@
 return 0;
 }
 
+pkt-makeResponse();
+
 switch(offset) {
   case OFF_DEVICE_FEATURES:
 warn(Guest tried to write device features.);
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2510/#review5652
---

Ship it!


I'm still not happy with the setupMemSlot()/disableMemSlot() names, but I can't 
think of any better names atm, so let's go with them for now.

- Andreas Sandberg


On Nov. 23, 2014, 2:51 p.m., Gabe Black wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2510/
 ---
 
 (Updated Nov. 23, 2014, 2:51 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10551:7767dc21318d
 ---
 Let other objects set up memory like regions in a KVM VM.
 
 
 Diffs
 -
 
   src/cpu/kvm/vm.hh f9fb64a72259a2514080151b5250a04c575d443a 
   src/cpu/kvm/vm.cc f9fb64a72259a2514080151b5250a04c575d443a 
 
 Diff: http://reviews.gem5.org/r/2510/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Gabe Black
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] KVM CPU when using multiple cores

[Andreas Sandberg via gem5-dev]

On 04/12/14 16:10, Nilay Vaish via gem5-dev wrote:

I have been trying to run ht kvm cpu when using multiple cores.  With
single threaded simulation, the simulation stops making progress if the
simulated system has more than 4 cores.  With multi-threaded simulation, I
do not see any progress even when two cores are being simulated.  For the
multi-threaded simulation, I made the following changes as suggested in
the comment for the changeset:   10157:5c2ecad1a3c9.  So, how many cores
have others tested kvm cpu with?  Is there something that I might not be
doing right?


I reported scalability numbers up to 8 cores for one of the Splash 2
benchmarks in my thesis, so 8 cores definitely work. IIRC, I tested it
on 32 cores as well, but I didn't report those numbers.

There are three issues you might be running into:

 * There might be devices (CPU child objects) that don't live in the
right thread.
 * The quantum might be too large (I never managed to get anything more
than 1ms to work).
 * Newly introduced bugs.

The code fragment I used in my old scripts was something like this:

if not no_kvm and cpus  1:
test_sys.eventq_index = 0
for idx, cpu in enumerate(test_sys.cpu_boot):
for obj in cpu.descendants():
obj.eventq_index = test_sys.eventq_index
cpu.eventq_index = idx + 1

The fragment above ensures that any descendants of the CPU are assigned
to the device thread and only the CPU lives in a separate thread.

//Andreas


-- 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.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in 
England  Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England  Wales, Company No:  2548782

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] KVM CPU when using multiple cores

[Andreas Sandberg via gem5-dev]

PDES in gem5 is implemented using a fairly standard quantum-based PDES 
approach (similar to WWT) where a barrier across all threads is enforced 
every N cycles. Objects (in practice, sub-trees of the object graph) can 
be assigned to their own event queues. Cross-queue scheduling of events 
is supported as long as the new events are scheduled at least N ticks 
into the future to ensure determinism.


To enable the  PDES functionality in gem5, you need to do two things 
from your configuration script: 1) Per-object queue assignment (the 
eventq_index property) and 2) set the simulation quantum on  the root 
object (the sim_quantum property). By default, all objects are assigned 
to event queue 0 and new queues are created as soon as the simulator 
encounters an object with a new queue ID.


You should keep in mind that very few objects support communication 
across parallel event queues. I have only used it to simulate multiple 
cores in parallel in KVM, but I think Steve has used it to simulate two 
parallel systems communicating over Ethernet. Since gem5 has a tendency 
to make unsynchronized cross-object method calls, you generally can't 
have devices in the same system in multiple different threads.


KVM abuses barrier synchronization in gem5 slightly, making it behave 
more like the kind of relaxed synchronization you find in Graphite. 
Since memory accesses in KVM are instantaneous, execution will always be 
correct. Synchronization will only be needed to keep devices in sync 
across cores. The way we solve inter-thread calls is by migrating to the 
target object's event queue (the interrupt controller's queue for 
interrupts and the VM's queue for MMIO). Is implemented by releasing the 
current event queue's lock, taking the target thread's lock, and then 
updating the thread's current event queue pointer. By releasing the lock 
of the current queue, we avoid multiple common deadlock scenarios.


It's probably possible to implement something similar to the relaxed 
synchronization approach I used for KVM in the atomic CPU (especially 
when using fastmem). The main problem here is probably the decode cache. 
Doing it with a proper memory system on the other hand is likely going 
to be very challenging due to express snoops.


//Andreas

On 2014-12-04 23:08, Gabe Black via gem5-dev wrote:

How do you set that up? Does it happen automatically? That sounds pretty
handy.

Gabe

On Thu, Dec 4, 2014 at 3:01 PM, Nilay Vaish via gem5-dev gem5-dev@gem5.org
wrote:


The simulator.  As in different cores of the simulated system are
simulated on different threads of the host system.

--
Nilay


On Thu, 4 Dec 2014, Gabe Black via gem5-dev wrote:

  This is somewhat tangential, but are you saying the simulator is

multithreaded now? Or just your simulation?

Gabe

On Thu, Dec 4, 2014 at 10:03 AM, Andreas Sandberg via gem5-dev 
gem5-dev@gem5.org wrote:

  On 04/12/14 16:10, Nilay Vaish via gem5-dev wrote:

  I have been trying to run ht kvm cpu when using multiple cores.  With

single threaded simulation, the simulation stops making progress if the
simulated system has more than 4 cores.  With multi-threaded
simulation, I
do not see any progress even when two cores are being simulated.  For
the
multi-threaded simulation, I made the following changes as suggested in
the comment for the changeset:   10157:5c2ecad1a3c9.  So, how many cores
have others tested kvm cpu with?  Is there something that I might not be
doing right?



I reported scalability numbers up to 8 cores for one of the Splash 2
benchmarks in my thesis, so 8 cores definitely work. IIRC, I tested it
on 32 cores as well, but I didn't report those numbers.

There are three issues you might be running into:

  * There might be devices (CPU child objects) that don't live in the
right thread.
  * The quantum might be too large (I never managed to get anything more
than 1ms to work).
  * Newly introduced bugs.

The code fragment I used in my old scripts was something like this:

 if not no_kvm and cpus  1:
 test_sys.eventq_index = 0
 for idx, cpu in enumerate(test_sys.cpu_boot):
 for obj in cpu.descendants():
 obj.eventq_index = test_sys.eventq_index
 cpu.eventq_index = idx + 1

The fragment above ensures that any descendants of the CPU are assigned
to the device thread and only the CPU lives in a separate thread.

//Andreas


-- 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.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
Registered in England  Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
Registered in England  Wales, Company No:  2548782

[gem5-dev] KVM broken on x86 due to changeset 1bd64b294fe4

[Andreas Sandberg via gem5-dev]

I just ran into some issues with kvm on x86. It seems like changeset
1bd64b294fe4 (x86: add LongModeAddressSize function to cpuid) breaks
Linux running in kvm. It seems like the offending change makes cpuid
report the virtual and physical address length as 255 bits, which
clearly doesn't make sense.

I can't justify spending any time on it, so could someone else have a
quick look? I'd suggest just reverting the patch for now unless someone
else has a better suggestion.

//Andreas


-- 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.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in 
England  Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England  Wales, Company No:  2548782

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] KVM broken on x86 due to changeset 1bd64b294fe4

[Andreas Sandberg via gem5-dev]

Thanks! I should have checked the upstream commit log.

//Andreas

On 26/11/14 13:54, Gabe Black via gem5-dev wrote:

I already fixed it. The change should be checked in, I think.

Gabe
On Nov 26, 2014 5:52 AM, Andreas Sandberg via gem5-dev gem5-dev@gem5.org
wrote:


I just ran into some issues with kvm on x86. It seems like changeset
1bd64b294fe4 (x86: add LongModeAddressSize function to cpuid) breaks
Linux running in kvm. It seems like the offending change makes cpuid
report the virtual and physical address length as 255 bits, which
clearly doesn't make sense.

I can't justify spending any time on it, so could someone else have a
quick look? I'd suggest just reverting the patch for now unless someone
else has a better suggestion.

//Andreas


-- 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.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
Registered in England  Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
Registered in England  Wales, Company No:  2548782

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev




-- 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.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in 
England  Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England  Wales, Company No:  2548782

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2515: x86: pc: Put a stub IO device at port 0xed which the kernel can use for delays.

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2515/#review5499
---

Ship it!


Ship It!

- Andreas Sandberg


On Nov. 19, 2014, 11:51 p.m., Gabe Black wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2515/
 ---
 
 (Updated Nov. 19, 2014, 11:51 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10550:dbbee17f23e1
 ---
 x86: pc: Put a stub IO device at port 0xed which the kernel can use for 
 delays.
 
 There was already a stub device at 0x80, the port traditionally used for an IO
 delay. 0x80 is also the port used for POST codes sent by firmware, and that
 may have prompted adding this port as a second option.
 
 
 Diffs
 -
 
   src/dev/x86/Pc.py 288eb5ee4b0026d0cc1f02ec31748e0eaac06bc3 
 
 Diff: http://reviews.gem5.org/r/2515/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Gabe Black
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2313: kvm, x86: Adding support for SE mode execution

[Andreas Sandberg via gem5-dev]

 On Nov. 18, 2014, 10:43 p.m., Gabe Black wrote:
  util/m5/m5ops.h, line 57
  http://reviews.gem5.org/r/2313/diff/4/?file=42082#file42082line57
 
  Why do we need psuedo ops for syscalls when there are actual syscall 
  instructions? The same goes for page faults. I'm not saying I know that we 
  don't, I'm just surprised that that would be necessary.
  
  It seems unlikely that a program is going to realize it's about to 
  cause a pagefault and obligingly call a special instruction first.

The problem is that KVM doesn't normally intercept syscalls. This means that 
they need a small syscall handler and fault handler stub to support SE mode in 
KVM (see RB #2322). There are other ways of communicating with the host (e.g., 
IO ports, hypervisor calls). The beauty of this solution is that it is 
supported seamlessly by the simulated CPUs, which makes it possible to do CPU 
switching reliably (we need to handle situation where the switch takes place 
while the stub code is executing). It should be fairly straight forward to 
extend this to simulated CPUs, making mixed FS/SE mode simulation possible.


 On Nov. 18, 2014, 10:43 p.m., Gabe Black wrote:
  src/arch/x86/pseudo_inst.hh, line 37
  http://reviews.gem5.org/r/2313/diff/4/?file=42075#file42075line37
 
  These should be camel case, not all lower case. ie. m5Syscall and 
  m5PageFault, depending on what you consider the boundary between words. 
  These should also be called gem5*.

I'd argue that we should stay with m5* in this case since the rest of the 
functions that are gem5-specific in the pseudo-inst interface is using that 
prefix. Having said that, we should probably rename them all at some point.


- Andreas


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2313/#review5480
---


On Sept. 30, 2014, 5:21 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2313/
 ---
 
 (Updated Sept. 30, 2014, 5:21 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10421:d4162e1b1c56
 ---
 kvm, x86: Adding support for SE mode execution
 This patch adds methods in KvmCPU model to handle KVM exits caused by syscall
 instructions and page faults. These types of exits will be encountered if
 KvmCPU is run in SE mode.
 
 
 Diffs
 -
 
   src/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/alpha/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/arm/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/arm/pseudo_inst.hh PRE-CREATION 
   src/arch/arm/pseudo_inst.cc PRE-CREATION 
   src/arch/mips/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/power/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/sparc/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/x86/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/x86/pseudo_inst.hh PRE-CREATION 
   src/arch/x86/pseudo_inst.cc PRE-CREATION 
   src/arch/x86/tlb.cc 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/x86/utility.cc 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/cpu/kvm/base.cc 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/sim/pseudo_inst.cc 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/sim/system.cc 28b31101d9e6e5e75d04448451986d6318383f3c 
   util/m5/m5ops.h 28b31101d9e6e5e75d04448451986d6318383f3c 
 
 Diff: http://reviews.gem5.org/r/2313/diff/
 
 
 Testing
 ---
 
 Quick regressions passed.
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2313: kvm, x86: Adding support for SE mode execution

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2313/#review5487
---



src/arch/arm/pseudo_inst.cc
http://reviews.gem5.org/r/2313/#comment4939

I'd suggest that you simplify this and get rid of all the arch-specific 
panic code and write it once in arch/generic/pseudo_inst.(cc|hh) and then 
import it in arch/XX/pseudo_inst.hh.

See arch/mips/mmapped_ipr.hh and arch/generic/mmapped_ipr.hh for an example.


Except for the issues pointed out by Gabe and Nilay, I think this looks good.

- Andreas Sandberg


On Sept. 30, 2014, 5:21 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2313/
 ---
 
 (Updated Sept. 30, 2014, 5:21 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10421:d4162e1b1c56
 ---
 kvm, x86: Adding support for SE mode execution
 This patch adds methods in KvmCPU model to handle KVM exits caused by syscall
 instructions and page faults. These types of exits will be encountered if
 KvmCPU is run in SE mode.
 
 
 Diffs
 -
 
   src/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/alpha/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/arm/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/arm/pseudo_inst.hh PRE-CREATION 
   src/arch/arm/pseudo_inst.cc PRE-CREATION 
   src/arch/mips/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/power/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/sparc/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/x86/SConscript 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/x86/pseudo_inst.hh PRE-CREATION 
   src/arch/x86/pseudo_inst.cc PRE-CREATION 
   src/arch/x86/tlb.cc 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/arch/x86/utility.cc 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/cpu/kvm/base.cc 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/sim/pseudo_inst.cc 28b31101d9e6e5e75d04448451986d6318383f3c 
   src/sim/system.cc 28b31101d9e6e5e75d04448451986d6318383f3c 
   util/m5/m5ops.h 28b31101d9e6e5e75d04448451986d6318383f3c 
 
 Diff: http://reviews.gem5.org/r/2313/diff/
 
 
 Testing
 ---
 
 Quick regressions passed.
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2513: KVM: Build in most of the KVM stuff even if we're not going to use it.

[Andreas Sandberg via gem5-dev]

On 18/11/14 13:22, Steve Reinhardt via gem5-dev wrote:

I haven't looked at the code in question, so I'm just going by what I've
seen in this email thread.  However, it seems like there ought to be some
alternative solutions here.  I like the general direction Andreas is going,
though it would be nice to avoid more multiple inheritance :).  The way I
see it, the basic idea there is to create an API (either on an existing
object like System or on a new object) that the device can call
irrespective of whether KVM is configured or not, but which gives enough
information to get the job done; then the other object can be responsible
for either coordinating with KVM or (presumably) ignoring all those calls
if KVM is not configured.

As a simpler alternative, maybe we don't need to give the kvm pointer to
the device via python; if the System object has an accessor that would
return the vm pointer, then the device could call that during
initialization, and it would of course just return NULL if kvm is not
configured


I would actually see it as a requirement of the new API that the
underlying kvm vm is not exposed to device models. Exposing the VM would
make the code base harder to maintain and extend (what if we want to add
support for another virtualization interface?).

The idea of adding a few more APIs to the System class seems good to me.
We already use it to coordinate physical memories. As far as I can tell
from the description a minimal API could look something like this:

void System::mapDeviceMemory(DeviceMemory mem)
void System::unmapDeviceMemory(DeviceMemory mem)

void System::registerMemoryCallbacks(const MemoryCallbackInterface
callbacks)

This would make it possible to get rid of the dependency between devices
and kvm. The kvm VM, or any other device that needs the information,
would just register a set of callbacks with the system (probably just
map/unmap notifications) and the device would just talk to the system
and be virtualization agnostic.

//Andreas


-- 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.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in 
England  Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England  Wales, Company No:  2548782

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2514: scons: Make the USE_KVM variable available in C++.

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2514/#review5488
---


The change itself makes sense, but I'd really prefer if we could avoid 
conditional compilation and use a kvm-agnostic interface to handle device 
memory. See my reply in the email thread for RB #2513.

Consider this a ship it if it really turns out that using conditional 
compilation is the way to go.

- Andreas Sandberg


On Nov. 19, 2014, 6:44 a.m., Gabe Black wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2514/
 ---
 
 (Updated Nov. 19, 2014, 6:44 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10548:07c3cbac4cdd
 ---
 scons: Make the USE_KVM variable available in C++.
 
 We need it to determine whether we should expect KVM related parameters
 exist in the cirrus graphics device.
 
 
 Diffs
 -
 
   SConstruct 288eb5ee4b0026d0cc1f02ec31748e0eaac06bc3 
 
 Diff: http://reviews.gem5.org/r/2514/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Gabe Black
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2510/#review5489
---



src/cpu/kvm/vm.hh
http://reviews.gem5.org/r/2510/#comment4940

The returned slot ID should be typedef:ed, or preferably a struct since 
that would make type checking more reliable.



src/cpu/kvm/vm.hh
http://reviews.gem5.org/r/2510/#comment4944

Would it make sense to rename this to mapMemSlot? In my opinion, that'd be 
more descriptive.



src/cpu/kvm/vm.hh
http://reviews.gem5.org/r/2510/#comment4942

Indentation is inconsistent with the rest of the file.



src/cpu/kvm/vm.hh
http://reviews.gem5.org/r/2510/#comment4941

What's this group comment doing here and where is it terminated?



src/cpu/kvm/vm.hh
http://reviews.gem5.org/r/2510/#comment4945

How about renaming this to unmapMemSlot?



src/cpu/kvm/vm.cc
http://reviews.gem5.org/r/2510/#comment4943

Inconsistent indentation.


Overall, I'd prefer this to be an internal API and have some way of notifying 
the VM through the System instead of allowing objects to poke around directly. 
See my reply in the email thread for RB #2513.

- Andreas Sandberg


On Nov. 18, 2014, 1:29 a.m., Gabe Black wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2510/
 ---
 
 (Updated Nov. 18, 2014, 1:29 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10546:b4c9aa186307
 ---
 Let other objects set up memory like regions in a KVM VM.
 
 
 Diffs
 -
 
   src/cpu/kvm/vm.hh f66948658a36b6874e84ee5da37e70d351287cb4 
   src/cpu/kvm/vm.cc f66948658a36b6874e84ee5da37e70d351287cb4 
 
 Diff: http://reviews.gem5.org/r/2510/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Gabe Black
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2515: x86: pc: Put a stub IO device at port 0xed which the kernel can use for delays.

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2515/#review5490
---



src/dev/x86/Pc.py
http://reviews.gem5.org/r/2515/#comment4946

I might be confused by the weird semantics of the gem5 configuration 
scripts, but isn't this killing the fake device for port 0x80?


- Andreas Sandberg


On Nov. 19, 2014, 9:26 a.m., Gabe Black wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2515/
 ---
 
 (Updated Nov. 19, 2014, 9:26 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10550:9fcdf186d7ba
 ---
 x86: pc: Put a stub IO device at port 0xed which the kernel can use for 
 delays.
 
 There was already a stub device at 0x80, the port traditionally used for an IO
 delay. 0x80 is also the port used for POST codes sent by firmware, and that
 may have prompted adding this port as a second option.
 
 
 Diffs
 -
 
   src/dev/x86/Pc.py 288eb5ee4b0026d0cc1f02ec31748e0eaac06bc3 
 
 Diff: http://reviews.gem5.org/r/2515/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Gabe Black
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: cpu: Probe points for basic PMU stats

[Andreas Sandberg via gem5-dev]

changeset 2a0fe8bca031 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=2a0fe8bca031
description:
cpu: Probe points for basic PMU stats

This changeset adds probe points that can be used to implement PMU
counters for CPU stats. The following probes are supported:

  * BaseCPU::ppCycles / Cycles
  * BaseCPU::ppRetiredInsts / RetiredInsts
  * BaseCPU::ppRetiredLoads / RetiredLoads
  * BaseCPU::ppRetiredStores / RetiredStores
  * BaseCPU::ppRetiredBranches RetiredBranches

diffstat:

 src/cpu/base.cc   |  36 +
 src/cpu/base.hh   |  51 +++
 src/cpu/minor/execute.cc  |   2 +
 src/cpu/minor/pipeline.hh |   5 
 src/cpu/o3/cpu.cc |  14 ++--
 src/cpu/simple/atomic.cc  |   7 +-
 src/cpu/simple/base.cc|   3 ++
 src/cpu/simple/timing.cc  |  29 --
 src/cpu/simple/timing.hh  |   4 ++-
 src/sim/ticked_object.hh  |  15 +
 10 files changed, 150 insertions(+), 16 deletions(-)

diffs (truncated from 380 to 300 lines):

diff -r 25c5da51bbe0 -r 2a0fe8bca031 src/cpu/base.cc
--- a/src/cpu/base.cc   Thu Oct 16 05:49:41 2014 -0400
+++ b/src/cpu/base.cc   Thu Oct 16 05:49:41 2014 -0400
@@ -283,6 +283,42 @@
 }
 }
 
+ProbePoints::PMUUPtr
+BaseCPU::pmuProbePoint(const char *name)
+{
+ProbePoints::PMUUPtr ptr;
+ptr.reset(new ProbePoints::PMU(getProbeManager(), name));
+
+return ptr;
+}
+
+void
+BaseCPU::regProbePoints()
+{
+ppCycles = pmuProbePoint(Cycles);
+
+ppRetiredInsts = pmuProbePoint(RetiredInsts);
+ppRetiredLoads = pmuProbePoint(RetiredLoads);
+ppRetiredStores = pmuProbePoint(RetiredStores);
+ppRetiredBranches = pmuProbePoint(RetiredBranches);
+}
+
+void
+BaseCPU::probeInstCommit(const StaticInstPtr inst)
+{
+if (!inst-isMicroop() || inst-isLastMicroop())
+ppRetiredInsts-notify(1);
+
+
+if (inst-isLoad())
+ppRetiredLoads-notify(1);
+
+if (inst-isStore())
+ppRetiredLoads-notify(1);
+
+if (inst-isControl())
+ppRetiredBranches-notify(1);
+}
 
 void
 BaseCPU::regStats()
diff -r 25c5da51bbe0 -r 2a0fe8bca031 src/cpu/base.hh
--- a/src/cpu/base.hh   Thu Oct 16 05:49:41 2014 -0400
+++ b/src/cpu/base.hh   Thu Oct 16 05:49:41 2014 -0400
@@ -62,6 +62,7 @@
 #include sim/eventq.hh
 #include sim/full_system.hh
 #include sim/insttracer.hh
+#include sim/probe/pmu.hh
 #include sim/system.hh
 
 struct BaseCPUParams;
@@ -280,6 +281,8 @@
 virtual void startup();
 virtual void regStats();
 
+void regProbePoints() M5_ATTR_OVERRIDE;
+
 void registerThreadContexts();
 
 /**
@@ -437,6 +440,54 @@
  */
 void scheduleLoadStop(ThreadID tid, Counter loads, const char *cause);
 
+  public:
+/**
+ * @{
+ * @name PMU Probe points.
+ */
+
+/**
+ * Helper method to trigger PMU probes for a committed
+ * instruction.
+ *
+ * @param inst Instruction that just committed
+ */
+virtual void probeInstCommit(const StaticInstPtr inst);
+
+/**
+ * Helper method to instantiate probe points belonging to this
+ * object.
+ *
+ * @param name Name of the probe point.
+ * @return A unique_ptr to the new probe point.
+ */
+ProbePoints::PMUUPtr pmuProbePoint(const char *name);
+
+/** CPU cycle counter */
+ProbePoints::PMUUPtr ppCycles;
+
+/**
+ * Instruction commit probe point.
+ *
+ * This probe point is triggered whenever one or more instructions
+ * are committed. It is normally triggered once for every
+ * instruction. However, CPU models committing bundles of
+ * instructions may call notify once for the entire bundle.
+ */
+ProbePoints::PMUUPtr ppRetiredInsts;
+
+/** Retired load instructions */
+ProbePoints::PMUUPtr ppRetiredLoads;
+/** Retired store instructions */
+ProbePoints::PMUUPtr ppRetiredStores;
+
+/** Retired branches (any type) */
+ProbePoints::PMUUPtr ppRetiredBranches;
+
+/** @} */
+
+
+
 // Function tracing
   private:
 bool functionTracingEnabled;
diff -r 25c5da51bbe0 -r 2a0fe8bca031 src/cpu/minor/execute.cc
--- a/src/cpu/minor/execute.cc  Thu Oct 16 05:49:41 2014 -0400
+++ b/src/cpu/minor/execute.cc  Thu Oct 16 05:49:41 2014 -0400
@@ -853,6 +853,8 @@
 /* Set the CP SeqNum to the numOps commit number */
 if (inst-traceData)
 inst-traceData-setCPSeq(thread-numOp);
+
+cpu.probeInstCommit(inst-staticInst);
 }
 
 bool
diff -r 25c5da51bbe0 -r 2a0fe8bca031 src/cpu/minor/pipeline.hh
--- a/src/cpu/minor/pipeline.hh Thu Oct 16 05:49:41 2014 -0400
+++ b/src/cpu/minor/pipeline.hh Thu Oct 16 05:49:41 2014 -0400
@@ -126,6 +126,11 @@
  *  stages and pipeline advance) */
 void evaluate();
 
+void countCycles(Cycles delta) M5_ATTR_OVERRIDE
+{
+cpu.ppCycles-notify(delta);
+}
+
 void minorTrace() const;
 
 /** Functions 

[gem5-dev] changeset in gem5: sim: Add typedefs for PMU probe points

[Andreas Sandberg via gem5-dev]

changeset 20443473c68a in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=20443473c68a
description:
sim: Add typedefs for PMU probe points

In order to show make PMU probe points usable across different PMU
implementations, we want a common probe interface. This patch the
namespace ProbePoins that contains typedefs for probe points that are
shared between multiple SimObjects. It also adds typedefs for the PMU
probe interface.

diffstat:

 src/sim/probe/pmu.hh   |  61 ++
 src/sim/probe/probe.hh |  15 
 2 files changed, 76 insertions(+), 0 deletions(-)

diffs (90 lines):

diff -r 810f5a48a920 -r 20443473c68a src/sim/probe/pmu.hh
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/src/sim/probe/pmu.hh  Thu Oct 16 05:49:38 2014 -0400
@@ -0,0 +1,61 @@
+/*
+ * Copyright (c) 2014 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.
+ *
+ * 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.
+ *
+ * Authors: Andreas Sandberg
+ */
+#ifndef __SIM_PROBE_PMU_HH__
+#define __SIM_PROBE_PMU_HH__
+
+#include memory
+
+#include sim/probe/probe.hh
+
+namespace ProbePoints {
+
+/**
+ * PMU probe point
+ *
+ * This probe point provides a unified interface for PMU
+ * instrumentation of SimObjects. SimObjects that need PMU
+ * instrumentation should implement probes of this type call the
+ * notify method with the event count increment as its only parameter.
+ */
+typedef ProbePointArguint64_t PMU;
+typedef std::unique_ptrPMU PMUUPtr;
+
+}
+
+#endif
diff -r 810f5a48a920 -r 20443473c68a src/sim/probe/probe.hh
--- a/src/sim/probe/probe.hhThu Oct 16 05:49:37 2014 -0400
+++ b/src/sim/probe/probe.hhThu Oct 16 05:49:38 2014 -0400
@@ -73,6 +73,21 @@
 class ProbeListener;
 
 /**
+ * Name space containing shared probe point declarations.
+ *
+ * Probe types that are shared between multiple types of SimObjects
+ * should live in this name space. This makes it possible to use a
+ * common instrumentation interface for devices such as PMUs that have
+ * different implementations in different ISAs.
+ */
+namespace ProbePoints {
+/* Note: This is only here for documentation purposes, new probe
+ * points should normally be declared in their own header files. See
+ * for example pmu.hh.
+ */
+}
+
+/**
  * This class is a minimal wrapper around SimObject. It is used to declare
  * a python derived object that can be added as a ProbeListener to any other
  * SimObject.
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: cpu: Add branch predictor PMU probe points

[Andreas Sandberg via gem5-dev]

changeset e975e8afba8b in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=e975e8afba8b
description:
cpu: Add branch predictor PMU probe points

This changeset adds probe points that can be used to implement PMU
counters for branch predictor stats. The following probes are
supported:

 * BPRedUnit::ppBranches / Branches
 * BPRedUnit::ppMisses / Misses

diffstat:

 src/cpu/pred/bpred_unit.hh  |  31 +++
 src/cpu/pred/bpred_unit_impl.hh |  20 
 2 files changed, 51 insertions(+), 0 deletions(-)

diffs (106 lines):

diff -r afeb5cdb3907 -r e975e8afba8b src/cpu/pred/bpred_unit.hh
--- a/src/cpu/pred/bpred_unit.hhThu Oct 16 05:49:39 2014 -0400
+++ b/src/cpu/pred/bpred_unit.hhThu Oct 16 05:49:40 2014 -0400
@@ -56,6 +56,7 @@
 #include cpu/inst_seq.hh
 #include cpu/static_inst.hh
 #include params/BranchPredictor.hh
+#include sim/probe/pmu.hh
 #include sim/sim_object.hh
 
 /**
@@ -76,6 +77,8 @@
  */
 void regStats();
 
+void regProbePoints() M5_ATTR_OVERRIDE;
+
 /** Perform sanity checks after a drain. */
 void drainSanityCheck() const;
 
@@ -290,6 +293,34 @@
 Stats::Scalar usedRAS;
 /** Stat for number of times the RAS is incorrect. */
 Stats::Scalar RASIncorrect;
+
+  protected:
+/**
+ * @{
+ * @name PMU Probe points.
+ */
+
+/**
+ * Helper method to instantiate probe points belonging to this
+ * object.
+ *
+ * @param name Name of the probe point.
+ * @return A unique_ptr to the new probe point.
+ */
+ProbePoints::PMUUPtr pmuProbePoint(const char *name);
+
+
+/**
+ * Branches seen by the branch predictor
+ *
+ * @note This counter includes speculative branches.
+ */
+ProbePoints::PMUUPtr ppBranches;
+
+/** Miss-predicted branches */
+ProbePoints::PMUUPtr ppMisses;
+
+/** @} */
 };
 
 #endif // __CPU_PRED_BPRED_UNIT_HH__
diff -r afeb5cdb3907 -r e975e8afba8b src/cpu/pred/bpred_unit_impl.hh
--- a/src/cpu/pred/bpred_unit_impl.hh   Thu Oct 16 05:49:39 2014 -0400
+++ b/src/cpu/pred/bpred_unit_impl.hh   Thu Oct 16 05:49:40 2014 -0400
@@ -119,6 +119,22 @@
 ;
 }
 
+ProbePoints::PMUUPtr
+BPredUnit::pmuProbePoint(const char *name)
+{
+ProbePoints::PMUUPtr ptr;
+ptr.reset(new ProbePoints::PMU(getProbeManager(), name));
+
+return ptr;
+}
+
+void
+BPredUnit::regProbePoints()
+{
+ppBranches = pmuProbePoint(Branches);
+ppMisses = pmuProbePoint(Misses);
+}
+
 void
 BPredUnit::drainSanityCheck() const
 {
@@ -141,6 +157,7 @@
 TheISA::PCState target = pc;
 
 ++lookups;
+ppBranches-notify(1);
 
 void *bp_history = NULL;
 
@@ -259,6 +276,8 @@
 TheISA::PCState target;
 
 ++lookups;
+ppBranches-notify(1);
+
 DPRINTF(Branch, [tid:%i] [sn:%i] %s ... PC %s doing branch 
 prediction\n, tid, seqNum,
 inst-disassemble(instPC.instAddr()), instPC);
@@ -438,6 +457,7 @@
 History pred_hist = predHist[tid];
 
 ++condIncorrect;
+ppMisses-notify(1);
 
 DPRINTF(Branch, [tid:%i]: Squashing from sequence number %i, 
 setting target to %s.\n, tid, squashed_sn, corrTarget);
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: arm: Add helper methods to setup architected ...

[Andreas Sandberg via gem5-dev]

changeset a42b8d98fddc in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=a42b8d98fddc
description:
arm: Add helper methods to setup architected PMU events

diffstat:

 src/arch/arm/ArmPMU.py |  53 ++
 1 files changed, 53 insertions(+), 0 deletions(-)

diffs (63 lines):

diff -r 2a0fe8bca031 -r a42b8d98fddc src/arch/arm/ArmPMU.py
--- a/src/arch/arm/ArmPMU.pyThu Oct 16 05:49:41 2014 -0400
+++ b/src/arch/arm/ArmPMU.pyThu Oct 16 05:49:42 2014 -0400
@@ -78,6 +78,59 @@
 for name in args:
 self._deferred_event_types.append((event_id, obj, name))
 
+def addArchEvents(self,
+  cpu=None,
+  itb=None, dtb=None,
+  icache=None, dcache=None,
+  l2cache=None):
+Add architected events to the PMU.
+
+This method can be called multiple times with only a subset of
+the keyword arguments set. This enables event registration in
+configuration scripts to happen closer to the instantiation of
+the instrumented objects (e.g., the memory system) instead of
+a central point.
+
+CPU events should also be registered once per CPU that is
+sharing the PMU (e.g., when switching between CPU models).
+
+
+bpred = cpu.branchPred if cpu and not isNullPointer(cpu.branchPred) \
+else None
+
+# 0x01: L1I_CACHE_REFILL
+self.addEventProbe(0x02, itb, Refills)
+# 0x03: L2D_CACHE_REFILL
+# 0x04: L1D_CACHE
+self.addEventProbe(0x05, dtb, Refills)
+self.addEventProbe(0x06, cpu, RetiredLoads)
+self.addEventProbe(0x07, cpu, RetiredStores)
+self.addEventProbe(0x08, cpu, RetiredInsts)
+# 0x09: EXC_TAKEN
+# 0x0A: EXC_RETURN
+# 0x0B: CID_WRITE_RETIRED
+self.addEventProbe(0x0C, cpu, RetiredBranches)
+# 0x0D: BR_IMMED_RETIRED
+# 0x0E: BR_RETURN_RETIRED
+# 0x0F: UNALIGEND_LDST_RETIRED
+self.addEventProbe(0x10, bpred, Misses)
+self.addEventProbe(0x11, cpu, Cycles)
+self.addEventProbe(0x12, bpred, Branches)
+self.addEventProbe(0x13, cpu, RetiredLoads, RetiredStores)
+# 0x14: L1I_CACHE
+# 0x15: L1D_CACHE_WB
+# 0x16: L2D_CACHE
+# 0x17: L2D_CACHE_REFILL
+# 0x18: L2D_CACHE_WB
+# 0x19: BUS_ACCESS
+# 0x1A: MEMORY_ERROR
+# 0x1B: INST_SPEC
+# 0x1C: TTBR_WRITE_RETIRED
+# 0x1D: BUS_CYCLES
+# 0x1E: CHAIN
+# 0x1F: L1D_CACHE_ALLOCATE
+# 0x20: L2D_CACHE_ALLOCATE
+
 platform = Param.Platform(Parent.any, Platform this device is part of.)
 eventCounters = Param.Int(31, Number of supported PMU counters)
 pmuInterrupt = Param.Int(68, PMU GIC interrupt number)
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: sim: Add support for serializing BitUnionXX

[Andreas Sandberg via gem5-dev]

changeset 810f5a48a920 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=810f5a48a920
description:
sim: Add support for serializing BitUnionXX

BitUnion instances can normally not be used with the SERIALIZE_SCALAR
and UNSERIALIZE_SCALAR macros due to the way they are converted
between their storage type and their actual type. This changeset adds
a set of parm(In|Out) functions specifically for gem5 bit unions to
work around the issue.

diffstat:

 src/sim/serialize.hh |  24 
 1 files changed, 24 insertions(+), 0 deletions(-)

diffs (49 lines):

diff -r 64809024b924 -r 810f5a48a920 src/sim/serialize.hh
--- a/src/sim/serialize.hh  Thu Oct 16 05:49:37 2014 -0400
+++ b/src/sim/serialize.hh  Thu Oct 16 05:49:37 2014 -0400
@@ -43,6 +43,7 @@
 #include map
 #include vector
 
+#include base/bitunion.hh
 #include base/types.hh
 
 class IniFile;
@@ -63,14 +64,37 @@
 template class T
 void paramOut(std::ostream os, const std::string name, const T param);
 
+template typename DataType, typename BitUnion
+void paramOut(std::ostream os, const std::string name,
+  const BitfieldBackend::BitUnionOperatorsDataType, BitUnion p)
+{
+paramOut(os, name, p.__data);
+}
+
 template class T
 void paramIn(Checkpoint *cp, const std::string section,
  const std::string name, T param);
 
+template typename DataType, typename BitUnion
+void paramIn(Checkpoint *cp, const std::string section,
+ const std::string name,
+ BitfieldBackend::BitUnionOperatorsDataType, BitUnion p)
+{
+paramIn(cp, section, name, p.__data);
+}
+
 template class T
 bool optParamIn(Checkpoint *cp, const std::string section,
  const std::string name, T param);
 
+template typename DataType, typename BitUnion
+bool optParamIn(Checkpoint *cp, const std::string section,
+const std::string name,
+BitfieldBackend::BitUnionOperatorsDataType, BitUnion p)
+{
+return optParamIn(cp, section, name, p.__data);
+}
+
 template class T
 void arrayParamOut(std::ostream os, const std::string name,
const T *param, unsigned size);
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: arm: Add a model of an ARM PMUv3

[Andreas Sandberg via gem5-dev]

changeset afeb5cdb3907 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=afeb5cdb3907
description:
arm: Add a model of an ARM PMUv3

This class implements a subset of the ARM PMU v3 specification as
described in the ARMv8 reference manual. It supports most of the
features of the PMU, however the following features are known to be
missing:

 * Event filtering (e.g., from different privilege levels).
 * Access controls (the PMU currently ignores the execution level).
 * The chain counter (event no. 0x1E) is unimplemented.

The PMU itself does not implement any events, it merely provides an
interface for the configuration scripts to hook up probes that drive
events. Configuration scripts should call addEventProbe() to configure
custom events or high-level methods to configure architected
events. The Python implementation of addEventProbe() automatically
delays event type registration until after instantiation.

In order to support CPU switching and some combined counters (e.g.,
memory references synthesized from loads and stores), the PMU allows
multiple probes per event type. When creating a system that switches
between CPU models that share the same PMU, PMU events for all of the
CPU models can be registered with the PMU.

Kudos to Matt Horsnell for the initial gem5 implementation of the PMU.

diffstat:

 src/arch/arm/ArmISA.py |4 +
 src/arch/arm/ArmPMU.py |   83 ++
 src/arch/arm/SConscript|4 +
 src/arch/arm/isa.cc|   67 +++--
 src/arch/arm/isa.hh|8 +
 src/arch/arm/isa_device.cc |   62 
 src/arch/arm/isa_device.hh |   99 +++
 src/arch/arm/pmu.cc|  559 +
 src/arch/arm/pmu.hh|  495 +++
 9 files changed, 1353 insertions(+), 28 deletions(-)

diffs (truncated from 1543 to 300 lines):

diff -r 20443473c68a -r afeb5cdb3907 src/arch/arm/ArmISA.py
--- a/src/arch/arm/ArmISA.pyThu Oct 16 05:49:38 2014 -0400
+++ b/src/arch/arm/ArmISA.pyThu Oct 16 05:49:39 2014 -0400
@@ -40,6 +40,8 @@
 from m5.proxy import *
 from m5.SimObject import SimObject
 
+from ArmPMU import ArmPMU
+
 class ArmISA(SimObject):
 type = 'ArmISA'
 cxx_class = 'ArmISA::ISA'
@@ -47,6 +49,8 @@
 
 system = Param.System(Parent.any, System this ISA object belongs to)
 
+pmu = Param.ArmPMU(NULL, Performance Monitoring Unit)
+
 midr = Param.UInt32(0x410fc0f0, MIDR value)
 
 # See section B4.1.93 - B4.1.94 of the ARM ARM
diff -r 20443473c68a -r afeb5cdb3907 src/arch/arm/ArmPMU.py
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/src/arch/arm/ArmPMU.pyThu Oct 16 05:49:39 2014 -0400
@@ -0,0 +1,83 @@
+# -*- mode:python -*-
+# Copyright (c) 2009-2014 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.
+#
+# 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.
+#
+# 

[gem5-dev] changeset in gem5: arm: Add TLB PMU probes

[Andreas Sandberg via gem5-dev]

changeset 25c5da51bbe0 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=25c5da51bbe0
description:
arm: Add TLB PMU probes

This changeset adds probe points that can be used to implement PMU
counters for TLB stats. The following probes are supported:

* ArmISA::TLB::ppRefills / TLB Refills (TLB insertions)

diffstat:

 src/arch/arm/tlb.cc |  7 +++
 src/arch/arm/tlb.hh |  6 ++
 2 files changed, 13 insertions(+), 0 deletions(-)

diffs (54 lines):

diff -r e975e8afba8b -r 25c5da51bbe0 src/arch/arm/tlb.cc
--- a/src/arch/arm/tlb.cc   Thu Oct 16 05:49:40 2014 -0400
+++ b/src/arch/arm/tlb.cc   Thu Oct 16 05:49:41 2014 -0400
@@ -197,6 +197,7 @@
 table[0] = entry;
 
 inserts++;
+ppRefills-notify(1);
 }
 
 void
@@ -531,6 +532,12 @@
 accesses = readAccesses + writeAccesses + instAccesses;
 }
 
+void
+TLB::regProbePoints()
+{
+ppRefills.reset(new ProbePoints::PMU(getProbeManager(), Refills));
+}
+
 Fault
 TLB::translateSe(RequestPtr req, ThreadContext *tc, Mode mode,
  Translation *translation, bool delay, bool timing)
diff -r e975e8afba8b -r 25c5da51bbe0 src/arch/arm/tlb.hh
--- a/src/arch/arm/tlb.hh   Thu Oct 16 05:49:40 2014 -0400
+++ b/src/arch/arm/tlb.hh   Thu Oct 16 05:49:41 2014 -0400
@@ -53,6 +53,7 @@
 #include mem/request.hh
 #include params/ArmTLB.hh
 #include sim/fault_fwd.hh
+#include sim/probe/pmu.hh
 #include sim/tlb.hh
 
 class ThreadContext;
@@ -131,6 +132,9 @@
 Stats::Formula misses;
 Stats::Formula accesses;
 
+/** PMU probe for TLB refills */
+ProbePoints::PMUUPtr ppRefills;
+
 int rangeMRU; //On lookup, only move entries ahead when outside rangeMRU
 
 bool bootUncacheability;
@@ -291,6 +295,8 @@
 
 void regStats();
 
+void regProbePoints() M5_ATTR_OVERRIDE;
+
 /**
  * Get the table walker master port. This is used for migrating
  * port connections during a CPU takeOverFrom() call. For
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: ext: Update fputils to rev 6a47fd8358

[Andreas Sandberg via gem5-dev]

changeset 5d4ebc92d32e in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=5d4ebc92d32e
description:
ext: Update fputils to rev 6a47fd8358

This patch updates fputils to the latest revision (6a47fd8358) from
the upstream repository (github.com/andysan/fputils). Most notably,
this includes changes that export a limited set of 64-bit float
manipulation and avoids a warning about unused 64-bit floats in clang.

diffstat:

 ext/fputils/Makefile.am   |   8 ++-
 ext/fputils/SConscript|   1 +
 ext/fputils/configure.ac  |  36 ++
 ext/fputils/configure.in  |  36 --
 ext/fputils/fp64.c|  45 ++
 ext/fputils/fp80.c|  56 -
 ext/fputils/fpbits.h  |  17 ++
 ext/fputils/include/fputils/fp64.h|  73 +
 ext/fputils/include/fputils/fp80.h|  60 
 ext/fputils/include/fputils/fptypes.h |  86 +++
 10 files changed, 327 insertions(+), 91 deletions(-)

diffs (truncated from 567 to 300 lines):

diff -r 60b4471a8181 -r 5d4ebc92d32e ext/fputils/Makefile.am
--- a/ext/fputils/Makefile.am   Thu Oct 16 05:49:57 2014 -0400
+++ b/ext/fputils/Makefile.am   Thu Oct 16 05:49:58 2014 -0400
@@ -6,11 +6,17 @@
 
 lib_LTLIBRARIES = libfputils.la
 
-include_HEADERS = include/fputils/fp80.h
+include_HEADERS = \
+   include/fputils/fp80.h \
+   include/fputils/fp64.h \
+   include/fputils/fptypes.h
 
 libfputils_la_SOURCES = \
include/fputils/fp80.h \
+   include/fputils/fp64.h \
+   include/fputils/fptypes.h \
fpbits.h \
+   fp64.c \
fp80.c
 
 
diff -r 60b4471a8181 -r 5d4ebc92d32e ext/fputils/SConscript
--- a/ext/fputils/SConscriptThu Oct 16 05:49:57 2014 -0400
+++ b/ext/fputils/SConscriptThu Oct 16 05:49:58 2014 -0400
@@ -40,6 +40,7 @@
 fpenv.Append(CCFLAGS=['-std=c99'])
 
 fpenv.Library('fputils', [
+fpenv.SharedObject('fp64.c'),
 fpenv.SharedObject('fp80.c'),
 ])
 
diff -r 60b4471a8181 -r 5d4ebc92d32e ext/fputils/configure.ac
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/ext/fputils/configure.ac  Thu Oct 16 05:49:58 2014 -0400
@@ -0,0 +1,36 @@
+AC_INIT(libfputils, 1.0, andr...@sandberg.pp.se)
+
+AC_CONFIG_MACRO_DIR([m4])
+
+AM_INIT_AUTOMAKE([foreign -Wall])
+
+DX_PDF_FEATURE(OFF)
+DX_PS_FEATURE(OFF)
+DX_MAN_FEATURE(OFF)
+DX_INIT_DOXYGEN([libfputils])
+
+AC_REQUIRE_AUX_FILE([tap-driver.sh])
+
+AC_PROG_CC
+AC_PROG_CC_C99
+AC_PROG_LIBTOOL
+AC_PROG_AWK
+
+if test x$ac_cv_prog_cc_c99 = xno; then
+  AC_MSG_ERROR([Could not enable C99 support in compiler.])
+fi
+
+AM_CFLAGS=-Wall -Werror
+AM_CPPFLAGS=-I\$(abs_top_srcdir)/include
+
+AC_SUBST(AM_CFLAGS)
+AC_SUBST(AM_CPPFLAGS)
+
+AC_CONFIG_HEADERS([config.h])
+
+AC_CONFIG_FILES([ \
+Doxyfile \
+   Makefile \
+tests/Makefile \
+])
+AC_OUTPUT
diff -r 60b4471a8181 -r 5d4ebc92d32e ext/fputils/configure.in
--- a/ext/fputils/configure.in  Thu Oct 16 05:49:57 2014 -0400
+++ /dev/null   Thu Jan 01 00:00:00 1970 +
@@ -1,36 +0,0 @@
-AC_INIT(libfputils, 1.0, andr...@sandberg.pp.se)
-
-AC_CONFIG_MACRO_DIR([m4])
-
-AM_INIT_AUTOMAKE([foreign -Wall -Werror])
-
-DX_PDF_FEATURE(OFF)
-DX_PS_FEATURE(OFF)
-DX_MAN_FEATURE(OFF)
-DX_INIT_DOXYGEN([libfputils])
-
-AC_REQUIRE_AUX_FILE([tap-driver.sh])
-
-AC_PROG_CC
-AC_PROG_CC_C99
-AC_PROG_LIBTOOL
-AC_PROG_AWK
-
-if test x$ac_cv_prog_cc_c99 = xno; then
-  AC_MSG_ERROR([Could not enable C99 support in compiler.])
-fi
-
-AM_CFLAGS=-Wall -Werror
-AM_CPPFLAGS=-I\$(abs_top_srcdir)/include
-
-AC_SUBST(AM_CFLAGS)
-AC_SUBST(AM_CPPFLAGS)
-
-AC_CONFIG_HEADERS([config.h])
-
-AC_CONFIG_FILES([ \
-Doxyfile \
-   Makefile \
-tests/Makefile \
-])
-AC_OUTPUT
diff -r 60b4471a8181 -r 5d4ebc92d32e ext/fputils/fp64.c
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/ext/fputils/fp64.cThu Oct 16 05:49:58 2014 -0400
@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2014, Andreas Sandberg
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. 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.
+ *
+ * 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 

[gem5-dev] changeset in gem5: dev: Refactor terminal-UART interface to ma...

[Andreas Sandberg via gem5-dev]

changeset 8a7724f13288 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=8a7724f13288
description:
dev: Refactor terminal-UART interface to make it more generic

The terminal currently assumes that the transport to the guest always
inherits from the Uart class. This assumption breaks when
implementing, for example, a VirtIO consoles. This patch removes this
assumption by adding pointer to the from the terminal to the uart and
replacing it with a more general callback interface. The Uart, or any
other class using the terminal, class implements an instance of the
callbacks class and registers it with the terminal.

diffstat:

 src/dev/terminal.cc |  18 +++---
 src/dev/terminal.hh |  17 +++--
 src/dev/uart.cc |   8 +---
 src/dev/uart.hh |   6 --
 4 files changed, 39 insertions(+), 10 deletions(-)

diffs (131 lines):

diff -r c81407818741 -r 8a7724f13288 src/dev/terminal.cc
--- a/src/dev/terminal.cc   Sat Sep 20 17:17:49 2014 -0400
+++ b/src/dev/terminal.cc   Sat Sep 20 17:17:50 2014 -0400
@@ -99,8 +99,8 @@
  * Terminal code
  */
 Terminal::Terminal(const Params *p)
-: SimObject(p), listenEvent(NULL), dataEvent(NULL), number(p-number),
-  data_fd(-1), txbuf(16384), rxbuf(16384), outfile(NULL)
+: SimObject(p), termDataAvail(NULL), listenEvent(NULL), dataEvent(NULL),
+  number(p-number), data_fd(-1), txbuf(16384), rxbuf(16384), outfile(NULL)
 #if TRACING_ON == 1
   , linebuf(16384)
 #endif
@@ -129,6 +129,17 @@
 delete dataEvent;
 }
 
+void
+Terminal::regDataAvailCallback(Callback *c)
+{
+// This can happen if the user has connected multiple UARTs to the
+// same terminal. In that case, each of them tries to register
+// callbacks.
+if (termDataAvail)
+fatal(Terminal already has already been associated with a UART.\n);
+termDataAvail = c;
+}
+
 ///
 // socket creation and terminal attach
 //
@@ -215,7 +226,8 @@
 if (len) {
 rxbuf.write((char *)buf, len);
 // Inform the UART there is data available
-uart-dataAvailable();
+assert(termDataAvail);
+termDataAvail-process();
 }
 }
 
diff -r c81407818741 -r 8a7724f13288 src/dev/terminal.hh
--- a/src/dev/terminal.hh   Sat Sep 20 17:17:49 2014 -0400
+++ b/src/dev/terminal.hh   Sat Sep 20 17:17:50 2014 -0400
@@ -38,6 +38,7 @@
 
 #include iostream
 
+#include base/callback.hh
 #include base/circlebuf.hh
 #include base/pollevent.hh
 #include base/socket.hh
@@ -46,12 +47,24 @@
 #include sim/sim_object.hh
 
 class TerminalListener;
-class Uart;
 
 class Terminal : public SimObject
 {
   public:
-Uart *uart;
+/**
+ * Register a data available callback into the transport layer.
+ *
+ * The terminal needs to call the underlying transport layer to
+ * inform it of available data. The transport layer uses this
+ * method to register a callback that informs it of pending data.
+ *
+ * @param c Callback instance from transport layer.
+ */
+void regDataAvailCallback(Callback *c);
+
+  protected:
+/** Currently registered transport layer callbacks */
+Callback *termDataAvail;
 
   protected:
 class ListenEvent : public PollEvent
diff -r c81407818741 -r 8a7724f13288 src/dev/uart.cc
--- a/src/dev/uart.cc   Sat Sep 20 17:17:49 2014 -0400
+++ b/src/dev/uart.cc   Sat Sep 20 17:17:50 2014 -0400
@@ -39,10 +39,12 @@
 using namespace std;
 
 Uart::Uart(const Params *p, Addr pio_size)
-: BasicPioDevice(p, pio_size), platform(p-platform), term(p-terminal)
+: BasicPioDevice(p, pio_size),
+  platform(p-platform), term(p-terminal),
+  callbackDataAvail(this)
 {
 status = 0;
 
-// set back pointers
-term-uart = this;
+// setup terminal callbacks
+term-regDataAvailCallback(callbackDataAvail);
 }
diff -r c81407818741 -r 8a7724f13288 src/dev/uart.hh
--- a/src/dev/uart.hh   Sat Sep 20 17:17:49 2014 -0400
+++ b/src/dev/uart.hh   Sat Sep 20 17:17:50 2014 -0400
@@ -36,9 +36,9 @@
 #define __UART_HH__
 
 #include dev/io_device.hh
+#include dev/terminal.hh
 #include params/Uart.hh
 
-class Terminal;
 class Platform;
 
 const int RX_INT = 0x1;
@@ -46,7 +46,6 @@
 
 class Uart : public BasicPioDevice
 {
-
   protected:
 int status;
 Platform *platform;
@@ -72,6 +71,9 @@
  * @return interrupt status
  */
 bool intStatus() { return status ? true : false; }
+
+  protected:
+MakeCallbackUart, Uart::dataAvailable callbackDataAvail;
 };
 
 #endif // __UART_HH__
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: dev, pci: Implement basic VirtIO support

[Andreas Sandberg via gem5-dev]

changeset a26a20060ba3 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=a26a20060ba3
description:
dev, pci: Implement basic VirtIO support

This patch adds support for VirtIO over the PCI bus. It does so by
providing the following new SimObjects:

 * VirtIODeviceBase - Abstract base class for VirtIO devices.
 * PciVirtIO - VirtIO PCI transport interface.

A VirtIO device is hooked up to the guest system by adding a PciVirtIO
device to the PCI bus and connecting it to a VirtIO device using the
vio parameter.

New VirtIO devices should inherit from VirtIODevice base and
implementing one or more VirtQueues. The VirtQueues are usually
device-specific and all derive from the VirtQueue class. Queues must
be registered with the base class from the constructor since the
device assumes that the number of queues stay constant.

diffstat:

 src/dev/virtio/SConscript|   51 ++
 src/dev/virtio/VirtIO.py |   71 +++
 src/dev/virtio/base.cc   |  481 +++
 src/dev/virtio/base.hh   |  881 +++
 src/dev/virtio/pci.cc|  222 ++
 src/dev/virtio/pci.hh|   89 
 src/dev/virtio/virtio_ring.h |  163 +++
 7 files changed, 1958 insertions(+), 0 deletions(-)

diffs (truncated from 1986 to 300 lines):

diff -r 8a7724f13288 -r a26a20060ba3 src/dev/virtio/SConscript
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/src/dev/virtio/SConscript Sat Sep 20 17:17:51 2014 -0400
@@ -0,0 +1,51 @@
+# -*- mode:python -*-
+
+# Copyright (c) 2014 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.
+#
+# 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.
+#
+# Authors: Andreas Sandberg
+
+Import('*')
+
+if env['TARGET_ISA'] == 'null':
+Return()
+
+SimObject('VirtIO.py')
+
+Source('base.cc')
+Source('pci.cc')
+
+DebugFlag('VIO', 'VirtIO base functionality')
+DebugFlag('VIOPci', 'VirtIO PCI transport')
diff -r 8a7724f13288 -r a26a20060ba3 src/dev/virtio/VirtIO.py
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/src/dev/virtio/VirtIO.py  Sat Sep 20 17:17:51 2014 -0400
@@ -0,0 +1,71 @@
+# -*- mode:python -*-
+
+# Copyright (c) 2014 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.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met: redistributions of 

[gem5-dev] changeset in gem5: dev: Add support for 9p proxying over VirtIO

[Andreas Sandberg via gem5-dev]

changeset 4a501e0f7540 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=4a501e0f7540
description:
dev: Add support for 9p proxying over VirtIO

This patch adds support for 9p filesystem proxying over VirtIO. It can
currently operate by connecting to a 9p server over a socket
(VirtIO9PSocket) or by starting the diod 9p server and connecting over
pipe (VirtIO9PDiod).


*WARNING*: Checkpoints are currently not supported for systems with 9p
 proxies!

diffstat:

 src/dev/virtio/SConscript  |4 +
 src/dev/virtio/VirtIO9P.py |   70 ++
 src/dev/virtio/fs9p.cc |  481 +
 src/dev/virtio/fs9p.hh |  371 ++
 4 files changed, 926 insertions(+), 0 deletions(-)

diffs (truncated from 955 to 300 lines):

diff -r 28bc070b5a86 -r 4a501e0f7540 src/dev/virtio/SConscript
--- a/src/dev/virtio/SConscript Sat Sep 20 17:17:53 2014 -0400
+++ b/src/dev/virtio/SConscript Sat Sep 20 17:17:54 2014 -0400
@@ -45,13 +45,17 @@
 SimObject('VirtIO.py')
 SimObject('VirtIOConsole.py')
 SimObject('VirtIOBlock.py')
+SimObject('VirtIO9P.py')
 
 Source('base.cc')
 Source('pci.cc')
 Source('console.cc')
 Source('block.cc')
+Source('fs9p.cc')
 
 DebugFlag('VIO', 'VirtIO base functionality')
 DebugFlag('VIOPci', 'VirtIO PCI transport')
 DebugFlag('VIOConsole', 'VirtIO console device')
 DebugFlag('VIOBlock', 'VirtIO block device')
+DebugFlag('VIO9P', 'General 9p over VirtIO debugging')
+DebugFlag('VIO9PData', 'Dump data in VirtIO 9p connections')
diff -r 28bc070b5a86 -r 4a501e0f7540 src/dev/virtio/VirtIO9P.py
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/src/dev/virtio/VirtIO9P.pySat Sep 20 17:17:54 2014 -0400
@@ -0,0 +1,70 @@
+# -*- mode:python -*-
+
+# Copyright (c) 2014 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.
+#
+# 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.
+#
+# Authors: Andreas Sandberg
+
+from m5.params import *
+from m5.proxy import *
+from VirtIO import VirtIODeviceBase
+
+class VirtIO9PBase(VirtIODeviceBase):
+type = 'VirtIO9PBase'
+abstract = True
+cxx_header = 'dev/virtio/fs9p.hh'
+
+queueSize = Param.Unsigned(32, Output queue size (pages))
+tag = Param.String(gem5, Mount tag)
+
+
+class VirtIO9PProxy(VirtIO9PBase):
+type = 'VirtIO9PProxy'
+abstract = True
+cxx_header = 'dev/virtio/fs9p.hh'
+
+class VirtIO9PDiod(VirtIO9PProxy):
+type = 'VirtIO9PDiod'
+cxx_header = 'dev/virtio/fs9p.hh'
+
+diod = Param.String(/usr/sbin/diod, Path to diod)
+root = Param.String(/tmp, Path to export through diod)
+
+class VirtIO9PSocket(VirtIO9PProxy):
+type = 'VirtIO9PSocket'
+cxx_header = 'dev/virtio/fs9p.hh'
+
+server = Param.String(127.0.0.1, 9P server address or host name)
+port = Param.String(564, 9P server port)
diff -r 28bc070b5a86 -r 4a501e0f7540 src/dev/virtio/fs9p.cc
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/src/dev/virtio/fs9p.ccSat Sep 20 

[gem5-dev] changeset in gem5: dev: Add a VirtIO console device model

[Andreas Sandberg via gem5-dev]

changeset 42d0d62ee057 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=42d0d62ee057
description:
dev: Add a VirtIO console device model

diffstat:

 src/dev/virtio/SConscript   |3 +
 src/dev/virtio/VirtIOConsole.py |   51 +
 src/dev/virtio/console.cc   |  122 +++
 src/dev/virtio/console.hh   |  155 
 4 files changed, 331 insertions(+), 0 deletions(-)

diffs (truncated from 356 to 300 lines):

diff -r a26a20060ba3 -r 42d0d62ee057 src/dev/virtio/SConscript
--- a/src/dev/virtio/SConscript Sat Sep 20 17:17:51 2014 -0400
+++ b/src/dev/virtio/SConscript Sat Sep 20 17:17:52 2014 -0400
@@ -43,9 +43,12 @@
 Return()
 
 SimObject('VirtIO.py')
+SimObject('VirtIOConsole.py')
 
 Source('base.cc')
 Source('pci.cc')
+Source('console.cc')
 
 DebugFlag('VIO', 'VirtIO base functionality')
 DebugFlag('VIOPci', 'VirtIO PCI transport')
+DebugFlag('VIOConsole', 'VirtIO console device')
diff -r a26a20060ba3 -r 42d0d62ee057 src/dev/virtio/VirtIOConsole.py
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/src/dev/virtio/VirtIOConsole.py   Sat Sep 20 17:17:52 2014 -0400
@@ -0,0 +1,51 @@
+# -*- mode:python -*-
+
+# Copyright (c) 2014 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.
+#
+# 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.
+#
+# Authors: Andreas Sandberg
+
+from m5.params import *
+from m5.proxy import *
+from VirtIO import VirtIODeviceBase
+
+class VirtIOConsole(VirtIODeviceBase):
+type = 'VirtIOConsole'
+cxx_header = 'dev/virtio/console.hh'
+
+qRecvSize = Param.Unsigned(16, Receive queue size (descriptors))
+qTransSize = Param.Unsigned(16, Transmit queue size (descriptors))
+
+terminal = Param.Terminal(Parent.any, The terminal)
diff -r a26a20060ba3 -r 42d0d62ee057 src/dev/virtio/console.cc
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/src/dev/virtio/console.cc Sat Sep 20 17:17:52 2014 -0400
@@ -0,0 +1,122 @@
+/*
+ * Copyright (c) 2014 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.
+ *
+ * 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 

[gem5-dev] changeset in gem5: dev: Add a VirtIO block device model

[Andreas Sandberg via gem5-dev]

changeset 28bc070b5a86 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=28bc070b5a86
description:
dev: Add a VirtIO block device model

diffstat:

 src/dev/virtio/SConscript |3 +
 src/dev/virtio/VirtIOBlock.py |   50 +++
 src/dev/virtio/block.cc   |  172 ++
 src/dev/virtio/block.hh   |  186 ++
 4 files changed, 411 insertions(+), 0 deletions(-)

diffs (truncated from 438 to 300 lines):

diff -r 42d0d62ee057 -r 28bc070b5a86 src/dev/virtio/SConscript
--- a/src/dev/virtio/SConscript Sat Sep 20 17:17:52 2014 -0400
+++ b/src/dev/virtio/SConscript Sat Sep 20 17:17:53 2014 -0400
@@ -44,11 +44,14 @@
 
 SimObject('VirtIO.py')
 SimObject('VirtIOConsole.py')
+SimObject('VirtIOBlock.py')
 
 Source('base.cc')
 Source('pci.cc')
 Source('console.cc')
+Source('block.cc')
 
 DebugFlag('VIO', 'VirtIO base functionality')
 DebugFlag('VIOPci', 'VirtIO PCI transport')
 DebugFlag('VIOConsole', 'VirtIO console device')
+DebugFlag('VIOBlock', 'VirtIO block device')
diff -r 42d0d62ee057 -r 28bc070b5a86 src/dev/virtio/VirtIOBlock.py
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/src/dev/virtio/VirtIOBlock.py Sat Sep 20 17:17:53 2014 -0400
@@ -0,0 +1,50 @@
+# -*- mode:python -*-
+
+# Copyright (c) 2014 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.
+#
+# 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.
+#
+# Authors: Andreas Sandberg
+
+from m5.params import *
+from m5.proxy import *
+from VirtIO import VirtIODeviceBase
+
+class VirtIOBlock(VirtIODeviceBase):
+type = 'VirtIOBlock'
+cxx_header = 'dev/virtio/block.hh'
+
+queueSize = Param.Unsigned(128, Output queue size (pages))
+
+image = Param.DiskImage(Disk image)
diff -r 42d0d62ee057 -r 28bc070b5a86 src/dev/virtio/block.cc
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/src/dev/virtio/block.cc   Sat Sep 20 17:17:53 2014 -0400
@@ -0,0 +1,172 @@
+/*
+ * Copyright (c) 2014 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.
+ *
+ * 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 

Re: [gem5-dev] Review Request 2322: SegInit, x86: Segment initialization to support KvmCPU in SE

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2322/#review5336
---

Ship it!


Ship It!

- Andreas Sandberg


On Sept. 16, 2014, 4:37 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2322/
 ---
 
 (Updated Sept. 16, 2014, 4:37 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10302:8bbfa3e4752c
 ---
 SegInit, x86: Segment initialization to support KvmCPU in SE
 This patch sets up low and high privilege code and data segments and places 
 them
 in the following order: cs low, ds low, ds, cs, in the GDT. Additionally, a
 syscall and page fault handler for KvmCPU in SE mode are defined. The order of
 the segment selectors in GDT is required in this manner for interrupt handling
 to work properly. Segment initialization is done for all the thread
 contexts.
 
 
 Diffs
 -
 
   src/arch/x86/process.cc bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/arch/x86/regs/misc.hh bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/arch/x86/system.hh bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/arch/x86/system.cc bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/sim/Process.py bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/sim/process.hh bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/sim/process.cc bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
 
 Diff: http://reviews.gem5.org/r/2322/diff/
 
 
 Testing
 ---
 
 Quick regression tests
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2322: SegInit, x86: Segment initialization to support KvmCPU in SE

[Andreas Sandberg via gem5-dev]

 On Sept. 19, 2014, 10 a.m., Andreas Sandberg wrote:
  Ship It!

Just a minor thing: You should get rid of the SegInit keyword on the summary 
line since that's not in the list of recognized keywords.


- Andreas


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2322/#review5336
---


On Sept. 16, 2014, 4:37 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2322/
 ---
 
 (Updated Sept. 16, 2014, 4:37 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10302:8bbfa3e4752c
 ---
 SegInit, x86: Segment initialization to support KvmCPU in SE
 This patch sets up low and high privilege code and data segments and places 
 them
 in the following order: cs low, ds low, ds, cs, in the GDT. Additionally, a
 syscall and page fault handler for KvmCPU in SE mode are defined. The order of
 the segment selectors in GDT is required in this manner for interrupt handling
 to work properly. Segment initialization is done for all the thread
 contexts.
 
 
 Diffs
 -
 
   src/arch/x86/process.cc bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/arch/x86/regs/misc.hh bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/arch/x86/system.hh bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/arch/x86/system.cc bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/sim/Process.py bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/sim/process.hh bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/sim/process.cc bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
 
 Diff: http://reviews.gem5.org/r/2322/diff/
 
 
 Testing
 ---
 
 Quick regression tests
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2313: kvm, x86: Adding support for SE mode execution

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2313/#review5338
---



src/arch/x86/tlb.cc
http://reviews.gem5.org/r/2313/#comment4848

Could you get rid of this code duplication before committing?

I'd suggest that you rewrite it something like this:

if (m5opRange) {
 ...
} else if (FullSystem) {
 ...
}




src/cpu/kvm/base.hh
http://reviews.gem5.org/r/2313/#comment4849

Is this supposed to be here?



src/sim/pseudo_inst.cc
http://reviews.gem5.org/r/2313/#comment4850

I think these are reserved for temporary user extensions, so you should 
probably use another range.

I'd suggest that you just number the functions as 0x60 and 0x61 and make a 
note that the 0x60-0x6f range is intended for SE mode calls.


Looks good overall, but there are some minor issues I'd like you to look into. 
As far as I'm concerned, you can go ahead and commit this patch when you have 
fixed them.

- Andreas Sandberg


On Sept. 16, 2014, 4:34 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2313/
 ---
 
 (Updated Sept. 16, 2014, 4:34 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10301:9a09f71f7234
 ---
 kvm, x86: Adding support for SE mode execution
 This patch adds methods in KvmCPU model to handle KVM exits caused by syscall
 instructions and page faults. These types of exits will be encountered if
 KvmCPU is run in SE mode.
 
 
 Diffs
 -
 
   src/arch/x86/tlb.cc bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/arch/x86/utility.cc bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/cpu/kvm/base.hh bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/cpu/kvm/base.cc bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/sim/pseudo_inst.hh bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/sim/pseudo_inst.cc bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   src/sim/system.cc bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
   util/m5/m5ops.h bec0c5ffc3237096570fe4c802aeb37e1e396d1b 
 
 Diff: http://reviews.gem5.org/r/2313/diff/
 
 
 Testing
 ---
 
 Quick regressions passed.
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: sim: Automatically unregister probe listeners

[Andreas Sandberg via gem5-dev]

changeset e2c43045a81b in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=e2c43045a81b
description:
sim: Automatically unregister probe listeners

The ProbeListener base class automatically registers itself with a
probe manager. Currently, the class does not unregister a itself when
it is destroyed, which makes removing probes listeners somewhat
cumbersome. This patch adds an automatic call to
manager-removeListener in the ProbeListener destructor, which solves
the problem.

diffstat:

 src/sim/probe/probe.cc |  8 +++-
 src/sim/probe/probe.hh |  6 +-
 2 files changed, 12 insertions(+), 2 deletions(-)

diffs (38 lines):

diff -r c12ec2a0de52 -r e2c43045a81b src/sim/probe/probe.cc
--- a/src/sim/probe/probe.ccTue Sep 09 04:36:34 2014 -0400
+++ b/src/sim/probe/probe.ccTue Sep 09 04:36:43 2014 -0400
@@ -62,11 +62,17 @@
 listeners.clear();
 }
 
-ProbeListener::ProbeListener(ProbeManager *manager, const std::string name)
+ProbeListener::ProbeListener(ProbeManager *_manager, const std::string _name)
+: manager(_manager), name(_name)
 {
 manager-addListener(name, *this);
 }
 
+ProbeListener::~ProbeListener()
+{
+manager-removeListener(name, *this);
+}
+
 ProbeListenerObject*
 ProbeListenerObjectParams::create()
 {
diff -r c12ec2a0de52 -r e2c43045a81b src/sim/probe/probe.hh
--- a/src/sim/probe/probe.hhTue Sep 09 04:36:34 2014 -0400
+++ b/src/sim/probe/probe.hhTue Sep 09 04:36:43 2014 -0400
@@ -104,7 +104,11 @@
 {
   public:
 ProbeListener(ProbeManager *manager, const std::string name);
-virtual ~ProbeListener() {}
+virtual ~ProbeListener();
+
+  protected:
+ProbeManager *const manager;
+const std::string name;
 };
 
 /**
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: sim: Fix resource leak in BaseGlobalEvent

[Andreas Sandberg via gem5-dev]

changeset 280cc9b0794f in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=280cc9b0794f
description:
sim: Fix resource leak in BaseGlobalEvent

Static analysis revealed that BaseGlobalEvent::barrier was never
deallocated. This changeset solves this leak by making the barrier
allocation a part of the BaseGlobalEvent instead of storing a pointer
to a separate heap-allocated barrier.

diffstat:

 src/sim/global_event.cc |  4 ++--
 src/sim/global_event.hh |  4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diffs (36 lines):

diff -r 919c02740209 -r 280cc9b0794f src/sim/global_event.cc
--- a/src/sim/global_event.cc   Tue Sep 09 04:36:31 2014 -0400
+++ b/src/sim/global_event.cc   Tue Sep 09 04:36:32 2014 -0400
@@ -34,9 +34,9 @@
 std::mutex BaseGlobalEvent::globalQMutex;
 
 BaseGlobalEvent::BaseGlobalEvent(Priority p, Flags f)
+: barrier(numMainEventQueues),
+  barrierEvent(numMainEventQueues, NULL)
 {
-barrierEvent.resize(numMainEventQueues);
-barrier = new Barrier(numMainEventQueues);
 }
 
 
diff -r 919c02740209 -r 280cc9b0794f src/sim/global_event.hh
--- a/src/sim/global_event.hh   Tue Sep 09 04:36:31 2014 -0400
+++ b/src/sim/global_event.hh   Tue Sep 09 04:36:32 2014 -0400
@@ -100,7 +100,7 @@
 // while waiting on the barrier to prevent deadlocks if
 // another thread wants to lock the event queue.
 EventQueue::ScopedRelease release(curEventQueue());
-return _globalEvent-barrier-wait();
+return _globalEvent-barrier.wait();
 }
 
   public:
@@ -109,7 +109,7 @@
 
 //! The barrier that all threads wait on before performing the
 //! global event.
-Barrier *barrier;
+Barrier barrier;
 
 //! The individual local event instances (one per thread/event queue).
 std::vectorBarrierEvent * barrierEvent;
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: arch, cpu: Factor out the ExecContext into a ...

[Andreas Sandberg via gem5-dev]

changeset 4207f9bfcceb in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=4207f9bfcceb
description:
arch, cpu: Factor out the ExecContext into a proper base class

We currently generate and compile one version of the ISA code per CPU
model. This is obviously wasting a lot of resources at compile
time. This changeset factors out the interface into a separate
ExecContext class, which also serves as documentation for the
interface between CPUs and the ISA code. While doing so, this
changeset also fixes up interface inconsistencies between the
different CPU models.

The main argument for using one set of ISA code per CPU model has
always been performance as this avoid indirect branches in the
generated code. However, this argument does not hold water. Booting
Linux on a simulated ARM system running in atomic mode
(opt/10.linux-boot/realview-simple-atomic) is actually 2% faster
(compiled using clang 3.4) after applying this patch. Additionally,
compilation time is decreased by 35%.

diffstat:

 SConstruct  |   12 +-
 src/arch/SConscript |   13 +-
 src/arch/arm/isa/includes.isa   |1 +
 src/arch/isa_parser.py  |   22 ++-
 src/cpu/SConscript  |   64 +
 src/cpu/base_dyn_inst.hh|   43 +
 src/cpu/checker/SConsopts   |4 +-
 src/cpu/checker/cpu.hh  |   27 ++-
 src/cpu/exec_context.cc |   40 +
 src/cpu/exec_context.hh |  264 +++
 src/cpu/inorder/SConsopts   |5 +-
 src/cpu/inorder/inorder_dyn_inst.cc |5 +-
 src/cpu/inorder/inorder_dyn_inst.hh |   46 -
 src/cpu/minor/SConsopts |5 +-
 src/cpu/minor/exec_context.hh   |   25 +-
 src/cpu/nocpu/SConsopts |2 +-
 src/cpu/o3/SConsopts|5 +-
 src/cpu/o3/dyn_inst.hh  |   15 +-
 src/cpu/ozone/SConsopts |8 +-
 src/cpu/simple/SConsopts|   10 +-
 src/cpu/simple/base.hh  |   30 ++--
 src/cpu/simple_thread.cc|   16 ++
 src/cpu/static_inst.hh  |   38 ++--
 23 files changed, 406 insertions(+), 294 deletions(-)

diffs (truncated from 1355 to 300 lines):

diff -r 98771a936b61 -r 4207f9bfcceb SConstruct
--- a/SConstructWed Sep 03 07:42:21 2014 -0400
+++ b/SConstructWed Sep 03 07:42:22 2014 -0400
@@ -1025,17 +1025,10 @@
 
 # Dict of available CPU model objects.  Accessible as CpuModel.dict.
 dict = {}
-list = []
-defaults = []
 
 # Constructor.  Automatically adds models to CpuModel.dict.
-def __init__(self, name, filename, includes, strings, default=False):
+def __init__(self, name, default=False):
 self.name = name   # name of model
-self.filename = filename   # filename for output exec code
-self.includes = includes   # include files needed in exec file
-# The 'strings' dict holds all the per-CPU symbols we can
-# substitute into templates etc.
-self.strings = strings
 
 # This cpu is enabled by default
 self.default = default
@@ -1044,7 +1037,6 @@
 if name in CpuModel.dict:
 raise AttributeError, CpuModel '%s' already registered % name
 CpuModel.dict[name] = self
-CpuModel.list.append(name)
 
 Export('CpuModel')
 
@@ -1086,7 +1078,7 @@
 EnumVariable('TARGET_ISA', 'Target ISA', 'alpha', all_isa_list),
 ListVariable('CPU_MODELS', 'CPU models',
  sorted(n for n,m in CpuModel.dict.iteritems() if m.default),
- sorted(CpuModel.list)),
+ sorted(CpuModel.dict.keys())),
 BoolVariable('EFENCE', 'Link with Electric Fence malloc debugger',
  False),
 BoolVariable('SS_COMPATIBLE_FP',
diff -r 98771a936b61 -r 4207f9bfcceb src/arch/SConscript
--- a/src/arch/SConscript   Wed Sep 03 07:42:21 2014 -0400
+++ b/src/arch/SConscript   Wed Sep 03 07:42:22 2014 -0400
@@ -95,13 +95,11 @@
 # The emitter patches up the sources  targets to include the
 # autogenerated files as targets and isa parser itself as a source.
 def isa_desc_emitter(target, source, env):
-cpu_models = list(env['CPU_MODELS'])
-cpu_models.append('CheckerCPU')
-
 # List the isa parser as a source.
-source += [ isa_parser ]
-# Add in the CPU models.
-source += [ Value(m) for m in cpu_models ]
+source += [
+isa_parser,
+Value(ExecContext),
+]
 
 # Specify different targets depending on if we're running the ISA
 # parser for its dependency information, or for the generated files.
@@ -137,8 +135,7 @@
 
 # Skip over the ISA description itself and the parser to the CPU models.
 models = [ s.get_contents() for s in source[2:] ]
-cpu_models = [CpuModel.dict[cpu] for cpu in models]
- 

Re: [gem5-dev] Review Request 2372: style: add .clang-format file

[Andreas Sandberg via gem5-dev]

 On Sept. 1, 2014, 6:14 p.m., Andreas Sandberg wrote:
  .clang-format, line 18
  http://reviews.gem5.org/r/2372/diff/1/?file=41128#file41128line18
 
  Has this changed name? The clang documentation lists 
  DerivePointerAlignment, but not DerivePointerBinding.
 
 Nilay Vaish wrote:
 Documentation for version 3.4 lists DerivePointerBinding.

That explains it. I was looking at the 3.6 documentation. 
(http://clang.llvm.org/docs/ClangFormatStyleOptions.html)


- Andreas


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2372/#review5321
---


On Sept. 3, 2014, 5:50 a.m., Nilay Vaish wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2372/
 ---
 
 (Updated Sept. 3, 2014, 5:50 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10318:34b549ec182b
 ---
 style: add .clang-format file
 
 The format specified in this file is used by clang-format to fix
 the formatting of a given file.  Hopefully, this will ease the burden
 on the developers as they no longer need to manually format things.
 
 
 Diffs
 -
 
   .clang-format PRE-CREATION 
 
 Diff: http://reviews.gem5.org/r/2372/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Nilay Vaish
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2372: style: add .clang-format file

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2372/#review5321
---


I looked into this tool a couple of months ago. At the time, I encountered some 
issues relating to indentation (IIRC, the way we indent case labels by 2 spaces 
wasn't supported). My conclusion at the time was that it wasn't possible to 
configure clang-format to adhere to the gem5 style. Has this changed? If not, 
I'd suggest that we don't commit this for now and wait while clang-format 
matures.


.clang-format
http://reviews.gem5.org/r/2372/#comment4837

Shouldn't this be -2?



.clang-format
http://reviews.gem5.org/r/2372/#comment4838

I used 78 here.



.clang-format
http://reviews.gem5.org/r/2372/#comment4840

Has this changed name? The clang documentation lists 
DerivePointerAlignment, but not DerivePointerBinding.



.clang-format
http://reviews.gem5.org/r/2372/#comment4839

Shouldn't this be 'Linux'? That'd add a newline after 
function/namespace/class definitions.



.clang-format
http://reviews.gem5.org/r/2372/#comment4841

??


- Andreas Sandberg


On Sept. 1, 2014, 7:05 a.m., Nilay Vaish wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2372/
 ---
 
 (Updated Sept. 1, 2014, 7:05 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10318:a79d63495fed
 ---
 style: add .clang-format file
 
 The format specified in this file is used by clang-format to fix
 the formatting of a given file.
 
 
 Diffs
 -
 
   .clang-format PRE-CREATION 
 
 Diff: http://reviews.gem5.org/r/2372/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Nilay Vaish
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] changeset in gem5: style: Fixup strange semantics in hg m5style

[Andreas Sandberg via gem5-dev]

Currently, the only documentation is the doc string, which Mercurial
formats into a usage string, in do_check_style. Is there anything
specific you feel is missing from that piece of documentation?

I just did a quick search on the wiki and it seems like the style script
isn't really documented anywhere. Should I add a section about it to the
Coding Style page?

//Andreas

On 26/08/14 17:57, Steve Reinhardt via gem5-dev wrote:

Thanks for this, Andreas!  I've tried to use it in the past and it never
did what I wanted... I assumed it was broken, not that it was working
correctly according to obscure semantics.

Is there any documentation for this beyond the brief 'usage'  lines in the
script?  It would be great if there were (1) an extended help text like
other mercurial commands and (2) something on the wiki to let people know
it exists and how to get it to work with hg.

Thanks,

Steve



On Tue, Aug 26, 2014 at 8:14 AM, Andreas Sandberg via gem5-dev 
gem5-dev@gem5.org wrote:


changeset 62c95c428a3d in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=62c95c428a3d
description:
 style: Fixup strange semantics in hg m5style

 The 'hg m5style' command had some rather strange semantics. When
 called without arguments, it applied the style checker to all added
 files and modified regions of modified files. However, when
providing
 a list of files, it used that list as an ignore list instead of
 specifically checking those files.

 This patch makes the m5style command behave more like other
Mercurial
 commands where the arguments are used to specify which files to
work
 on instead of which files to ignore.

diffstat:

  util/style.py |  114
-
  1 files changed, 64 insertions(+), 50 deletions(-)

diffs (159 lines):

diff -r 933dfb9d8279 -r 62c95c428a3d util/style.py
--- a/util/style.py Tue Aug 26 10:13:45 2014 -0400
+++ b/util/style.py Tue Aug 26 10:14:07 2014 -0400
@@ -1,4 +1,16 @@
  #! /usr/bin/env python
+# Copyright (c) 2014 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) 2007,2011 The Hewlett-Packard Development Company
  # All rights reserved.
@@ -35,7 +47,7 @@

  from os.path import dirname, join as joinpath
  from itertools import count
-from mercurial import bdiff, mdiff
+from mercurial import bdiff, mdiff, commands

  current_dir = dirname(__file__)
  sys.path.insert(0, current_dir)
@@ -378,27 +390,25 @@
  msg(i, line, 'improper spacing after %s' %
match.group(1))
  bad()

-def do_check_style(hgui, repo, *files, **args):
-check files for proper m5 style guidelines
+
+def do_check_style(hgui, repo, *pats, **opts):
+check files for proper m5 style guidelines
+
+Without an argument, checks all modified and added files for gem5
+coding style violations. A list of files can be specified to limit
+the checker to a subset of the repository. The style rules are
+normally applied on a diff of the repository state (i.e., added
+files are checked in their entirety while only modifications of
+modified files are checked).
+
+The --all option can be specified to include clean files and check
+modified files in their entirety.
+
  from mercurial import mdiff, util

-auto = args.get('auto', False)
-if auto:
-auto = 'f'
-ui = MercurialUI(hgui, hgui.verbose, auto)
-
-if files:
-files = frozenset(files)
-
-def skip(name):
-# We never want to handle symlinks, so always skip them: If the
location
-# pointed to is a directory, skip it. If the location is a file
inside
-# the gem5 directory, it will be checked as a file, so symlink
can be
-# skipped. If the location is a file outside gem5, we don't want
to
-# check it anyway.
-if os.path.islink(name):
-return True
-return files and name in files
+opt_fix_white = opts.get('fix_white', False)
+opt_all = opts.get('all', False)
+ui = MercurialUI(hgui, hgui.verbose, opt_fix_white)

  def prompt(name, func, regions=all_regions):
  result = ui.prompt((a)bort, (i)gnore, or (f)ix?, 'aif', 'a')
@@ -409,39 +419,40 @@

  return False

-modified, added, removed, deleted, unknown, ignore

Re: [gem5-dev] Review Request 2312: Mem: adding a multi-level page table class

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2312/#review5312
---

Ship it!


Looks good. Thanks for addressing the issues I raised earlier!

Please make sure that Mem: is all lower case when you commit so it complies 
with the commit guidelines.

- Andreas Sandberg


On Aug. 25, 2014, 10:08 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2312/
 ---
 
 (Updated Aug. 25, 2014, 10:08 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10264:5cf3d07a2e8b
 ---
 Mem: adding a multi-level page table class
 This patch defines a multi-level page table class that stores the page table 
 in
 system memory, consistent with ISA specifications. In this way, cpu models 
 that
 use the actual hardware to execute (e.g. KvmCPU), are able to traverse the 
 page
 table.
 
 
 Diffs
 -
 
   src/mem/multi_level_page_table.hh PRE-CREATION 
   src/mem/multi_level_page_table.cc PRE-CREATION 
   src/mem/multi_level_page_table_impl.hh PRE-CREATION 
   src/mem/page_table.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/mem/page_table.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/mem/se_translating_port_proxy.hh 
 c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/process.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
 
 Diff: http://reviews.gem5.org/r/2312/diff/
 
 
 Testing
 ---
 
 Regressions passed.
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2319: Mem: adding architectural page table support for SE mode

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2319/#review5313
---

Ship it!


Ship It!

- Andreas Sandberg


On Aug. 25, 2014, 10:10 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2319/
 ---
 
 (Updated Aug. 25, 2014, 10:10 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10265:d518cde600e0
 ---
 Mem: adding architectural page table support for SE mode
 This patch enables the use of page tables that are stored in system memory
 and respect x86 specification, in SE mode. It defines an architectural
 page table for x86 as a MultiLevelPageTable class and puts a placeholder
 class for other ISAs page tables, giving the possibility for future
 implementation.
 
 
 Diffs
 -
 
   src/arch/alpha/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/arm/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/mips/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/power/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/sparc/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/x86/pagetable.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/x86/pagetable_walker.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/x86/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/x86/system.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/mem/SConscript c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/Process.py c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/process.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
 
 Diff: http://reviews.gem5.org/r/2319/diff/
 
 
 Testing
 ---
 
 Regressions passed.
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: sparc: Fixup bit ordering in the PSTATE bit u...

[Andreas Sandberg via gem5-dev]

changeset e475a7861078 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=e475a7861078
description:
sparc: Fixup bit ordering in the PSTATE bit union

The order of the MSB and LSB bit of the mm field in the PSTATE union
is wrong. Any access to this field will currently be ignored and reads
will always return zero. This patch fixes the ordering so it is MSB,
LSB instead of LSB, MSB.

diffstat:

 src/arch/sparc/miscregs.hh |  2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diffs (12 lines):

diff -r 4966471a1ba1 -r e475a7861078 src/arch/sparc/miscregs.hh
--- a/src/arch/sparc/miscregs.hhTue Aug 26 10:13:03 2014 -0400
+++ b/src/arch/sparc/miscregs.hhTue Aug 26 10:13:23 2014 -0400
@@ -129,7 +129,7 @@
 Bitfield2 priv;
 Bitfield3 am;
 Bitfield4 pef;
-Bitfield6, 7 mm;
+Bitfield7, 6 mm;
 Bitfield8 tle;
 Bitfield9 cle;
 Bitfield10 pid0;
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: base: Add a static assert to check bit union ...

[Andreas Sandberg via gem5-dev]

changeset 4593282280e4 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=4593282280e4
description:
base: Add a static assert to check bit union ranges

If a bit field in a bit union specified as BitfieldLSB, MSB instead
of BitfieldMSB, LSB the code silently fails and the field is read as
zero. This changeset introduces a static assert that tests, at compile
time, that the bit order is correct.

diffstat:

 src/base/bitunion.hh |  3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diffs (13 lines):

diff -r e475a7861078 -r 4593282280e4 src/base/bitunion.hh
--- a/src/base/bitunion.hh  Tue Aug 26 10:13:23 2014 -0400
+++ b/src/base/bitunion.hh  Tue Aug 26 10:13:28 2014 -0400
@@ -85,6 +85,9 @@
 templateint first, int last=first
 class Bitfield : public BitfieldBaseType
 {
+static_assert(first = last,
+  Bitfield ranges must be specified as msb, lsb);
+
   public:
 operator const uint64_t () const
 {
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: style: Fixup strange semantics in hg m5style

[Andreas Sandberg via gem5-dev]

changeset 62c95c428a3d in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=62c95c428a3d
description:
style: Fixup strange semantics in hg m5style

The 'hg m5style' command had some rather strange semantics. When
called without arguments, it applied the style checker to all added
files and modified regions of modified files. However, when providing
a list of files, it used that list as an ignore list instead of
specifically checking those files.

This patch makes the m5style command behave more like other Mercurial
commands where the arguments are used to specify which files to work
on instead of which files to ignore.

diffstat:

 util/style.py |  114 -
 1 files changed, 64 insertions(+), 50 deletions(-)

diffs (159 lines):

diff -r 933dfb9d8279 -r 62c95c428a3d util/style.py
--- a/util/style.py Tue Aug 26 10:13:45 2014 -0400
+++ b/util/style.py Tue Aug 26 10:14:07 2014 -0400
@@ -1,4 +1,16 @@
 #! /usr/bin/env python
+# Copyright (c) 2014 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) 2007,2011 The Hewlett-Packard Development Company
 # All rights reserved.
@@ -35,7 +47,7 @@
 
 from os.path import dirname, join as joinpath
 from itertools import count
-from mercurial import bdiff, mdiff
+from mercurial import bdiff, mdiff, commands
 
 current_dir = dirname(__file__)
 sys.path.insert(0, current_dir)
@@ -378,27 +390,25 @@
 msg(i, line, 'improper spacing after %s' % match.group(1))
 bad()
 
-def do_check_style(hgui, repo, *files, **args):
-check files for proper m5 style guidelines
+
+def do_check_style(hgui, repo, *pats, **opts):
+check files for proper m5 style guidelines
+
+Without an argument, checks all modified and added files for gem5
+coding style violations. A list of files can be specified to limit
+the checker to a subset of the repository. The style rules are
+normally applied on a diff of the repository state (i.e., added
+files are checked in their entirety while only modifications of
+modified files are checked).
+
+The --all option can be specified to include clean files and check
+modified files in their entirety.
+
 from mercurial import mdiff, util
 
-auto = args.get('auto', False)
-if auto:
-auto = 'f'
-ui = MercurialUI(hgui, hgui.verbose, auto)
-
-if files:
-files = frozenset(files)
-
-def skip(name):
-# We never want to handle symlinks, so always skip them: If the 
location
-# pointed to is a directory, skip it. If the location is a file inside
-# the gem5 directory, it will be checked as a file, so symlink can be
-# skipped. If the location is a file outside gem5, we don't want to
-# check it anyway.
-if os.path.islink(name):
-return True
-return files and name in files
+opt_fix_white = opts.get('fix_white', False)
+opt_all = opts.get('all', False)
+ui = MercurialUI(hgui, hgui.verbose, opt_fix_white)
 
 def prompt(name, func, regions=all_regions):
 result = ui.prompt((a)bort, (i)gnore, or (f)ix?, 'aif', 'a')
@@ -409,39 +419,40 @@
 
 return False
 
-modified, added, removed, deleted, unknown, ignore, clean = repo.status()
+
+# Import the match (repository file name matching helper)
+# function. Different versions of Mercurial keep it in different
+# modules and implement them differently.
+try:
+from mercurial import scmutil
+m = scmutil.match(repo[None], pats, opts)
+except ImportError:
+from mercurial import cmdutil
+m = cmdutil.match(repo, pats, opts)
+
+modified, added, removed, deleted, unknown, ignore, clean = \
+repo.status(match=m, clean=opt_all)
+if not opt_all:
+try:
+wctx = repo.workingctx()
+except:
+from mercurial import context
+wctx = context.workingctx(repo)
+
+files = [ (fn, all_regions) for fn in added ] + \
+[ (fn,  modregions(wctx, fn)) for fn in modified ]
+else:
+files = [ (fn, all_regions) for fn in added + modified + clean ]
 
 whitespace = Whitespace(ui)
 sorted_includes = SortedIncludes(ui)
-for fname in added:

[gem5-dev] changeset in gem5: base: Add compiler macros for C++11 final/ove...

[Andreas Sandberg via gem5-dev]

changeset 56772eb01583 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=56772eb01583
description:
base: Add compiler macros for C++11 final/override

Add the macros M5_ATTR_FINAL and M5_ATTR_OVERRIDE which are defined to
final and override respectively if supported by the compiler. This is
done to allow a smooth transition to gcc = 4.7.

diffstat:

 src/base/compiler.hh |  50 ++
 1 files changed, 38 insertions(+), 12 deletions(-)

diffs (65 lines):

diff -r b7715fb7cf9f -r 56772eb01583 src/base/compiler.hh
--- a/src/base/compiler.hh  Tue Aug 26 10:13:31 2014 -0400
+++ b/src/base/compiler.hh  Tue Aug 26 10:13:33 2014 -0400
@@ -43,23 +43,49 @@
 #ifndef __BASE_COMPILER_HH__
 #define __BASE_COMPILER_HH__
 
+// gcc C++11 status: http://gcc.gnu.org/projects/cxx0x.html
+// clang C++11 status: http://clang.llvm.org/cxx_status.html
 // http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
 
-#if defined(__GNUC__)
-#define M5_ATTR_NORETURN  __attribute__((noreturn))
-#define M5_DUMMY_RETURN
-#define M5_VAR_USED __attribute__((unused))
+/* Support for override control (final/override) */
+#undef M5_COMP_HAS_OVERRIDE_CONTROL
+
+#if defined(__GNUC__)  !defined(__clang__) /* Check for gcc */
+
+#  define M5_GCC_VERSION(maj, min) \
+(__GNUC__  (maj) || (__GNUC__ == (maj)  __GNUC_MINOR__ = (min)))
+
+#  define M5_COMP_HAS_OVERRIDE_CONTROL M5_GCC_VERSION(4, 7)
+
+#elif defined(__clang__) /* Check for clang */
+
+#  define M5_COMP_HAS_OVERRIDE_CONTROL __has_feature(cxx_override_control)
+
+#else
+#  error Need to define compiler options in base/compiler.hh
+#endif
+
+
+#if M5_COMP_HAS_OVERRIDE_CONTROL
+#  define M5_ATTR_FINAL final
+#  define M5_ATTR_OVERRIDE override
+#else
+#  define M5_ATTR_FINAL
+#  define M5_ATTR_OVERRIDE
+#endif
+
+#if defined(__GNUC__) // clang or gcc
+#  define M5_ATTR_NORETURN  __attribute__((noreturn))
+#  define M5_DUMMY_RETURN
+#  define M5_VAR_USED __attribute__((unused))
+#  define M5_ATTR_PACKED __attribute__ ((__packed__))
+#  define M5_NO_INLINE __attribute__ ((__noinline__))
+#endif
 
 #if defined(__clang__)
-#define M5_CLASS_VAR_USED M5_VAR_USED
+#  define M5_CLASS_VAR_USED M5_VAR_USED
 #else
-#define M5_CLASS_VAR_USED
-#endif
-
-#define M5_ATTR_PACKED __attribute__ ((__packed__))
-#define M5_NO_INLINE __attribute__ ((__noinline__))
-#else
-#error Need to define compiler options in base/compiler.hh
+#  define M5_CLASS_VAR_USED
 #endif
 
 #endif // __BASE_COMPILER_HH__
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: style: Add support for a style ignore list an...

[Andreas Sandberg via gem5-dev]

changeset b58f6afe14c5 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=b58f6afe14c5
description:
style: Add support for a style ignore list and ignore ext/

There are some directories within the repository where we don't want
to enforce our coding style. Specifically, we don't want the style
hooks to warn whenever we update external code in the ext/ directory.

diffstat:

 util/style.py |  36 
 1 files changed, 36 insertions(+), 0 deletions(-)

diffs (67 lines):

diff -r 62c95c428a3d -r b58f6afe14c5 util/style.py
--- a/util/style.py Tue Aug 26 10:14:07 2014 -0400
+++ b/util/style.py Tue Aug 26 10:14:30 2014 -0400
@@ -67,6 +67,37 @@
 
 format_types = set(('C', 'C++'))
 
+
+def re_ignore(expr):
+Helper function to create regular expression ignore file
+matcher functions
+
+rex = re.compile(expr)
+def match_re(fname):
+return rex.match(fname)
+return match_re
+
+# This list contains a list of functions that are called to determine
+# if a file should be excluded from the style matching rules or
+# not. The functions are called with the file name relative to the
+# repository root (without a leading slash) as their argument. A file
+# is excluded if any function in the list returns true.
+style_ignores = [
+# Ignore external projects as they are unlikely to follow the gem5
+# coding convention.
+re_ignore(^ext/),
+]
+
+def check_ignores(fname):
+Check if a file name matches any of the ignore rules
+
+for rule in style_ignores:
+if rule(fname):
+return True
+
+return False
+
+
 def modified_regions(old_data, new_data):
 regions = Regions()
 beg = None
@@ -408,6 +439,7 @@
 
 opt_fix_white = opts.get('fix_white', False)
 opt_all = opts.get('all', False)
+opt_no_ignore = opts.get('no_ignore', False)
 ui = MercurialUI(hgui, hgui.verbose, opt_fix_white)
 
 def prompt(name, func, regions=all_regions):
@@ -447,6 +479,9 @@
 whitespace = Whitespace(ui)
 sorted_includes = SortedIncludes(ui)
 for fname, mod_regions in files:
+if not opt_no_ignore and check_ignores(fname):
+continue
+
 fpath = joinpath(repo.root, fname)
 
 if whitespace.apply(fpath, prompt, mod_regions):
@@ -515,6 +550,7 @@
 ('w', 'fix-white', False, _(automatically fix whitespace)),
 ('a', 'all', False,
  _(include clean files and unmodified parts of modified files)),
+('', 'no-ignore', False, _(ignore the style ignore list)),
 ] +  commands.walkopts,
 _('hg m5style [-a] [FILE]...')),
 '^m5format' :
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: base: Replace the internal varargs stuff with...

[Andreas Sandberg via gem5-dev]

changeset 933dfb9d8279 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=933dfb9d8279
description:
base: Replace the internal varargs stuff with C++11 constructs

We currently use our own home-baked support for type-safe variadic
functions. This is confusing and somewhat limited (e.g., cprintf only
supports a limited number of arguments). This changeset converts all
uses of our internal varargs support to use C++11 variadic macros.

diffstat:

 src/arch/generic/debugfaults.hh |5 +-
 src/arch/x86/bios/intelmp.cc|2 +-
 src/base/cprintf.hh |   76 -
 src/base/misc.cc|   80 -
 src/base/misc.hh|   67 ++--
 src/base/trace.cc   |   55 +-
 src/base/trace.hh   |   28 +++-
 src/base/varargs.hh |  308 
 8 files changed, 167 insertions(+), 454 deletions(-)

diffs (truncated from 840 to 300 lines):

diff -r 56772eb01583 -r 933dfb9d8279 src/arch/generic/debugfaults.hh
--- a/src/arch/generic/debugfaults.hh   Tue Aug 26 10:13:33 2014 -0400
+++ b/src/arch/generic/debugfaults.hh   Tue Aug 26 10:13:45 2014 -0400
@@ -112,8 +112,9 @@
 class M5VarArgsFault : public M5DebugFault
 {
   public:
-M5VarArgsFault(const std::string format, CPRINTF_DECLARATION) :
-M5DebugFault((DebugFunc)Func, csprintf(format, VARARGS_ALLARGS))
+templatetypename ...Args
+M5VarArgsFault(const std::string format, const Args ...args) :
+M5DebugFault((DebugFunc)Func, csprintf(format, args...))
 {}
 };
 
diff -r 56772eb01583 -r 933dfb9d8279 src/arch/x86/bios/intelmp.cc
--- a/src/arch/x86/bios/intelmp.cc  Tue Aug 26 10:13:33 2014 -0400
+++ b/src/arch/x86/bios/intelmp.cc  Tue Aug 26 10:13:45 2014 -0400
@@ -92,7 +92,7 @@
 if (str.length()  length) {
 memcpy(cleanedString, str.c_str(), length);
 warn(Intel MP configuration table string \%s\ 
-will be truncated to \%s\.\n, str, cleanedString);
+ will be truncated to \%s\.\n, str, (char *)cleanedString);
 } else {
 memcpy(cleanedString, str.c_str(), str.length());
 memset(cleanedString + str.length(), 0, length - str.length());
diff -r 56772eb01583 -r 933dfb9d8279 src/base/cprintf.hh
--- a/src/base/cprintf.hh   Tue Aug 26 10:13:33 2014 -0400
+++ b/src/base/cprintf.hh   Tue Aug 26 10:13:45 2014 -0400
@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2014 ARM Limited
  * Copyright (c) 2002-2006 The Regents of The University of Michigan
  * All rights reserved.
  *
@@ -27,6 +28,7 @@
  *
  * Authors: Nathan Binkert
  *  Steve Reinhardt
+ *  Andreas Sandberg
  */
 
 #ifndef __BASE_CPRINTF_HH__
@@ -38,13 +40,9 @@
 #include string
 
 #include base/cprintf_formats.hh
-#include base/varargs.hh
 
 namespace cp {
 
-#define CPRINTF_DECLARATION VARARGS_DECLARATION(cp::Print)
-#define CPRINTF_DEFINITION VARARGS_DEFINITION(cp::Print)
-
 struct Print
 {
   protected:
@@ -128,33 +126,42 @@
 
 } // namespace cp
 
-typedef VarArgs::Listcp::Print CPrintfArgsList;
+inline void
+ccprintf(cp::Print print)
+{
+print.end_args();
+}
 
-inline void
-ccprintf(std::ostream stream, const char *format, const CPrintfArgsList args)
+
+templatetypename T, typename ...Args void
+ccprintf(cp::Print print, const T value, const Args ...args)
+{
+print.add_arg(value);
+
+ccprintf(print, args...);
+}
+
+
+templatetypename ...Args void
+ccprintf(std::ostream stream, const char *format, const Args ...args)
 {
 cp::Print print(stream, format);
-args.add_args(print);
+
+ccprintf(print, args...);
 }
 
-inline void
-ccprintf(std::ostream stream, const char *format, CPRINTF_DECLARATION)
+
+templatetypename ...Args void
+cprintf(const char *format, const Args ...args)
 {
-cp::Print print(stream, format);
-VARARGS_ADDARGS(print);
+ccprintf(std::cout, format, args...);
 }
 
-inline void
-cprintf(const char *format, CPRINTF_DECLARATION)
-{
-ccprintf(std::cout, format, VARARGS_ALLARGS);
-}
-
-inline std::string
-csprintf(const char *format, CPRINTF_DECLARATION)
+templatetypename ...Args std::string
+csprintf(const char *format, const Args ...args)
 {
 std::stringstream stream;
-ccprintf(stream, format, VARARGS_ALLARGS);
+ccprintf(stream, format, args...);
 return stream.str();
 }
 
@@ -163,31 +170,22 @@
  * time converting const char * to std::string since we don't take
  * advantage of it.
  */
-inline void
-ccprintf(std::ostream stream, const std::string format,
- const CPrintfArgsList args)
+templatetypename ...Args void
+ccprintf(std::ostream stream, const std::string format, const Args ...args)
 {
-ccprintf(stream, format.c_str(), args);
+ccprintf(stream, format.c_str(), args...);
 }
 
-inline void
-ccprintf(std::ostream stream, const std::string format, CPRINTF_DECLARATION)
+templatetypename ...Args void
+cprintf(const std::string format, const Args 

Re: [gem5-dev] Review Request 2319: Mem: adding architectural page table support for SE mode

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2319/#review5246
---



src/arch/x86/pagetable.hh
http://reviews.gem5.org/r/2319/#comment4785

I'd prefer to see this refactored so that the X86 architectural page table 
inherits from the multi-level page table instead. (See RB2312.) I find it very 
hard to believe that there is any performance improvement by using templates 
here.


Thanks for fixing this! This changeset looks good except for the comment above.

- Andreas Sandberg


On July 28, 2014, 11:30 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2319/
 ---
 
 (Updated July 28, 2014, 11:30 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10265:c300dff4dd76
 ---
 Mem: adding architectural page table support for SE mode
 This patch enables the use of page tables that are stored in system memory
 and respect x86 specification, in SE mode. It defines an architectural
 page table for x86 as a MultiLevelPageTable class and puts a placeholder
 class for other ISAs page tables, giving the possibility for future
 implementation.
 
 
 Diffs
 -
 
   src/arch/alpha/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/arm/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/mips/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/power/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/sparc/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/x86/pagetable.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/x86/pagetable_walker.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/x86/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/x86/system.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/mem/SConscript c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/Process.py c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/process.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
 
 Diff: http://reviews.gem5.org/r/2319/diff/
 
 
 Testing
 ---
 
 Regressions passed.
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2312: Mem: adding a multi-level page table class

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2312/#review5245
---



src/mem/multi_level_page_table.hh
http://reviews.gem5.org/r/2312/#comment4786

It'd be great if you could write a short description of how the page table 
works. I know that this is sort of obvious from how page tables are usually 
implemented, but having it documented means that it is easier to reuse the code.

Pseudo code would be nice... :)



src/mem/multi_level_page_table.hh
http://reviews.gem5.org/r/2312/#comment4784

In general, I prefer having a abstract base classes for interfaces since 
that makes documentation and compile-time testing easier and the code becomes 
more self-documenting. I don't think the performance impact in this case would 
be noticeable.

Could you either refactor the code so that the ISA code inherits from 
MultiLevelPageTable instead of using the template? (Or at the very least 
document the ISAOps interface.)



src/mem/multi_level_page_table.hh
http://reviews.gem5.org/r/2312/#comment4787

Constant?



src/mem/multi_level_page_table.hh
http://reviews.gem5.org/r/2312/#comment4788

Constant?



src/mem/multi_level_page_table_impl.hh
http://reviews.gem5.org/r/2312/#comment4789

These should probably go into the initialization list.



src/mem/page_table.hh
http://reviews.gem5.org/r/2312/#comment4783

Any particular reason why these aren't const any more? I kinda like having 
constants declared as such since that means the compiler brings out the stick 
when I screw up...


The issues above are really minor issues. The only major thing is that I'd like 
you to consider refactoring the code slightly to get rid of the template. Keep 
up the good work!

- Andreas Sandberg


On July 28, 2014, 11:29 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2312/
 ---
 
 (Updated July 28, 2014, 11:29 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10264:49232146d7c8
 ---
 Mem: adding a multi-level page table class
 This patch defines a multi-level page table class that stores the page table 
 in
 system memory, consistent with ISA specifications. In this way, cpu models 
 that
 use the actual hardware to execute (e.g. KvmCPU), are able to traverse the 
 page
 table.
 
 
 Diffs
 -
 
   src/mem/multi_level_page_table.hh PRE-CREATION 
   src/mem/multi_level_page_table.cc PRE-CREATION 
   src/mem/multi_level_page_table_impl.hh PRE-CREATION 
   src/mem/page_table.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/mem/page_table.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/mem/se_translating_port_proxy.hh 
 c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/process.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
 
 Diff: http://reviews.gem5.org/r/2312/diff/
 
 
 Testing
 ---
 
 Regressions passed.
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2322: SegInit, x86: Segment initialization to support KvmCPU in SE

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2322/#review5249
---

Ship it!



src/sim/process.hh
http://reviews.gem5.org/r/2322/#comment4791

Style issue: Member variables should not start with initial caps. Rename to 
kvmInSE or useKvmInSE.


Looks good other than the style issue above. As far as I'm concerned, you don't 
need to re-post after fixing that.

- Andreas Sandberg


On Aug. 1, 2014, 4:53 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2322/
 ---
 
 (Updated Aug. 1, 2014, 4:53 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10268:53d5cfcbb5a1
 ---
 SegInit, x86: Segment initialization to support KvmCPU in SE
 This patch sets up low and high privilege code and data segments and places 
 them
 in the following order: cs low, ds low, ds, cs, in the GDT. Additionally, a
 syscall and page fault handler for KvmCPU in SE mode are defined. The order of
 the segment selectors in GDT is required in this manner for interrupt handling
 to work properly. Segment initialization is done for all the thread
 contexts.
 
 
 Diffs
 -
 
   src/arch/x86/process.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/x86/regs/misc.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/x86/system.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/arch/x86/system.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/Process.py c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/process.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/sim/process.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
 
 Diff: http://reviews.gem5.org/r/2322/diff/
 
 
 Testing
 ---
 
 Quick regression tests
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2313: kvm, x86: Adding support for SE mode execution

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2313/#review5250
---


I'm really not happy about the use of kvm-specific ports magic here. It would 
have been nicer having a m5ops based interface that just passes the 
fault/syscall to the arch-specific code from any CPU model that uses the m5ops 
interface. Specifically, I'm a bit concerned about what would happen if you 
switch in the syscall or fault handler code before you enter into gem5. In this 
case the fault/syscall will be lost since the simulated CPUs don't know how to 
handle the port magic. If you have a good reason to believe that this won't be 
an issue, I'm happy to have this committed. I have fixed quite a few switching 
bugs in the past and I know that things like these are likely to come back and 
bite you and are a pain to diagnose.


- Andreas Sandberg


On Aug. 1, 2014, 4:52 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2313/
 ---
 
 (Updated Aug. 1, 2014, 4:52 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10267:46ad52c66c87
 ---
 kvm, x86: Adding support for SE mode execution
 This patch adds methods in KvmCPU model to handle KVM exits caused by syscall
 instructions and page faults. These types of exits will be encountered if
 KvmCPU is run in SE mode.
 
 
 Diffs
 -
 
   src/arch/x86/system.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/cpu/kvm/base.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/cpu/kvm/base.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/cpu/kvm/x86_cpu.hh c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
   src/cpu/kvm/x86_cpu.cc c00b5ba43967e7e48a28b7ddc48c9f4afaf2ab76 
 
 Diff: http://reviews.gem5.org/r/2313/diff/
 
 
 Testing
 ---
 
 Quick regressions passed.
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: util: Fix state leakage in the SortIncludes s...

[Andreas Sandberg via gem5-dev]

changeset 84b4d6af0ecc in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=84b4d6af0ecc
description:
util: Fix state leakage in the SortIncludes style verifier

There are cases where the state of a SortIncludes object gets messed
up and leaks between invocations/files. This typically happens when a
file ends with an include block (dump_block() gets called at the end
of __call__). In this case, the state of the class is not reset
between files. This bug manifests itself as ghost includes that leak
between files when applying the style hooks.

This changeset adds a reset at the beginning of the __call__ method
which ensures that the class is always in a clean state when
processing a new file.

diffstat:

 util/sort_includes.py |  3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diffs (20 lines):

diff -r 68da5ef4bb6f -r 84b4d6af0ecc util/sort_includes.py
--- a/util/sort_includes.py Wed Aug 13 06:57:24 2014 -0400
+++ b/util/sort_includes.py Wed Aug 13 06:57:25 2014 -0400
@@ -72,7 +72,7 @@
 includes_re = tuple((a, b, re.compile(c)) for a,b,c in includes_re)
 
 def __init__(self):
-self.reset()
+pass
 
 def reset(self):
 # clear all stored headers
@@ -103,6 +103,7 @@
 prev = l
 
 def __call__(self, lines, filename, language):
+self.reset()
 leading_blank = False
 blanks = 0
 block = False
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: scons: Build the branch predictor for all CPUs

[Andreas Sandberg via gem5-dev]

changeset c7187ee80868 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=c7187ee80868
description:
scons: Build the branch predictor for all CPUs

The branch predictor is normally only built when a CPU that uses a
branch predictor is built. The list of CPUs is currently incomplete as
the simple CPUs support branch predictors (for warming, branch stats,
etc). In practice, all CPU models now use branch predictors, so this
changeset removes the CPU model check and replaces it with a check for
the NULL ISA.

diffstat:

 src/cpu/pred/SConscript |  23 ---
 1 files changed, 12 insertions(+), 11 deletions(-)

diffs (31 lines):

diff -r 5b67e1bdf6ad -r c7187ee80868 src/cpu/pred/SConscript
--- a/src/cpu/pred/SConscript   Wed Aug 13 06:57:30 2014 -0400
+++ b/src/cpu/pred/SConscript   Wed Aug 13 06:57:31 2014 -0400
@@ -30,15 +30,16 @@
 
 Import('*')
 
-if 'InOrderCPU' in env['CPU_MODELS'] or 'O3CPU' in env['CPU_MODELS'] \
-or 'Minor' in env['CPU_MODELS']:
-SimObject('BranchPredictor.py')
+if env['TARGET_ISA'] == 'null':
+Return()
 
-Source('bpred_unit.cc')
-Source('2bit_local.cc')
-Source('btb.cc')
-Source('ras.cc')
-Source('tournament.cc')
-Source ('bi_mode.cc')
-DebugFlag('FreeList')
-DebugFlag('Branch')
+SimObject('BranchPredictor.py')
+
+Source('bpred_unit.cc')
+Source('2bit_local.cc')
+Source('btb.cc')
+Source('ras.cc')
+Source('tournament.cc')
+Source ('bi_mode.cc')
+DebugFlag('FreeList')
+DebugFlag('Branch')
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: base: Remove unused M5_PRAGMA_NORETURN

[Andreas Sandberg via gem5-dev]

changeset ef888b246cd0 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=ef888b246cd0
description:
base: Remove unused M5_PRAGMA_NORETURN

The M5_PRAGMA_NORETURN macro was only used in for
__exit_message. Since the macro only holds a stub definition and all
functions with noreturn semantics use the M5_ATTR_NORETURN, this
macros is completely redundant.

diffstat:

 src/base/compiler.hh |  1 -
 src/base/misc.hh |  1 -
 2 files changed, 0 insertions(+), 2 deletions(-)

diffs (22 lines):

diff -r 4cbfdcdb2144 -r ef888b246cd0 src/base/compiler.hh
--- a/src/base/compiler.hh  Wed Aug 13 06:57:26 2014 -0400
+++ b/src/base/compiler.hh  Wed Aug 13 06:57:27 2014 -0400
@@ -47,7 +47,6 @@
 
 #if defined(__GNUC__)
 #define M5_ATTR_NORETURN  __attribute__((noreturn))
-#define M5_PRAGMA_NORETURN(x)
 #define M5_DUMMY_RETURN
 #define M5_VAR_USED __attribute__((unused))
 
diff -r 4cbfdcdb2144 -r ef888b246cd0 src/base/misc.hh
--- a/src/base/misc.hh  Wed Aug 13 06:57:26 2014 -0400
+++ b/src/base/misc.hh  Wed Aug 13 06:57:27 2014 -0400
@@ -71,7 +71,6 @@
VARARGS_ALLARGS);
 }
 
-M5_PRAGMA_NORETURN(__exit_message)
 #define exit_message(prefix, code, ...)\
 __exit_message(prefix, code, __FUNCTION__, __FILE__, __LINE__, \
__VA_ARGS__)
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: power: Remove unused private members to fix c...

[Andreas Sandberg via gem5-dev]

changeset faa9dfc465ef in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=faa9dfc465ef
description:
power: Remove unused private members to fix compile-time warning

Certain versions of clang complain about unused private members if
they are not used. This changeset removes such members from the
POWER-specific ProcessInfo struct to silence the warning.

diffstat:

 src/arch/power/stacktrace.cc |  1 -
 src/arch/power/stacktrace.hh |  9 -
 2 files changed, 0 insertions(+), 10 deletions(-)

diffs (30 lines):

diff -r 362875aec1ba -r faa9dfc465ef src/arch/power/stacktrace.cc
--- a/src/arch/power/stacktrace.cc  Wed Aug 13 06:57:28 2014 -0400
+++ b/src/arch/power/stacktrace.cc  Wed Aug 13 06:57:29 2014 -0400
@@ -38,7 +38,6 @@
 namespace PowerISA {
 
 ProcessInfo::ProcessInfo(ThreadContext *_tc)
-: tc(_tc)
 {
 panic(ProcessInfo constructor not implemented.\n);
 }
diff -r 362875aec1ba -r faa9dfc465ef src/arch/power/stacktrace.hh
--- a/src/arch/power/stacktrace.hh  Wed Aug 13 06:57:28 2014 -0400
+++ b/src/arch/power/stacktrace.hh  Wed Aug 13 06:57:29 2014 -0400
@@ -47,15 +47,6 @@
 
 class ProcessInfo
 {
-  private:
-ThreadContext *tc;
-
-int thread_info_size;
-int task_struct_size;
-int task_off;
-int pid_off;
-int name_off;
-
   public:
 ProcessInfo(ThreadContext *_tc);
 
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: scons: Silence clang 3.4 warnings on Ubuntu 1...

[Andreas Sandberg via gem5-dev]

changeset 362875aec1ba in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=362875aec1ba
description:
scons: Silence clang 3.4 warnings on Ubuntu 12.04

This changeset fixes three types of warnings that occur in clang 3.4
on Ubuntu 12.04:

 * Certain versions of libstdc++ (primarily 4.8) use struct and class
   interchangeably. This triggers a warning in clang.

 * Swig has a tendency to generate code with the register class which
   was deprecated in C++11. This triggers a deprecation warning in
   clang.

 * Swig sometimes generates Python wrapper code which returns
   uninitialized values. It's unclear if this is actually a problem
   (the cases might be limited to failure paths). We'll silence these
   warnings for now since there is little we can do about the
   generated code.

diffstat:

 SConstruct |  7 ++-
 src/SConscript |  8 
 2 files changed, 14 insertions(+), 1 deletions(-)

diffs (35 lines):

diff -r ef888b246cd0 -r 362875aec1ba SConstruct
--- a/SConstructWed Aug 13 06:57:27 2014 -0400
+++ b/SConstructWed Aug 13 06:57:28 2014 -0400
@@ -638,7 +638,12 @@
 # is relying on this
 main.Append(CCFLAGS=['-Wno-tautological-compare',
  '-Wno-parentheses',
- '-Wno-self-assign'])
+ '-Wno-self-assign',
+ # Some versions of libstdc++ (4.8?) seem to
+ # use struct hash and class hash
+ # interchangeably.
+ '-Wno-mismatched-tags',
+ ])
 
 main.Append(TCMALLOC_CCFLAGS=['-fno-builtin'])
 
diff -r ef888b246cd0 -r 362875aec1ba src/SConscript
--- a/src/SConscriptWed Aug 13 06:57:27 2014 -0400
+++ b/src/SConscriptWed Aug 13 06:57:28 2014 -0400
@@ -940,6 +940,14 @@
 # with non-virtual destructors
 new_env.Append(CXXFLAGS=['-Wdelete-non-virtual-dtor'])
 
+swig_env.Append(CCFLAGS=[
+# Some versions of SWIG can return uninitialized values
+'-Wno-sometimes-uninitialized',
+# Register storage is requested in a lot of places in
+# SWIG-generated code.
+'-Wno-deprecated-register',
+])
+
 werror_env = new_env.Clone()
 werror_env.Append(CCFLAGS='-Werror')
 
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: cpu: Don't forward declare RefCountingPtr

[Andreas Sandberg via gem5-dev]

changeset 4cbfdcdb2144 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=4cbfdcdb2144
description:
cpu: Don't forward declare RefCountingPtr

RefCountingPtr is sometimes forward declared to avoid having to
include refcnt.hh. This does not work since we typically return
instances of RefCountingPtr rather than references to instances. The
only reason this currently works is that we include refcnt.hh in
cprintf.hh, which leaks the header to most other source files. This
changeset replaces such forward declarations with an include of
refcnt.hh.

diffstat:

 src/base/types.hh  |  3 ++-
 src/cpu/static_inst_fwd.hh |  3 ++-
 src/sim/fault_fwd.hh   |  3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diffs (48 lines):

diff -r 84b4d6af0ecc -r 4cbfdcdb2144 src/base/types.hh
--- a/src/base/types.hh Wed Aug 13 06:57:25 2014 -0400
+++ b/src/base/types.hh Wed Aug 13 06:57:26 2014 -0400
@@ -42,6 +42,8 @@
 #include cassert
 #include ostream
 
+#include base/refcnt.hh
+
 /** uint64_t constant */
 #define ULL(N)  ((uint64_t)N##ULL)
 /** int64_t constant */
@@ -177,7 +179,6 @@
 const PortID InvalidPortID = (PortID)-1;
 
 class FaultBase;
-template class T class RefCountingPtr;
 typedef RefCountingPtrFaultBase Fault;
 
 #endif // __BASE_TYPES_HH__
diff -r 84b4d6af0ecc -r 4cbfdcdb2144 src/cpu/static_inst_fwd.hh
--- a/src/cpu/static_inst_fwd.hhWed Aug 13 06:57:25 2014 -0400
+++ b/src/cpu/static_inst_fwd.hhWed Aug 13 06:57:26 2014 -0400
@@ -31,8 +31,9 @@
 #ifndef __CPU_STATIC_INST_FWD_HH__
 #define __CPU_STATIC_INST_FWD_HH__
 
+#include base/refcnt.hh
+
 class StaticInst;
-template class T class RefCountingPtr;
 typedef RefCountingPtrStaticInst StaticInstPtr;
 
 #endif // __CPU_STATIC_INST_FWD_HH__
diff -r 84b4d6af0ecc -r 4cbfdcdb2144 src/sim/fault_fwd.hh
--- a/src/sim/fault_fwd.hh  Wed Aug 13 06:57:25 2014 -0400
+++ b/src/sim/fault_fwd.hh  Wed Aug 13 06:57:26 2014 -0400
@@ -31,8 +31,9 @@
 #ifndef __SIM_FAULT_FWD_HH__
 #define __SIM_FAULT_FWD_HH__
 
+#include base/refcnt.hh
+
 class FaultBase;
-template class T class RefCountingPtr;
 typedef RefCountingPtrFaultBase Fault;
 
 FaultBase * const NoFault = 0;
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in gem5: mips: Remove unused private members to fix co...

[Andreas Sandberg via gem5-dev]

changeset 5b67e1bdf6ad in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=5b67e1bdf6ad
description:
mips: Remove unused private members to fix compile-time warning

Certain versions of clang complain about unused private members if
they are not used. This changeset removes such members from the
MIPS-specific classes to silence the warning.

diffstat:

 src/arch/mips/interrupts.hh   |   6 --
 src/arch/mips/linux/system.hh |  39 ---
 src/arch/mips/stacktrace.hh   |   2 --
 src/arch/mips/tlb.cc  |  15 ---
 src/dev/mips/malta_io.hh  |   3 ---
 5 files changed, 0 insertions(+), 65 deletions(-)

diffs (122 lines):

diff -r faa9dfc465ef -r 5b67e1bdf6ad src/arch/mips/interrupts.hh
--- a/src/arch/mips/interrupts.hh   Wed Aug 13 06:57:29 2014 -0400
+++ b/src/arch/mips/interrupts.hh   Wed Aug 13 06:57:30 2014 -0400
@@ -59,7 +59,6 @@
 
 Interrupts(Params * p) : SimObject(p)
 {
-newInfoSet = false;
 }
 
 void
@@ -127,11 +126,6 @@
 {
 fatal(Unserialization of Interrupts Unimplemented for MIPS);
 }
-
-  private:
-bool newInfoSet;
-int newIpl;
-int newSummary;
 };
 
 }
diff -r faa9dfc465ef -r 5b67e1bdf6ad src/arch/mips/linux/system.hh
--- a/src/arch/mips/linux/system.hh Wed Aug 13 06:57:29 2014 -0400
+++ b/src/arch/mips/linux/system.hh Wed Aug 13 06:57:30 2014 -0400
@@ -87,45 +87,6 @@
 Addr InitrdSize() const { return Param() + 0x108; }
 static const int CommandLineSize = 256;
 
-  private:
-#ifndef NDEBUG
-/** Event to halt the simulator if the kernel calls panic()  */
-BreakPCEvent *kernelPanicEvent;
-
-/** Event to halt the simulator if the kernel calls die_if_kernel  */
-BreakPCEvent *kernelDieEvent;
-#endif
-
-/**
- * Event to skip determine_cpu_caches() because we don't support
- * the IPRs that the code can access to figure out cache sizes
- */
-SkipFuncEvent *skipCacheProbeEvent;
-
-/** PC based event to skip the ide_delay_50ms() call */
-SkipFuncEvent *skipIdeDelay50msEvent;
-
-/**
- * PC based event to skip the dprink() call and emulate its
- * functionality
- */
-Linux::DebugPrintkEvent *debugPrintkEvent;
-
-/**
- * Skip calculate_delay_loop() rather than waiting for this to be
- * calculated
- */
-SkipDelayLoopEvent *skipDelayLoopEvent;
-
-/**
- * Event to print information about thread switches if the trace flag
- * Thread is set
- */
-PrintThreadInfo *printThreadEvent;
-
-/** Grab the PCBB of the idle process when it starts */
-IdleStartEvent *idleStartEvent;
-
   public:
 typedef LinuxMipsSystemParams Params;
 LinuxMipsSystem(Params *p);
diff -r faa9dfc465ef -r 5b67e1bdf6ad src/arch/mips/stacktrace.hh
--- a/src/arch/mips/stacktrace.hh   Wed Aug 13 06:57:29 2014 -0400
+++ b/src/arch/mips/stacktrace.hh   Wed Aug 13 06:57:30 2014 -0400
@@ -45,8 +45,6 @@
   private:
 ThreadContext *tc;
 
-int thread_info_size;
-int task_struct_size;
 int task_off;
 int pid_off;
 int name_off;
diff -r faa9dfc465ef -r 5b67e1bdf6ad src/arch/mips/tlb.cc
--- a/src/arch/mips/tlb.cc  Wed Aug 13 06:57:29 2014 -0400
+++ b/src/arch/mips/tlb.cc  Wed Aug 13 06:57:30 2014 -0400
@@ -59,21 +59,6 @@
 //  MIPS TLB
 //
 
-static inline mode_type
-getOperatingMode(MiscReg Stat)
-{
-if ((Stat  0x1006) != 0 || (Stat  0x18) ==0) {
-return mode_kernel;
-} else if ((Stat  0x18) == 0x8) {
-return mode_supervisor;
-} else if ((Stat  0x18) == 0x10) {
-return mode_user;
-} else {
-return mode_number;
-}
-}
-
-
 TLB::TLB(const Params *p)
 : BaseTLB(p), size(p-size), nlu(0)
 {
diff -r faa9dfc465ef -r 5b67e1bdf6ad src/dev/mips/malta_io.hh
--- a/src/dev/mips/malta_io.hh  Wed Aug 13 06:57:29 2014 -0400
+++ b/src/dev/mips/malta_io.hh  Wed Aug 13 06:57:30 2014 -0400
@@ -51,9 +51,6 @@
  */
 class MaltaIO : public BasicPioDevice
 {
-  private:
-struct tm tm;
-
   protected:
 
 class RTC : public MC146818
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] RFC: New include file ordering

[Andreas Sandberg via gem5-dev]

Hi Everyone,

I have a change I'd like to make to the gem5 coding style that I believe
will improve our code quality.

Currently, the gem5 coding style mandates that includes are grouped four
different blocks (alphabetical ordering within a block):

  * Python headers
  * C system/stdlib includes
  * C++ stdlib includes
  * M5 includes

I propose that we change this to include an object's main header file
first (e.g., foo.cc would include foo.hh first). This ensures that the
header file does not depend on include file ordering and avoids
surprises down the road when someone tries to reuse code. This kind of
include file ordering is pretty common and is used at, for example,
Google [1].

Comments/ideas?

If everyone is happy with this change, I'll go ahead and post a patch
for the style checker and update the Wiki. In order to keep the code
base reasonably stable, I propose that we only apply this to new code
and gradually migrate the old code.

Thanks,
Andreas

[1]
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Names_and_Order_of_Includes


-- 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.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in 
England  Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England  Wales, Company No:  2548782

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2301: config, x86: add ethernet support for x86 fullsystem

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2301/#review5213
---

Ship it!


LGTM. Thanks for fixing this!

- Andreas Sandberg


On July 18, 2014, 1:29 a.m., Jiuyue Ma wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2301/
 ---
 
 (Updated July 18, 2014, 1:29 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 config, x86: add ethernet support for x86 fullsystem
 
 This patch add a IGbE_e1000 ethernet device to x86 fs system, it does
 the followings:
 1) add IGbE_e1000 to x86_sys.pc.ethernet,
 2) connect x86_sys.pc.ethernet.pio/config/dma to x86_sys.iobus,
 3) add interrupt assignment for x86_sys.pc.ethernet in MP table.
 
 
 Diffs
 -
 
   configs/common/FSConfig.py 878f2f30b12d38f619b80b5d80d52498946f6ad1 
 
 Diff: http://reviews.gem5.org/r/2301/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jiuyue Ma
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2314: config: swap bus_id of ISA/PCI in X86 IntelMPTable

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2314/#review5196
---

Ship it!


Thanks for taking the time to fix this! This looks good, I'm happy to have this 
committed.

I'd suggest that you add a line to the commit message saying that this fixes 
PCI interrupt routing and discovery on Linux.

- Andreas Sandberg


On July 17, 2014, 6:27 a.m., Jiuyue Ma wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2314/
 ---
 
 (Updated July 17, 2014, 6:27 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 config: swap bus_id of ISA/PCI in X86 IntelMPTable
 
 This patch assign bus_id=0 to PCI bus and bus_id=1 to ISA bus for
 X86 platform. Because PCI device get config space address using
 Pc::calcPciConfigAddr() which requires assert(bus==0).
 
 
 Diffs
 -
 
   configs/common/FSConfig.py 878f2f30b12d38f619b80b5d80d52498946f6ad1 
 
 Diff: http://reviews.gem5.org/r/2314/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jiuyue Ma
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2315: config: force IO devices mapped to 0xC0000000-0xFFFF0000 and bridge these address to iobus for x86

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2315/#review5197
---

Ship it!


Thanks for fixing this!

The changeset looks good, but I'd appreciate if you could shorten the summary 
line (the first line) of the commit message to comply with the commit 
requirements (http://www.gem5.org/Commit_Access). The maximum line length for 
the summary line is 65 characters.

Something like this should suffice as a summary line:
config, x86: Ensure that PCI devs get bridged to the memory bus

Other than the above, I'm happy to have this committed.

- Andreas Sandberg


On July 17, 2014, 6:29 a.m., Jiuyue Ma wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2315/
 ---
 
 (Updated July 17, 2014, 6:29 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 config: force IO devices mapped to 0xC000-0x and bridge these 
 address to iobus for x86
 
 This patch force IO device to be mapped to 0xC000-0x by
 reserve anything between the end of memory and 3GB if memory is less
 than 3GB. It also statically bridge these address range to the IO bus,
 which guaranty access to pci address space will pass though bridge to
 iobus.
 
 
 Diffs
 -
 
   configs/common/FSConfig.py 878f2f30b12d38f619b80b5d80d52498946f6ad1 
 
 Diff: http://reviews.gem5.org/r/2315/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jiuyue Ma
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2301: config: add ethernet support for x86 fullsystem

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2301/#review5200
---


I'm sorry I didn't spot this earlier, but it seems like you forgot to include 
the ethernet device's interrupt in the MP table. See the interrupt declaration 
for pci_dev4_inta for an example. Without that entry, Linux won't be able to 
setup interrupt routing properly.

- Andreas Sandberg


On July 17, 2014, 5:35 a.m., Jiuyue Ma wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2301/
 ---
 
 (Updated July 17, 2014, 5:35 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 config: add ethernet support for x86 fullsystem
 
 This patch add a IGbE_e1000 ethernet device to x86 fs system.
 
 
 Diffs
 -
 
   configs/common/FSConfig.py 878f2f30b12d38f619b80b5d80d52498946f6ad1 
 
 Diff: http://reviews.gem5.org/r/2301/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jiuyue Ma
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2301: config: add ethernet support for x86 fullsystem

[Andreas Sandberg via gem5-dev]

 On July 17, 2014, 10:35 a.m., Andreas Sandberg wrote:
  I'm sorry I didn't spot this earlier, but it seems like you forgot to 
  include the ethernet device's interrupt in the MP table. See the interrupt 
  declaration for pci_dev4_inta for an example. Without that entry, Linux 
  won't be able to setup interrupt routing properly.
 
 Jiuyue Ma wrote:
 We don't need anything in MP table, kernel can configure PCI device's 
 interrupt itself :)
 Maybe only ISA device need a MP table entry?
 
 After apply 2314/2315/2310 and 2300(needed by 2.6.28.4; 2.6.22.9 did not 
 care this),
 I can boot system and discoverconfigure ethernet device successfully.
 
 --- boot log 
 Intel(R) PRO/1000 Network Driver - version 7.3.20-k2Copyright (c) 
 1999-2006 Intel Corporation.
 PCI: Enabling device :00:02.0 ( - 0002)
 e1000: :00:02.0: e1000_probe: (PCI:33MHz:32-bit) 00:90:00:00:00:01
 e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection
 
 --- config 
 loading script...
 Script from M5 readfile is empty, starting bash shell...
 (none) / # ifconfig eth0 192.168.0.1
 ADDRCONF(NETDEV_UP): eth0: link is not ready
 (none) / # e1000: eth0: e1000_watchdog: NIC Link is Up 1000 Mbps Full 
 Duplex, Flow Control: None
 ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
 ifconfig lo 127.0.0.1
 (none) / # ping 192.168.0.1
 PING 192.168.0.1 (192.168.0.1) 56(84) bytes of data.
 64 bytes from 192.168.0.1: icmp_seq=1 ttl=64 time=0.008 ms
 
 --- 192.168.0.1 ping statistics ---
 1 packets transmitted, 1 received, 0% packet loss, time 0ms
 rtt min/avg/max/mdev = 0.008/0.008/0.008/0.000 ms

It probably depends on the kernel version. I tested with 3.4.94 and got a 
warning that the MP table was broken because it didn't find the interrupt 
routing information for the ethernet device. I think the warning even stated 
that the kernel was unable to figure out the interrupt.


- Andreas


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2301/#review5200
---


On July 17, 2014, 5:35 a.m., Jiuyue Ma wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2301/
 ---
 
 (Updated July 17, 2014, 5:35 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 config: add ethernet support for x86 fullsystem
 
 This patch add a IGbE_e1000 ethernet device to x86 fs system.
 
 
 Diffs
 -
 
   configs/common/FSConfig.py 878f2f30b12d38f619b80b5d80d52498946f6ad1 
 
 Diff: http://reviews.gem5.org/r/2301/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jiuyue Ma
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2301: config: add ethernet support for x86 fullsystem

[Andreas Sandberg via gem5-dev]

On 16/07/14 11:01, Jiuyue Ma via gem5-dev wrote:



On July 15, 2014, 1:26 p.m., Andreas Sandberg wrote:

Could you split this into two (or potentially three) different patches?

The PCI/ISA bus ID fixes look fine and should definitely go upstream ASAP. As 
far as I'm concerned that particular part of the patch can be submitted 
separately right away as it is a small bug fixes that is limited to a few lines.

I'm not so sure about the bridge ranges though. The PCI specification seems to 
allow devices to be mapped to pretty much any region of the memory space, so we 
can probably not do what you're doing currently to determine the range. 
Unfortunately, I can't think of a good solution off the top of my head. I'll 
discuss it with some colleagues and get back to you.

Split into three patches may be better:
1) PCI/ISA bus ID fixes
2) add/connect ethernet device
3) bridge range fixes
I will do this split latter tomorrow.


Thank you! That's a good split.


About the bridge range, I have tried to modify recvRangeChange() interface of 
Bridge: change bridge's slave port range when its master side changed.
I think this should be a better solution than current one. But I failed to deal 
with the address conflict of mem_ctrls and iobridge/iocache in membus. T_T


I've discussed this with a couple of colleagues and it is not obvious
how to sort out the bridge issue. I think the bridge used to
automatically discover which device ranges were connected and magically
just work (in most cases), but this could get pretty hairy in the
general case depending on bus topology.

I checked how Linux handles address assignments (see e820.c somewhere in
arch/x86/) and it simply looks for a big hole (don't remember the
minimum size) in the memory map below 4GB. In practice, this finds the
hole that starts at around 3GB on most PCs. We could exploit this
behaviour to force IO devices to be mapped in a specific range (gem5
seems to assume 0xC000-0x for devices). That would require
the following changes:

 * Statically bridge 0xC000-0x to the IO bus. (The upper
64kB are reserved for m5ops.)
 * Make sure that no E820 entry covers the IO range. (There isn't a
separate device entry type.)
 * If RAM is less than 3GB reserve anything between the end of ram and
3GB. (I.e., everything below 3GB is covered by the E820 map and is
either ram or reserved.)


I think the above would be the simplest solution for now since it
doesn't involve mucking around with the bridge and it should just work.

//Andreas


-- 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.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in 
England  Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England  Wales, Company No:  2548782

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2301: config: add ethernet support for x86 fullsystem

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2301/#review5190
---


Could you split this into two (or potentially three) different patches?

The PCI/ISA bus ID fixes look fine and should definitely go upstream ASAP. As 
far as I'm concerned that particular part of the patch can be submitted 
separately right away as it is a small bug fixes that is limited to a few lines.

I'm not so sure about the bridge ranges though. The PCI specification seems to 
allow devices to be mapped to pretty much any region of the memory space, so we 
can probably not do what you're doing currently to determine the range. 
Unfortunately, I can't think of a good solution off the top of my head. I'll 
discuss it with some colleagues and get back to you.


configs/common/FSConfig.py
http://reviews.gem5.org/r/2301/#comment4746

I might be wrong here, but I think the InterruptLine is actually the 
interrupt line as seen by the APIC. It doesn't have anything to do with the 
actual CPU interrupt since the APIC sorts out the routing. If this is the case, 
get rid of that part of the comment to avoid confusion.


- Andreas Sandberg


On June 18, 2014, 3:42 a.m., Jiuyue Ma wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2301/
 ---
 
 (Updated June 18, 2014, 3:42 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 config: add ethernet support for x86 fullsystem
 
 This patch add a IGbE_e1000 ethernet device to x86 fs system. To make it work 
 properly, 
 following changes were also made to FSConfig.py:
 
  - add [mem_size-1(or 3GB for 4GB memory), 0x] to bridge's ranges 
 for kernel configured pci device memory,
access to pci address space will pass though membus to bridge
 
  - add IGbE_e1000 to x86_sys.pc.ethernet
 
  - connect x86_sys.pc.ethernet.pio/config/dma to x86_sys.iobus
 
  - swap bus_id of ISA/PCI in X86 IntelMPTable
In gem5 Pc::calcPciConfigAddr(), it required assert(bus==0), but linux 
 kernel cannot
config ethernet device connected to ISA bus, so we swap bus_id of ISA/PCI
 
 
 Diffs
 -
 
   configs/common/FSConfig.py b2850bdcec070052f3a0f5efa8bf748eca1f5d44 
 
 Diff: http://reviews.gem5.org/r/2301/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jiuyue Ma
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2167: mem: re-factor LRU code and add random replacement cache tags

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2167/#review5191
---


Overall, I think this patch looks good and the refactoring is a well-needed 
change to the way we implement replacement policies.

At a high level, I'd really appreciate it if you could split this patch into 
two patches so the refactoring becomes its own commit. That makes things like 
bisection much cleaner in the future.

I also found the naming of the different classes a bit confusing. I would 
suggest that you rename BaseLRU - BaseSetAssoc or something similar. After 
all, the common stuff that goes into this base class isn't really LRU specific. 
I also get a bit confused by the PseudoLRU class. When I think of pseudo-LRU, I 
generally think of things like tree-based LRU or the algorithm used in Nehalem. 
I'd suggest that you rename it to something with random in the name instead.

- Andreas Sandberg


On June 25, 2014, 1:26 a.m., Anthony Gutierrez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2167/
 ---
 
 (Updated June 25, 2014, 1:26 a.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10243:8f4099f6b9a2
 ---
 mem: re-factor LRU code and add random replacement cache tags
 
 this patch implements a new tags class that uses a random replacement policy.
 these tags prefer to evict invalid blocks first, if none are available a
 replacement candidate is chosen at random.
 
 this patch factors out the common code in the LRU class and creates a new
 abstract class: the BaseLRU class. any LRU tag or derivative of LRU must
 implement the methods related to the actual replacement policy. these are the
 following methods, which are pure virtual methods in BaseLRU:
 
 accessBlock()
 findVictim()
 insertBlock()
 invalidate()
 
 
 Diffs
 -
 
   src/mem/cache/base.cc cb4e86c177672fde6be7a409793c944e36353fc0 
   src/mem/cache/cache.cc cb4e86c177672fde6be7a409793c944e36353fc0 
   src/mem/cache/tags/SConscript cb4e86c177672fde6be7a409793c944e36353fc0 
   src/mem/cache/tags/Tags.py cb4e86c177672fde6be7a409793c944e36353fc0 
   src/mem/cache/tags/base_lru.hh PRE-CREATION 
   src/mem/cache/tags/base_lru.cc PRE-CREATION 
   src/mem/cache/tags/lru.hh cb4e86c177672fde6be7a409793c944e36353fc0 
   src/mem/cache/tags/lru.cc cb4e86c177672fde6be7a409793c944e36353fc0 
   src/mem/cache/tags/pseudo_lru.hh PRE-CREATION 
   src/mem/cache/tags/pseudo_lru.cc PRE-CREATION 
 
 Diff: http://reviews.gem5.org/r/2167/diff/
 
 
 Testing
 ---
 
 Regressions pass
 
 
 Thanks,
 
 Anthony Gutierrez
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2312: Multi-level page table class.

[Andreas Sandberg via gem5-dev]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2312/#review5188
---


In general, I like the idea of having a proper, architected, page table in 
SE-mode. Long-term, this could hopefully mean that we can get rid of many of 
the differences between SE- and FS-mode.

High-level comments:

* Write a proper commit message (see http://www.m5sim.org/Commit_Access). 
Specifically, include a short one-line summary and a longer description of what 
the patch does and why. There is currently now way of telling what this patch 
intends to accomplish.

* Don't set the execute bit on src/sim/SConscript.

* Split into an architecture-agnostic patch and an x86-specific patch.



src/mem/multi_level_page_table.hh
http://reviews.gem5.org/r/2312/#comment4744

Please focus on what the class does instead of where it is intended to be 
used. I'd write the description along these lines:

This class implements an in-memory page table that follows the x86 page 
table specification. This can used instead of the PageTable class in SE mode to 
allow CPU models (mainly the X86KvmCPU) to do a normal page table walk.

@see Link to suitable documentation





src/mem/multi_level_page_table_impl.hh
http://reviews.gem5.org/r/2312/#comment4737

This method is identical to map() with the exception of the parameter to 
setPTEFields. Please create a separate helper method that is used by moth map() 
and mapNotPresent().



src/mem/page_table.hh
http://reviews.gem5.org/r/2312/#comment4738

Since you need to make the PageTable class a base class, could you also 
take the opportunity to redesign it slightly to make it more obvious what's 
going on here?

Specifically, I'd like to see a design with these classes:

PageTableBase - Should declare the interface used by all page table 
implementations. Methods like map/remap/unmap/etc. should be purely virtual. 
This might be a good place to document what the arch-specific page tables are 
meant to do

SEPageTable (or some other sensible name) - Inherits from PageTableBase and 
implements the page table functionality currently in PageTable.

NoArchPageTable ( and arch-specific ones) - Inherit from PageTableBase.



src/sim/Process.py
http://reviews.gem5.org/r/2312/#comment4745

Re-phrase this, I hardly get what this means even after reading the rest of 
the patch. What your patch is doing is really to maintain an in-memory version 
of the page table in the architecture-specific format. Make sure that's clear 
from the description. You could use something like this:

maintain an in-memory version of the page table in an 
architecture-specific format



- Andreas Sandberg


On July 11, 2014, 5:57 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2312/
 ---
 
 (Updated July 11, 2014, 5:57 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10253:359daed0c723
 ---
 Multi-level page table class.
 This is part of the larger effort of supporting virtualized execution in SE 
 mode.
 
 
 Diffs
 -
 
   src/arch/alpha/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/arm/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/mips/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/power/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/sparc/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/x86/pagetable.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/x86/pagetable_walker.cc c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/x86/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/x86/system.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/mem/SConscript c625a3c51bac050879457e666dd83299a36d761b 
   src/mem/multi_level_page_table.hh PRE-CREATION 
   src/mem/multi_level_page_table.cc PRE-CREATION 
   src/mem/multi_level_page_table_impl.hh PRE-CREATION 
   src/mem/page_table.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/sim/Process.py c625a3c51bac050879457e666dd83299a36d761b 
   src/sim/SConscript c625a3c51bac050879457e666dd83299a36d761b 
   src/sim/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/sim/process.cc c625a3c51bac050879457e666dd83299a36d761b 
 
 Diff: http://reviews.gem5.org/r/2312/diff/
 
 
 Testing
 ---
 
 Regressions passed.
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 2312: Multi-level page table class.

[Andreas Sandberg via gem5-dev]

 On July 14, 2014, 7:36 p.m., Andreas Sandberg wrote:
  In general, I like the idea of having a proper, architected, page table in 
  SE-mode. Long-term, this could hopefully mean that we can get rid of many 
  of the differences between SE- and FS-mode.
  
  High-level comments:
  
  * Write a proper commit message (see http://www.m5sim.org/Commit_Access). 
  Specifically, include a short one-line summary and a longer description of 
  what the patch does and why. There is currently now way of telling what 
  this patch intends to accomplish.
  
  * Don't set the execute bit on src/sim/SConscript.
  
  * Split into an architecture-agnostic patch and an x86-specific patch.
 

Also, please document what the architecture-specific page tables are supposed 
to do. A good place could be the PageTableBase class.


- Andreas


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2312/#review5188
---


On July 11, 2014, 5:57 p.m., Alexandru Dutu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.gem5.org/r/2312/
 ---
 
 (Updated July 11, 2014, 5:57 p.m.)
 
 
 Review request for Default.
 
 
 Repository: gem5
 
 
 Description
 ---
 
 Changeset 10253:359daed0c723
 ---
 Multi-level page table class.
 This is part of the larger effort of supporting virtualized execution in SE 
 mode.
 
 
 Diffs
 -
 
   src/arch/alpha/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/arm/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/mips/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/power/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/sparc/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/x86/pagetable.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/x86/pagetable_walker.cc c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/x86/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/arch/x86/system.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/mem/SConscript c625a3c51bac050879457e666dd83299a36d761b 
   src/mem/multi_level_page_table.hh PRE-CREATION 
   src/mem/multi_level_page_table.cc PRE-CREATION 
   src/mem/multi_level_page_table_impl.hh PRE-CREATION 
   src/mem/page_table.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/sim/Process.py c625a3c51bac050879457e666dd83299a36d761b 
   src/sim/SConscript c625a3c51bac050879457e666dd83299a36d761b 
   src/sim/process.hh c625a3c51bac050879457e666dd83299a36d761b 
   src/sim/process.cc c625a3c51bac050879457e666dd83299a36d761b 
 
 Diff: http://reviews.gem5.org/r/2312/diff/
 
 
 Testing
 ---
 
 Regressions passed.
 
 
 Thanks,
 
 Alexandru Dutu
 


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] KvmCPU Behaviour

[Andreas Sandberg via gem5-dev]

Hi Alex,

I had a quick look trying to reproduce the problem, but didn't manage to
do so. However, I remember this being a problem when I initially
developed the KVM stuff. I don't think it is related to changeset
5c2ecad1a3c9, instead I'd suspect it's something that happened before that.

In general, the KVM CPU model does not like high-frequency events. The
reason is that the CPU needs to simulate timing to make timer interrupts
happen correctly. The way this is done is by assuming that there is a
constant time conversion factor (hostFactor) between the host and the guest.

The problem here is that timing is a bit tricky. There is actually no
way (we could use performance counters, but the API is a bit broken so
it doesn't work well in practice) to setup a timer that only measures
time spent in KVM. Instead, we setup a normal POSIX timer that measures
wall-clock time.

In summary, I can think of three possible error sources:
 * Your system might be taking longer than usual to enter into KVM,
which makes it more sensitive to high-frequency events.

 * The time scaling factor (hostFactor) might be off.

 * Weird quirkiness in the AMD virtualization driver. I haven't tested
the KVM stuff on AMD hardware, so YMMV. A more recent kernel might help.


I usually calculate the scaling factor like this:

hostFactor = c * f_host / f_guest

Where f_host is the frequency of the host, f_guest the frequency of the
simulated CPU, and c a correction factor (usually close to 1, I used
0.95 for the experiments in my thesis [1]).

If setting the host factor doesn't help, I'd suggest that you disable
the DRAM model when running in KVM as a workaround. Also, I'd highly
appreciate if you could try to bisect this issue. There should be
support for bisection in Mercurial which lets you select two revisions
(a good and a bad) and then tries to pinpoint the exact revision where
the bug was introduced.

//Andreas

[1] http://urn.kb.se/resolve?urn=urn:se:uu:diva-220652

On 12/06/14 20:59, Alexandru Duţu via gem5-dev wrote:

Hi everyone,

It seems that the KVM enter-exit cycles with 0 instructions executed are
caused by the events scheduled from the DRAMCtrl object. The issues is that
these events have a very short period which is not sufficient for KvmCPU to
even start execution.

I am not familiar with DRAMCtrl memory, it seems that SimpleMemory does not
have this issue with KvmCPU.

Best regards,


On Wed, Jun 11, 2014 at 5:05 PM, Alexandru Duţu alex.d...@gmail.com wrote:


Hi all,

I am trying to understand the current behaviour of KvmCPU. Previous to the
following changeset

http://repo.gem5.org/gem5/rev/2360411a16be

after every kvmRun the status was switched to runningService, a new event
was scheduled to the event queue and so with new tick handleKvmExit will be
called.

However, with the latest changesets on kvm (
http://repo.gem5.org/gem5/rev/5c2ecad1a3c9) there are a lot of KVM
enter-exit cycles with 0 instructions executed. It seems that the vCPU gets
interrupted by the timer a lot more, which causes kvm to exit without
executing anything.

An explanation for this behaviour is highly appreciated, is there a need
for timer interrupts that are not allowing the vCPU to actually execute? Or
this might be a side effect of the differences in timers across different
x86 vendors (I am running on an AMD system)?

Best regards,
--
Alex








-- 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.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in 
England  Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England  Wales, Company No:  2548782

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Proposal to untemplate the o3 CPU

[Andreas Sandberg via gem5-dev]

Hi Mitch,

In general, I like the idea of removing some of the pointless/awkward 
templates we have in gem5. I would definitely support moving in this 
direction. However, I really dislike the idea of reviewing a 32k line 
patch. Reviewing such a patch would be a headache and I suspect RB would 
just fall over. Would it be possible to split this change into a series 
of smaller patches?


For example, you could split it into one patch per functional unit and a 
final patch that does some cleaning up. You could probably just 'fake' 
new un-templated class names as typedefs in the relevant header files.


//Andreas


On 2014-05-13 18:23, Mitch Hayenga via gem5-dev wrote:

Hi All,

Recently I have written a patch that removes templating from the o3 cpu.
  In general templating in o3 makes the code significantly more verbose,
adds compile time overheads, and doesn't actually benefit performance.  The
templating is largely pointless as 1) there aren't multiple versions of
fetch, rename, etc to make the  compile time Impl pattern worth doing 2)
Modern CPUs have indirect branch predictors that hide the penalties that
the templating was trying to mask.

*I was wondering what peoples feelings were on a patch of this sort? * It
is a quite large modification (~35k line patch file, changes almost all
localized to the o3 directory).  Many of the lines are simply because the
impl header files were changed to source files.

Here are a few benefits of the patch

- Cleaner, less verbose code.
- Due to the current templating/DynInst interaction, gem5 often requires
rebuilding the function execution signatures (o3_cpu_exec.o) when a
modification is made to the o3 cpu.  This patch eliminates having to
rebuild the execution signatures on o3 changes.
- Marginally better compile/run times.
- Moved base_dyn_inst_impl.hh into o3, it's too dependent on o3 as is.
 No other cpu does/should inherit from it anyway.
- Made the checker directly templated on the execution context (DynInst)
instead of an Impl like o3.  Seems like it was coded dependently on o3.


Here are some performance results for gem5.fast on GCC 4.9 and CLANG on
twolf from spec2k.

*Binary Size*
CLANG: 1.1% smaller without templating
GCC: Difference is negligible 0.0001%


*CLANG Compile Time (single threaded, no turboboost, two runs)*
*Templated*
 real21m32.240s
 user20m20.019s
 sys 1m6.721s

 real21m29.963s
 user20m17.016s
 sys 1m7.108s

*Untempated:*
 real21m24.396s
 user20m13.158s
 sys 1m5.798s

 real21m23.177s
 user20m11.911s
 sys 1m5.843s


*GCC Compile Time (-j8, did not disable turboboost)*
*Templated*
 real11m35.848s
 user67m20.828s
 sys 2m2.292s

*Untemplated:*
 real11m42.167s
 user67m7.572s
 sys 2m2.056s


*CLANG Run Time (Spec2k twolf)*
*Templated*
 Run 1) 1187.63
 Run 2) 1167.50
 Run 3) 1172.06

*Untemplated*
 Run 1) 1142.29
 Run 2) 1154.49
 Run 3) 1165.53


*GCC Run Time (Spec2k twolf, did not disable turboboost)*
*Templated*
 Run 1) 12m20.528s
*Untemplated*
Run 1) 12m19.700s



Any thoughts on eventually merging this?
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev


___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev