[gem5-dev] Change in gem5/gem5[develop]: util: Implement PIC assembly for the aarch64.

2020-04-08 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/27212 )


Change subject: util: Implement PIC assembly for the aarch64.
..

util: Implement PIC assembly for the aarch64.

When accessing m5_mem and building PIC code, we need to get the address
of m5_mem out of the global offset table, and then load the value from
there. If we try to load from m5_mem directly, the assembled code has a
relocation type the linker can't handle when building a shared object.

Change-Id: Ieb19c3d17c37ef810559ee24b68886b18ddcc869
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/27212
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M util/m5/src/m5op_arm_A64.S
1 file changed, 11 insertions(+), 1 deletion(-)

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



diff --git a/util/m5/src/m5op_arm_A64.S b/util/m5/src/m5op_arm_A64.S
index c0224e2..6f9f038 100644
--- a/util/m5/src/m5op_arm_A64.S
+++ b/util/m5/src/m5op_arm_A64.S
@@ -44,7 +44,17 @@
 .macro m5op_func, name, func
 .globl \name
 \name:
-ldr x9, m5_mem
+// Load the value of m5_mem into x9...
+#if defined(M5OP_PIC)
+// using the global offset table.
+adrp x9, :got:m5_mem
+ldr x9, [ x9, #:got_lo12:m5_mem ]
+ldr x9, [ x9 ]
+#else
+// normally.
+adrp x9, m5_mem
+ldr x9, [ x9, #:lo12:m5_mem ]
+#endif
 movz x10, #(\func << 8)
 ldr x0, [ x9, x10 ]
 ret

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ieb19c3d17c37ef810559ee24b68886b18ddcc869
Gerrit-Change-Number: 27212
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Earl Ou 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Yu-hsin Wang 
Gerrit-Reviewer: kokoro 
Gerrit-CC: Ciro Santilli 
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] m5 utility patch series

2020-04-08 Thread Gabe Black
On Wed, Apr 8, 2020 at 5:39 PM Jason Lowe-Power  wrote:

> Hey Gabe,
>
> I still don't feel like this gives me enough context to do a good job
> reviewing these changesets. For instance, before, I would go to util/m5 and
> run `make -f Makefile.` to create the m5 utility. What is the new
> process? It seems to be done with scons? Where is the binary created when
> you build it? What binaries can be built? There's *a lot* changing here,
> and I'd like to understand things from a high level before digging into the
> details of the code.
>
> To be more specific, what I would like to see is 1) a list of the major
> changes, and 2) what the new interface is for each of the changes. For
> instance:
> - These changesets update the m5 utility to be built with scons instead of
> make. Now, you run `` to build m5.
>

scons build/{version you want}, ie scons build/aarch64, or scons build/x86

The build outputs are in build/{version}/out, and are called the same
things as before. If you have all the cross compilers set up (what you'd
get with my build_cross_gcc.py script) you can just run "scons build/" to
get all versions of the utility and associated files.

There's *slightly* more to it since you can still select what cross
compiler you want for a particular version, and later in the series I add
unit tests which you can also build, but that's the main gist. It's not
really that much of a change at the top user level, and I did try to
explain everything in the commit messages.


> - These changesets now allow you to select how m5 ops are called at
> runtime. You can do that by .
>

See the link below.


>
> With this context, I can then review the code to make sure it accomplishes
> the goals in a reasonable way (which I think is the goal of code review).
>
> I found the m5 testing information you posted (
> https://docs.google.com/document/d/1cmHzhB1p3Kccc-1f0UJNbvuNced5splL8ApVWcZ9094/edit#).
> This is quite helpful to understand the testing changes! However, it still
> leaves out details about the other changes to the user-facing interface.
>

That's a good document to read for the later changes related to testing,
but there is also information here:

https://docs.google.com/document/d/1I7Jaj5_HssMSlvf-QKL1IsuBvV0Bm6a64At5vjhMIhU/edit#heading=h.z4lygqyg3e78

specifically the small section called "m5 utility", but the document has
other background information which might be useful. That is the primary
change as far as how the m5 utility is used, and the rest is fixing things
that have bitrotted, consistent support across ISAs, scalable build
infrastructure, tests, etc.


>
> Generally, I (personally) would appreciate some help reviewing huge sets
> of patches like this. I often only have 15-30 minutes at a time to review
> code, and when it takes a significant amount of time to track down the
> context to a changeset, it makes it much more difficult for me to review it.
>

Of course. There are folks out there that have been good about actually
replying to reviews (like yourself) and so I tend to put them on patches,
but I also try to add a lot of folks so it doesn't all fall to one or two
people. I've tried to put together documents for my larger efforts, but I
think it's hard to find time to read documents just like it's hard to find
time to review changes...


>
> Specifically, we're encouraging the community to create jira issues to
> link everything in one place. This would be *incredibly* helpful
> to the reviewers. In the Jira issue, you can put the design documentation
> (like the one linked above) as well as links to the changesets. Also, it
> would be great to have a link from the changeset description to the Jira. I
> think it's probably too strong to *require* jira links for all changesets,
> but IMO it should be rare that there's not a jira link in a changeset.
> We've added an "Issue-on:" tag for this use case.
>

Yeah, I think having a short, easy to remember tag to use in commit
messages is a very good idea. I'm pretty lazy, and so when I'm writing up a
CL, I don't want to have to go dig up the exact spelling, etc, for the tag
to record a JIRA issue. That's less of a concern if it's just for people,
but if we add integration with gerrit somehow where it posts CLs to
associated issues when they're checked in, shows the issue tag as a link in
the gerrit UI, etc., matching exactly will likely matter.


>
> Also, did you give any thoughts to my proposal to change the name from
> `m5` to `gem5`? Or maybe there's a better name :).
>

Yes, quoting from earlier in the thread:

"""
That's not a bad idea, although there are a lot of compatibility issues
that make me want to keep that separate. For instance, if we change all the
names for the symbols for calling the ops from m5_* to gem5_*, that would
break existing consumers of the library. It would be easy to fix them with
simple find/replace assuming they can be rebuilt and is probably worth
doing, but it would be a good idea to draw people's 

Re: [gem5-dev] Proposal: Replace license headers with SPDX identifiers

2020-04-08 Thread Bobby Bruce
I think this is a great idea. My only question is that some of our code has
a slightly extended BSD license. In particular I'm thinking about code ARM
is involved in (E.g. :
https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/arch/arm/decoder.cc
). Is the solution here something like?:

```
 Copyright (c) 2012-2014,2018 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.

SPDX-License-Identifier: BSD-3-Clause

```

Software copyrights, patents, licencing, etc have always confused the heck
out of me, so sorry if it's a newbie question :).

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

web: https://www.bobbybruce.net


On Wed, Apr 8, 2020 at 4:49 PM Jason Lowe-Power  wrote:

> Hi all,
>
> I propose to replace all of the BSD text in the headers of all files with
> the following:
>
> /*
>  * Copyright (c)  University of California.>
>  * All rights reserved.
>  *
>  * SPDX-License-Identifier: BSD-3-Clause
>  */
>
> We will also update the LICENSE file to say the following:
>
> ```
> Note: Individual files contain the following tag instead of the full
> license text.
>
> SPDX-License-Identifier: BSD-3-Clause
>
> This enables machine processing of license information based on the SPDX
> License Identifiers that are here available: http://spdx.org/licenses/
> ```
>
> See https://spdx.org/ids-how for more information about SPDX license IDs.
>
> Before we go through and make a giant changeset (or changesets) to do this,
> I'd like to make sure this is something that will work for the community.
>
> Let me know what you think!
>
> Cheers,
> Jason
> ___
> 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

Re: [gem5-dev] m5 utility patch series

2020-04-08 Thread Jason Lowe-Power
Hey Gabe,

I still don't feel like this gives me enough context to do a good job
reviewing these changesets. For instance, before, I would go to util/m5 and
run `make -f Makefile.` to create the m5 utility. What is the new
process? It seems to be done with scons? Where is the binary created when
you build it? What binaries can be built? There's *a lot* changing here,
and I'd like to understand things from a high level before digging into the
details of the code.

To be more specific, what I would like to see is 1) a list of the major
changes, and 2) what the new interface is for each of the changes. For
instance:
- These changesets update the m5 utility to be built with scons instead of
make. Now, you run `` to build m5.
- These changesets now allow you to select how m5 ops are called at
runtime. You can do that by .

With this context, I can then review the code to make sure it accomplishes
the goals in a reasonable way (which I think is the goal of code review).

I found the m5 testing information you posted (
https://docs.google.com/document/d/1cmHzhB1p3Kccc-1f0UJNbvuNced5splL8ApVWcZ9094/edit#).
This is quite helpful to understand the testing changes! However, it still
leaves out details about the other changes to the user-facing interface.

Generally, I (personally) would appreciate some help reviewing huge sets of
patches like this. I often only have 15-30 minutes at a time to review
code, and when it takes a significant amount of time to track down the
context to a changeset, it makes it much more difficult for me to review it.

Specifically, we're encouraging the community to create jira issues to link
everything in one place. This would be *incredibly* helpful
to the reviewers. In the Jira issue, you can put the design documentation
(like the one linked above) as well as links to the changesets. Also, it
would be great to have a link from the changeset description to the Jira. I
think it's probably too strong to *require* jira links for all changesets,
but IMO it should be rare that there's not a jira link in a changeset.
We've added an "Issue-on:" tag for this use case.

Also, did you give any thoughts to my proposal to change the name from `m5`
to `gem5`? Or maybe there's a better name :).

I want to emphasize that I (and the community) appreciate all of the work
that you have done and are doing to improve the gem5 codebase. Sometimes
it's just a little hard to keep up with your pace :). Thanks for all of the
hard work!

Thanks,
Jason

On Tue, Apr 7, 2020 at 4:01 PM Gabe Black  wrote:

> Hi Jason. As I've said somewhere before (maybe this thread?), that is
> mostly not changing. There will still be an m5 library for use in other
> programs, a java and lua wrapper, an m5 utility which takes basically the
> same commands (with some cruft removed), etc. The changes are that I'm
> refactoring and reimplementing the guts of the code and the build process
> so that it's scalable, features are implemented consistently across ISAs,
> bit rotted code is fixed, there are tests, etc.
>
> The main user visible change I've made is that the way the m5 ops are
> called run time selectable rather than build time selectable, as described
> in the document I referenced. I've also made it possible to use the utility
> to call m5 ops through ARM semihosting so they can be used with fast model
> CPUs. The build process is different now too, although that is more
> utilitarian. I've gotten rid of many of the previous setup's shortcomings
> which I won't enumerate here, but it still builds the same things more or
> less, plus the tests I'm adding.
>
> Gabe
>
> On Tue, Apr 7, 2020 at 7:55 AM Jason Lowe-Power 
> wrote:
>
>> Hey Gabe,
>>
>> I'd love to review your patches that you've posted improving the m5
>> utility, but I don't feel that I can review them well without understanding
>> what the end goal is. If you could provide some documentation on how you
>> see the m5 utility being used, then I can try to carve out some time to
>> review your code.
>>
>> Thanks,
>> Jason
>>
>> On Tue, Mar 31, 2020 at 3:28 PM Jason Lowe-Power 
>> wrote:
>>
>>> Oh, one more comment...
>>>
>>> Do you think it's worth changing the name to "gem5" instead of "m5".
>>> Since we're making big changes, it seems like now might be right time.
>>>
>>> Cheers,
>>> Jason
>>>
>>> On Tue, Mar 31, 2020 at 3:10 PM Jason Lowe-Power 
>>> wrote:
>>>
 Hey Gabe,

 First of all, thanks for this cleanup. We've needed to update this code
 for a long time!

 Do you have a pointer to what would be "new" documentation on the m5
 ops tools? I was briefly going through your changes and it's not clear how
 you're envisioning people using this library now. For instance, I'd like to
 understand:
 - How do I build the m5 binary for full system mode?
 - How do I link my application to the m5 "library" in SE mode?
 - How do I link my application to the m5 "library" in FS mode?

 Before, this wasn't 

[gem5-dev] [UPDATE] gem5 Workshop with ISCA 2020

2020-04-08 Thread Jason Lowe-Power
Hi all,

As you all likely already know, ISCA is going to be taking place virtually
due to the COVID-19 pandemic. See https://iscaconf.org/isca2020/ for more
information.

On the plus side, we believe this will make the gem5 workshop more
inclusive! Rather than limiting attendees to those that can travel to
Valencia, anyone will be able to participate!

We are making the following changes to the original call for abstracts
(original call below):
1) The deadline for submitting an abstract is now May 1st. You can still
use the following link to submit an abstract:
https://forms.gle/UnpFXRvpLEFKJBb46

2) For accepted abstracts, you will be asked to produce a short video and a
blog post for the gem5 blog. In the week leading up to the workshop, we
will advertise all of the accepted works.

3)  We will hold a synchronous virtual Q session (or sessions) during the
week of June 1st (the original time when ISCA was scheduled). During
these session(s), we will ask the authors of the accepted works to attend
to encourage discussion with the broader gem5 community. Specific times for
these will be announced soon. We will work with the community to choose a
time or times that are inclusive to those in different time zones.

Update on the gem5 Tutorial: Similarly for the gem5 tutorial, we will post
videos before the week of June 1st and hold session(s) for Q the week of
June 1st.

Thanks for being flexible! We look forward to your submissions!

Cheers,
Jason

On Tue, Mar 10, 2020 at 4:41 PM Jason Lowe-Power 
wrote:

> Hi all,
>
> [Apologies if you receive multiple notices!]
>
> In conjunction with ISCA 2020, we will be holding a gem5 Tutorial and
> Workshop on May 30th in Valencia, Spain. In the morning, we will be running
> a Learning gem5 Tutorial, and in the afternoon we will have a gem5 users’
> workshop. The workshop will begin with a keynote detailing the recent
> changes in gem5 and announcing the first stable version of gem5, gem5-20.
> The workshop will also include a number of community-contributed talks. See
> below for details on how to submit an abstract for a talk.
>
> Details can be found on the gem5 website:
> http://www.gem5.org/events/isca-2020
>
> Call for presentations
> 
> The gem5 community is excited to announce the 3rd gem5 Users’ workshop
> held in conjunction with ISCA 2020 in Valencia, Spain. The goal of the
> workshop is to provide a forum to discuss what is going on in the
> community, how we can best leverage each other’s contributions, and how we
> can continue to make gem5 a successful community-supported simulation
> framework. The workshop will be a half day in the afternoon of May 30.
>
> The workshop will follow a half-day “Learning gem5” tutorial.
>
> The workshop will include a keynote presentation “RE-gem5 and gem5-20:
> Past, Present, and Future of the gem5 Community Infrastructure.”
>
> We invite the gem5 community to submit abstracts (1-2 paragraphs) for
> short presentations. The scope of this workshop is broadly the gem5 user
> and development community. Topics of interest include:
> - New features added to gem5
> - New models added to gem5
> - Extensions and integrations with other simulators
> - Experience using gem5
> - Validation of gem5 models
>
> We encourage accepted presentations to post a full paper to arXiv or other
> archival repository in order to give other users a citable source for your
> contribution. These sources may be cited in future gem5 release notes.
>
> Please submit your abstracts via this Google Form. The deadline to submit
> an abstract is April 10th and we will send notifications by April 14th
> before the ISCA early registration deadline (April 16th). Due to the close
> proximity to other deadlines/conferences and the early registration
> deadline, there will not be any extension.
>
> Form for abstract submission: https://forms.gle/UnpFXRvpLEFKJBb46
>
> More information can be found on the gem5 website:
> https://www.gem5.org/events/isca-2020
>
> Looking forward to seeing you in Valencia!
>
>
> PS: This all assumes that ISCA will take place at the end of May. We are
> paying close attention to the current situation with COVID-19 and will
> update the website with any new information.
>
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: mem-ruby: Replace SLICC queueMemory calls with enqueue

2020-04-08 Thread Matthew Poremba (Gerrit)
Matthew Poremba has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/27427 )


Change subject: mem-ruby: Replace SLICC queueMemory calls with enqueue
..

mem-ruby: Replace SLICC queueMemory calls with enqueue

Calls to queueMemoryRead and queueMemoryWrite do not consider the size
of the queue between ruby directories and DRAMCtrl which causes infinite
buffering in the queued port between the two. This adds a MessageBuffer
in between which uses enqueues in SLICC and is therefore size checked
before any SLICC transaction pushing to the buffer can occur, removing
the infinite buffering between the two.

Change-Id: Iedb9070844e4f6c8532a9c914d126105ec98d0bc
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/27427
Tested-by: Gem5 Cloud Project GCB service account  
<345032938...@cloudbuild.gserviceaccount.com>

Tested-by: kokoro 
Reviewed-by: Bradford Beckmann 
Reviewed-by: Jason Lowe-Power 
Reviewed-by: Matt Sinclair 
Maintainer: Bradford Beckmann 
---
M configs/learning_gem5/part3/msi_caches.py
M configs/learning_gem5/part3/ruby_caches_MI_example.py
M configs/ruby/GPU_RfO.py
M configs/ruby/GPU_VIPER.py
M configs/ruby/GPU_VIPER_Baseline.py
M configs/ruby/GPU_VIPER_Region.py
M configs/ruby/MESI_Two_Level.py
M configs/ruby/MI_example.py
M configs/ruby/MOESI_CMP_directory.py
M configs/ruby/MOESI_CMP_token.py
M configs/ruby/MOESI_hammer.py
M src/learning_gem5/part3/MSI-dir.sm
M src/mem/ruby/protocol/MESI_Two_Level-dir.sm
M src/mem/ruby/protocol/MI_example-dir.sm
M src/mem/ruby/protocol/MOESI_AMD_Base-Region-dir.sm
M src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
M src/mem/ruby/protocol/MOESI_AMD_Base-probeFilter.sm
M src/mem/ruby/protocol/MOESI_CMP_directory-dir.sm
M src/mem/ruby/protocol/MOESI_CMP_token-dir.sm
M src/mem/ruby/protocol/MOESI_hammer-dir.sm
M src/mem/ruby/protocol/RubySlicc_Defines.sm
M src/mem/ruby/protocol/RubySlicc_MemControl.sm
M src/mem/ruby/slicc_interface/AbstractController.cc
M src/mem/ruby/slicc_interface/AbstractController.hh
M src/mem/slicc/symbols/StateMachine.py
25 files changed, 445 insertions(+), 172 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Bradford Beckmann: Looks good to me, approved; Looks good to me, approved
  Matt Sinclair: Looks good to me, approved
  kokoro: Regressions pass
  Gem5 Cloud Project GCB service account: Regressions pass



diff --git a/configs/learning_gem5/part3/msi_caches.py  
b/configs/learning_gem5/part3/msi_caches.py

index aeacd75..f899426 100644
--- a/configs/learning_gem5/part3/msi_caches.py
+++ b/configs/learning_gem5/part3/msi_caches.py
@@ -214,10 +214,11 @@
 self.forwardToCache = MessageBuffer(ordered = True)
 self.forwardToCache.master = ruby_system.network.slave

-# This is another special message buffer. It is used to send  
replies

-# from memory back to the controller. Any messages received on the
-# memory port (see self.memory above) will be directed to this
-# message buffer.
+# These are other special message buffers. They are used to send
+# requests to memory and responses from memory back to the  
controller.

+# Any messages sent or received on the memory port (see self.memory
+# above) will be directed through these message buffers.
+self.requestToMemory = MessageBuffer()
 self.responseFromMemory = MessageBuffer()

 class MyNetwork(SimpleNetwork):
diff --git a/configs/learning_gem5/part3/ruby_caches_MI_example.py  
b/configs/learning_gem5/part3/ruby_caches_MI_example.py

index 0a7e0d4..29b66a6 100644
--- a/configs/learning_gem5/part3/ruby_caches_MI_example.py
+++ b/configs/learning_gem5/part3/ruby_caches_MI_example.py
@@ -204,6 +204,7 @@
 self.dmaResponseFromDir.master = ruby_system.network.slave
 self.forwardFromDir = MessageBuffer()
 self.forwardFromDir.master = ruby_system.network.slave
+self.requestToMemory = MessageBuffer()
 self.responseFromMemory = MessageBuffer()

 class MyNetwork(SimpleNetwork):
diff --git a/configs/ruby/GPU_RfO.py b/configs/ruby/GPU_RfO.py
index 449c169..cf2fdbd 100644
--- a/configs/ruby/GPU_RfO.py
+++ b/configs/ruby/GPU_RfO.py
@@ -499,6 +499,7 @@

 dir_cntrl.triggerQueue = MessageBuffer(ordered = True)
 dir_cntrl.L3triggerQueue = MessageBuffer(ordered = True)
+dir_cntrl.requestToMemory = MessageBuffer()
 dir_cntrl.responseFromMemory = MessageBuffer()

 exec("system.dir_cntrl%d = dir_cntrl" % i)
diff --git a/configs/ruby/GPU_VIPER.py b/configs/ruby/GPU_VIPER.py
index 2c36426..71238ae 100644
--- a/configs/ruby/GPU_VIPER.py
+++ b/configs/ruby/GPU_VIPER.py
@@ -453,6 +453,7 @@

 dir_cntrl.triggerQueue = MessageBuffer(ordered = True)
 dir_cntrl.L3triggerQueue = MessageBuffer(ordered = True)
+dir_cntrl.requestToMemory = MessageBuffer()
 dir_cntrl.responseFromMemory = 

[gem5-dev] Proposal: Replace license headers with SPDX identifiers

2020-04-08 Thread Jason Lowe-Power
Hi all,

I propose to replace all of the BSD text in the headers of all files with
the following:

/*
 * Copyright (c) 
 * All rights reserved.
 *
 * SPDX-License-Identifier: BSD-3-Clause
 */

We will also update the LICENSE file to say the following:

```
Note: Individual files contain the following tag instead of the full
license text.

SPDX-License-Identifier: BSD-3-Clause

This enables machine processing of license information based on the SPDX
License Identifiers that are here available: http://spdx.org/licenses/
```

See https://spdx.org/ids-how for more information about SPDX license IDs.

Before we go through and make a giant changeset (or changesets) to do this,
I'd like to make sure this is something that will work for the community.

Let me know what you think!

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

[gem5-dev] Change in gem5/gem5[develop]: configs: add option for memory channel intlv.

2020-04-08 Thread Adrian Herrera (Gerrit)
Adrian Herrera has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/27629 )



Change subject: configs: add option for memory channel intlv.
..

configs: add option for memory channel intlv.

Current memory channel interleave is hard-coded to be maximum between 128
and system's cache line size. Making this value configurable enables use
cases with DMA masters accessing at higher than 128 block granularity.

Change-Id: I8123fa307efd3fd8f16c815ee74a84844bb51edb
---
M configs/common/MemConfig.py
M configs/common/Options.py
2 files changed, 6 insertions(+), 3 deletions(-)



diff --git a/configs/common/MemConfig.py b/configs/common/MemConfig.py
index d1cc655..9443520 100644
--- a/configs/common/MemConfig.py
+++ b/configs/common/MemConfig.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2013, 2017 ARM Limited
+# Copyright (c) 2013, 2017, 2020 ARM Limited
 # All rights reserved.
 #
 # The license below extends only to copyright in the software and shall
@@ -109,6 +109,7 @@
 opt_elastic_trace_en = getattr(options, "elastic_trace_en", False)
 opt_mem_ranks = getattr(options, "mem_ranks", None)
 opt_dram_powerdown = getattr(options, "enable_dram_powerdown", None)
+opt_mem_channels_intlv = getattr(options, "mem_channels_intlv", 128)

 if opt_mem_type == "HMC_2500_1x32":
 HMChost = HMC.config_hmc_host_ctrl(options, system)
@@ -154,7 +155,7 @@
 # byte granularity, or cache line granularity if larger than 128
 # byte. This value is based on the locality seen across a large
 # range of workloads.
-intlv_size = max(128, system.cache_line_size.value)
+intlv_size = max(opt_mem_channels_intlv, system.cache_line_size.value)

 # For every range (most systems will only have one), create an
 # array of controllers and set their parameters to match their
diff --git a/configs/common/Options.py b/configs/common/Options.py
index 6c92f36..6d0c6c2 100644
--- a/configs/common/Options.py
+++ b/configs/common/Options.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2013-2019 ARM Limited
+# Copyright (c) 2013-2020 ARM Limited
 # All rights reserved.
 #
 # The license below extends only to copyright in the software and shall
@@ -110,6 +110,8 @@
   help="Specify the physical memory size (single  
memory)")

 parser.add_option("--enable-dram-powerdown", action="store_true",
help="Enable low-power states in DRAMCtrl")
+parser.add_option("--mem-channels-intlv", type="int",
+  help="Memory channels interleave")


 parser.add_option("--memchecker", action="store_true")

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I8123fa307efd3fd8f16c815ee74a84844bb51edb
Gerrit-Change-Number: 27629
Gerrit-PatchSet: 1
Gerrit-Owner: Adrian Herrera 
Gerrit-MessageType: newchange
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: arch-arm, dev-arm: Autogen PSCI node in DTB

2020-04-08 Thread Giacomo Travaglini (Gerrit)
Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/27387 )


Change subject: arch-arm, dev-arm: Autogen PSCI node in DTB
..

arch-arm, dev-arm: Autogen PSCI node in DTB

This is controlled via the python only _have_psci parameter
This flag will be checked when auto-generarting a PSCI node. A client
(e.g Linux) would then be able to know if it can use the PSCI APIs

Change-Id: I16c4a67bd358eca3dfff6c98ab8a602a31e1c751
Signed-off-by: Giacomo Travaglini 
Reviewed-by: Nikos Nikoleris 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/27387
Tested-by: kokoro 
---
M src/arch/arm/ArmSystem.py
M src/dev/arm/RealView.py
2 files changed, 32 insertions(+), 4 deletions(-)

Approvals:
  Nikos Nikoleris: Looks good to me, approved
  Giacomo Travaglini: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/ArmSystem.py b/src/arch/arm/ArmSystem.py
index 07fdad6..6555ea9 100644
--- a/src/arch/arm/ArmSystem.py
+++ b/src/arch/arm/ArmSystem.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2009, 2012-2013, 2015-2019 ARM Limited
+# Copyright (c) 2009, 2012-2013, 2015-2020 ARM Limited
 # All rights reserved.
 #
 # The license below extends only to copyright in the software and shall
@@ -83,6 +83,12 @@
 "Base of the 64KiB PA range used for memory-mapped m5ops. Set to  
0 "

 "to disable.")

+# Set to true if simulation provides a PSCI implementation
+# This flag will be checked when auto-generating
+# a PSCI node. A client (e.g Linux) would then be able to
+# know if it can use the PSCI APIs
+_have_psci = False
+
 def generateDtb(self, filename):
 """
 Autogenerate DTB. Arguments are the folder where the DTB
diff --git a/src/dev/arm/RealView.py b/src/dev/arm/RealView.py
index 4cd6258..0a26a94 100644
--- a/src/dev/arm/RealView.py
+++ b/src/dev/arm/RealView.py
@@ -645,9 +645,13 @@
 yield node

 def annotateCpuDeviceNode(self, cpu, state):
-cpu.append(FdtPropertyStrings("enable-method", "spin-table"))
-cpu.append(FdtPropertyWords("cpu-release-addr", \
-state.addrCells(0x8000fff8)))
+system = self.system.unproxy(self)
+if system._have_psci:
+cpu.append(FdtPropertyStrings('enable-method', 'psci'))
+else:
+cpu.append(FdtPropertyStrings("enable-method", "spin-table"))
+cpu.append(FdtPropertyWords("cpu-release-addr", \
+state.addrCells(0x8000fff8)))

 class VExpress_EMM(RealView):
 _mem_regions = [ AddrRange('2GB', size='2GB') ]
@@ -1128,6 +1132,24 @@
 node.append(FdtPropertyWords("arm,hbi", [0x0]))
 node.append(FdtPropertyWords("arm,vexpress,site", [0xf]))

+system = self.system.unproxy(self)
+if system._have_psci:
+# PSCI functions exposed to the kernel
+if not system.have_security:
+raise AssertionError("PSCI requires EL3 (have_security)")
+
+psci_node = FdtNode('psci')
+psci_node.appendCompatible(['arm,psci-1.0', 'arm,psci-0.2',
+'arm,psci'])
+method = 'smc'
+psci_node.append(FdtPropertyStrings('method', method))
+psci_node.append(FdtPropertyWords('cpu_suspend', 0xc401))
+psci_node.append(FdtPropertyWords('cpu_off', 0x8402))
+psci_node.append(FdtPropertyWords('cpu_on', 0xc403))
+psci_node.append(FdtPropertyWords('sys_poweroff', 0x8408))
+psci_node.append(FdtPropertyWords('sys_reset', 0x8409))
+node.append(psci_node)
+
 yield node

 class VExpress_GEM5_V1_Base(VExpress_GEM5_Base):

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I16c4a67bd358eca3dfff6c98ab8a602a31e1c751
Gerrit-Change-Number: 27387
Gerrit-PatchSet: 2
Gerrit-Owner: Giacomo Travaglini 
Gerrit-Reviewer: Adrian Herrera 
Gerrit-Reviewer: Ciro Santilli 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Nikos Nikoleris 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: arch-sparc: MAP_32BIT does not exist on solaris

2020-04-08 Thread Giacomo Travaglini (Gerrit)

Hello Richard Cooper,

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

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

to review the following change.


Change subject: arch-sparc: MAP_32BIT does not exist on solaris
..

arch-sparc: MAP_32BIT does not exist on solaris

Judging by the mmap documentation for solaris:

https://docs.oracle.com/cd/E88353_01/html/E37841/mmap-2.html

MAP_32BIT is not defined. Instead it is using a MAP_LOW32 field
which is explicitly described as different from the MAP_32BIT
field in Linux distributions.

The patch is removing the mapping since:

* As mentioned solaris doesn't implement MAP_32BIT (Target)
* Not every host supports MAP_32BIT.
** http://man7.org/linux/man-pages/man2/mmap.2.html

In fact, assuming a Linux host, MAP_32BIT is defined for
x86-64 only, which means it is not possible to compile
gem5-SPARC on a (e.g.) Arm host.

Change-Id: Ibf234754941ae915e728db5fbc4ba1db3aaa1c81
Signed-off-by: Giacomo Travaglini 
Reviewed-by: Richard Cooper 
---
M src/arch/sparc/solaris/solaris.cc
M src/arch/sparc/solaris/solaris.hh
2 files changed, 0 insertions(+), 2 deletions(-)



diff --git a/src/arch/sparc/solaris/solaris.cc  
b/src/arch/sparc/solaris/solaris.cc

index c5b5902..33b2078 100644
--- a/src/arch/sparc/solaris/solaris.cc
+++ b/src/arch/sparc/solaris/solaris.cc
@@ -79,7 +79,6 @@
 SyscallFlagTransTable SparcSolaris::mmapFlagTable[] = {
   { TGT_MAP_SHARED, MAP_SHARED },
   { TGT_MAP_PRIVATE, MAP_PRIVATE },
-  { TGT_MAP_32BIT, MAP_32BIT},
   { TGT_MAP_ANON, MAP_ANON },
   { TGT_MAP_DENYWRITE, MAP_DENYWRITE },
   { TGT_MAP_EXECUTABLE, MAP_EXECUTABLE },
diff --git a/src/arch/sparc/solaris/solaris.hh  
b/src/arch/sparc/solaris/solaris.hh

index b2f126a..5ca811d 100644
--- a/src/arch/sparc/solaris/solaris.hh
+++ b/src/arch/sparc/solaris/solaris.hh
@@ -63,7 +63,6 @@

 static const unsigned TGT_MAP_SHARED= 0x1;
 static const unsigned TGT_MAP_PRIVATE   = 0x2;
-static const unsigned TGT_MAP_32BIT = 0x00040;
 static const unsigned TGT_MAP_ANON  = 0x00020;
 static const unsigned TGT_MAP_DENYWRITE = 0x00800;
 static const unsigned TGT_MAP_EXECUTABLE= 0x01000;

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ibf234754941ae915e728db5fbc4ba1db3aaa1c81
Gerrit-Change-Number: 27647
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini 
Gerrit-Reviewer: Richard Cooper 
Gerrit-MessageType: newchange
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: util: Standardize console output in the m5 writefile command.

2020-04-08 Thread Gabe Black (Gerrit)
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/27627 )



Change subject: util: Standardize console output in the m5 writefile  
command.

..

util: Standardize console output in the m5 writefile command.

When the command reports an error, it should then exit(2) and not just
return as if everything worked. When printing the number of bytes
written or the file being opened, it should write this non-error message
to cout, and not cerr.

Also used proper capitalization and punctuation in a couple messages.

Change-Id: I2c0d6592357965ed2eee8f090c8b3d530b354b9f
---
M util/m5/src/command/writefile.cc
1 file changed, 12 insertions(+), 6 deletions(-)



diff --git a/util/m5/src/command/writefile.cc  
b/util/m5/src/command/writefile.cc

index bef1932..7771dfe 100644
--- a/util/m5/src/command/writefile.cc
+++ b/util/m5/src/command/writefile.cc
@@ -26,6 +26,8 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */

+#include 
+
 #include 
 #include 
 #include 
@@ -41,12 +43,13 @@
 write_file(const DispatchTable , const std::string ,
const std::string _filename)
 {
-std::cerr << "opening " << filename << std::endl;
+std::cout << "Opening \"" << filename << "\"." << std::endl;
 std::ifstream src(filename, std::ios_base::in | std::ios_base::binary);

 if (!src) {
-std::cerr << "error opening " << filename << std::endl;
-return;
+std::cerr << "Error opening \"" << filename << "\": " <<
+strerror(errno) << std::endl;
+exit(2);
 }

 char buf[256 * 1024];
@@ -58,8 +61,11 @@
 src.seekg(offset);
 src.read(buf, sizeof(buf));
 int len = src.gcount();
-if (!src && !src.eof())
-break;
+if (!src && !src.eof()) {
+std::cerr << "Error reading \"" << filename << "\": " <<
+strerror(errno) << std::endl;
+exit(2);
+}
 char *wbuf = buf;
 while (len) {
 int bytes = (*dt.m5_write_file)(
@@ -71,7 +77,7 @@
 if (src.eof())
 break;
 }
-std::cerr << "Wrote " << offset << " bytes." << std::endl;
+std::cout << "Wrote " << offset << " bytes." << std::endl;
 }

 bool

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I2c0d6592357965ed2eee8f090c8b3d530b354b9f
Gerrit-Change-Number: 27627
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: util: Add a "writefile" unit test to the m5 utility.

2020-04-08 Thread Gabe Black (Gerrit)
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/27628 )



Change subject: util: Add a "writefile" unit test to the m5 utility.
..

util: Add a "writefile" unit test to the m5 utility.

Change-Id: Ic0e8d5fbbd5b6d6b57f674cef6460f94206a5872
---
M util/m5/src/command/SConscript.native
A util/m5/src/command/writefile.test.cc
2 files changed, 248 insertions(+), 0 deletions(-)



diff --git a/util/m5/src/command/SConscript.native  
b/util/m5/src/command/SConscript.native

index 7d23b2a..fc3e975 100644
--- a/util/m5/src/command/SConscript.native
+++ b/util/m5/src/command/SConscript.native
@@ -37,6 +37,7 @@
 'readfile',
 'resetstats',
 'sum',
+'writefile',
 )

 Return('command_tests')
diff --git a/util/m5/src/command/writefile.test.cc  
b/util/m5/src/command/writefile.test.cc

new file mode 100644
index 000..a1adf5c
--- /dev/null
+++ b/util/m5/src/command/writefile.test.cc
@@ -0,0 +1,247 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met: redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer;
+ * redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution;
+ * neither the name of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#include "args.hh"
+#include "command.hh"
+#include "dispatch_table.hh"
+
+uint64_t test_total_written;
+std::string test_host_file_name;
+
+std::vector test_written_data;
+uint64_t test_max_buf_size;
+
+uint64_t
+test_m5_write_file(void *buffer, uint64_t len, uint64_t offset,
+   const char *filename)
+{
+if (test_max_buf_size && len > test_max_buf_size)
+len = test_max_buf_size;
+
+test_total_written += len;
+
+if (test_host_file_name == "")
+test_host_file_name = filename;
+else
+EXPECT_EQ(test_host_file_name, filename);
+
+if (offset == 0)
+test_written_data.clear();
+
+size_t required_size = offset + len;
+if (test_written_data.size() < required_size)
+test_written_data.resize(required_size);
+
+memcpy(test_written_data.data() + offset, buffer, len);
+
+return len;
+}
+
+DispatchTable dt = { .m5_write_file = _m5_write_file };
+
+std::string cout_output;
+
+bool
+run(std::initializer_list arg_args)
+{
+test_total_written = 0;
+test_host_file_name = "";
+test_written_data.clear();
+
+Args args(arg_args);
+
+// Redirect cout into a stringstream.
+std::stringstream buffer;
+std::streambuf *orig = std::cout.rdbuf(buffer.rdbuf());
+
+bool res = Command::run(dt, args);
+
+// Capture the contents of the stringstream and restore cout.
+cout_output = buffer.str();
+std::cout.rdbuf(orig);
+
+return res;
+}
+
+class TempFile
+{
+  private:
+size_t _size;
+int fd;
+std::string _path;
+void *_buf;
+
+  public:
+TempFile(size_t _size) : _size(_size)
+{
+// Generate a temporary filename.
+char *tmp_name = strdup("/tmp/writefile.test.");
+fd = mkstemp(tmp_name);
+_path = tmp_name;
+free(tmp_name);
+
+// Make the file the appropriate length.
+assert(!ftruncate(fd, _size));
+
+// mmap the file.
+_buf = mmap(nullptr, _size, PROT_READ | PROT_WRITE, MAP_SHARED,  
fd, 0);

+assert(_buf);
+
+// Fill it with an incrementing 32 bit integers.
+
+int chunk_size = sizeof(uint32_t);
+size_t num_chunks = _size / chunk_size;
+int leftovers = _size % chunk_size;
+
+uint32_t *buf32 = (uint32_t *)_buf;
+

[gem5-dev] Change in gem5/gem5[develop]: util: Add unit tests for most remaining m5 utility commands.

2020-04-08 Thread Gabe Black (Gerrit)
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/27607 )



Change subject: util: Add unit tests for most remaining m5 utility commands.
..

util: Add unit tests for most remaining m5 utility commands.

The only two which still need unit tests are the more complex commands,
readfile and writefile.

Change-Id: Ib9984c71fb4449cbbbd1e2a43f3140975328d31f
---
M util/m5/src/command/SConscript.native
A util/m5/src/command/addsymbol.test.cc
A util/m5/src/command/checkpoint.test.cc
A util/m5/src/command/dumpresetstats.test.cc
A util/m5/src/command/dumpstats.test.cc
A util/m5/src/command/exit.test.cc
A util/m5/src/command/fail.test.cc
A util/m5/src/command/initparam.test.cc
A util/m5/src/command/loadsymbol.test.cc
A util/m5/src/command/resetstats.test.cc
10 files changed, 719 insertions(+), 0 deletions(-)



diff --git a/util/m5/src/command/SConscript.native  
b/util/m5/src/command/SConscript.native

index d27cd26..10e45d2 100644
--- a/util/m5/src/command/SConscript.native
+++ b/util/m5/src/command/SConscript.native
@@ -26,6 +26,15 @@
 Import('*')

 command_tests = (
+'addsymbol',
+'checkpoint',
+'dumpresetstats',
+'dumpstats',
+'exit',
+'fail',
+'initparam',
+'loadsymbol',
+'resetstats',
 'sum',
 )

diff --git a/util/m5/src/command/addsymbol.test.cc  
b/util/m5/src/command/addsymbol.test.cc

new file mode 100644
index 000..bd2fd65
--- /dev/null
+++ b/util/m5/src/command/addsymbol.test.cc
@@ -0,0 +1,70 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met: redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer;
+ * redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution;
+ * neither the name of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include 
+
+#include "args.hh"
+#include "command.hh"
+#include "dispatch_table.hh"
+
+uint64_t test_addr;
+std::string test_symbol;
+
+void
+test_m5_add_symbol(uint64_t addr, const char *symbol)
+{
+test_addr = addr;
+// Make a copy of what is in symbol so it's still available when we go  
to

+// check it later.
+test_symbol = symbol;
+}
+
+DispatchTable dt = { .m5_add_symbol = _m5_add_symbol };
+
+bool
+run(std::initializer_list arg_args)
+{
+Args args(arg_args);
+return Command::run(dt, args);
+}
+
+TEST(Exit, Arguments)
+{
+// Called with no arguments.
+EXPECT_FALSE(run({"addsymbol"}));
+
+// Called with one argument.
+EXPECT_FALSE(run({"addsymbol", "1"}));
+
+// Called with two arguments.
+EXPECT_TRUE(run({"addsymbol", "1234", "test_symbol_name"}));
+EXPECT_EQ(test_addr, 1234);
+EXPECT_EQ(test_symbol, "test_symbol_name");
+
+// Called with three arguments.
+EXPECT_FALSE(run({"addsymbol", "1", "2", "3"}));
+}
diff --git a/util/m5/src/command/checkpoint.test.cc  
b/util/m5/src/command/checkpoint.test.cc

new file mode 100644
index 000..fe15050
--- /dev/null
+++ b/util/m5/src/command/checkpoint.test.cc
@@ -0,0 +1,78 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met: redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer;
+ * redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution;
+ * neither the name of the copyright holders nor the names of its
+ * contributors may be used to endorse or 

[gem5-dev] Change in gem5/gem5[develop]: util: Add a unit test for the m5 utility's "readfile" command.

2020-04-08 Thread Gabe Black (Gerrit)
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/27608 )



Change subject: util: Add a unit test for the m5 utility's "readfile"  
command.

..

util: Add a unit test for the m5 utility's "readfile" command.

This feeds a fake file to the readfile command which is just a sequence
of incrementing 32 bit values. The incrementing values make sure that
the right region of the input file is being read at the right position,
and the relatively small size means there shouldn't be tons of zeroes
everywhere which can't be distinguished from each other.

Change-Id: I4286b1f92684f127c4885c29192c6c5244a61855
---
M util/m5/src/command/SConscript.native
A util/m5/src/command/readfile.test.cc
2 files changed, 209 insertions(+), 0 deletions(-)



diff --git a/util/m5/src/command/SConscript.native  
b/util/m5/src/command/SConscript.native

index 10e45d2..7d23b2a 100644
--- a/util/m5/src/command/SConscript.native
+++ b/util/m5/src/command/SConscript.native
@@ -34,6 +34,7 @@
 'fail',
 'initparam',
 'loadsymbol',
+'readfile',
 'resetstats',
 'sum',
 )
diff --git a/util/m5/src/command/readfile.test.cc  
b/util/m5/src/command/readfile.test.cc

new file mode 100644
index 000..d5ffe1d
--- /dev/null
+++ b/util/m5/src/command/readfile.test.cc
@@ -0,0 +1,208 @@
+/*
+ * Copyright 2020 Google Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met: redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer;
+ * redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution;
+ * neither the name of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include 
+
+#include 
+
+#include "args.hh"
+#include "command.hh"
+#include "dispatch_table.hh"
+
+uint64_t test_read_file_size;
+uint64_t test_max_buf_size;
+
+uint64_t test_total_read;
+
+uint64_t
+test_m5_read_file(void *buffer, uint64_t len, uint64_t offset)
+{
+// The "file" we're reading is just a series of incrementing 32 bit
+// integers.
+
+// If the buffer is entirely past the end of our "file", return 0.
+if (offset >= test_read_file_size)
+return 0;
+
+// If the buffer extends beyond our "file" truncate it.
+if (offset + len > test_read_file_size)
+len = test_read_file_size - offset;
+
+// If more data was requested than we want to send at once, truncate  
len.

+if (test_max_buf_size && len > test_max_buf_size)
+len = test_max_buf_size;
+
+int chunk_size = sizeof(uint32_t);
+
+// How much of len is still unaccounted for.
+uint64_t remaining = len;
+
+// How much overlaps with the preceeding chunk?
+int at_start = chunk_size - (offset % chunk_size);
+// If we don't even cover the entire previous chunk...
+if (at_start > len)
+at_start = len;
+remaining -= at_start;
+
+// How much overlaps with the following chunk?
+int at_end = remaining % chunk_size;
+remaining -= at_end;
+
+// The number of chunks are the number we cover fully, plus one for  
each

+// end were we partially overlap.
+uint64_t num_chunks = remaining / chunk_size +
+(at_start ? 1 : 0) + (at_end ? 1 : 0);
+
+// Build this part of the file.
+uint32_t *chunks = new uint32_t [num_chunks];
+
+uint32_t chunk_idx = offset / chunk_size;
+for (uint64_t i = 0; i < num_chunks; i++)
+chunks[i] = chunk_idx++;
+
+// Copy out to the requested buffer.
+memcpy(buffer, ((uint8_t *)chunks) + (chunk_size - at_start), len);
+
+// Clean up.
+delete [] chunks;
+
+test_total_read += len;
+return len;
+}
+
+DispatchTable dt = { .m5_read_file = _m5_read_file };
+
+std::string