Re: [gem5-dev] Review Request 3806: sparc: fix bugs caused by cd7f3a1dbf55

[Andreas Hansson]

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

Ship it!


I would argue that this is a prime example why removing SPARC is a good idea.

- Andreas Hansson


On Feb. 10, 2017, 8:31 p.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3806/
> ---
> 
> (Updated Feb. 10, 2017, 8:31 p.m.)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, and Steve Reinhardt.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11803:0ab0146d777a
> ---
> sparc: fix bugs caused by cd7f3a1dbf55
> 
> Turns out that SPARC SE mode relied on M5_pid being "0" in
> all cases. The entries in the SPARC TLBs are accessed with
> M5_pid as their context. This is buggy in the sense that it
> will never work with more than one process or any
> initialization that doesn't have the M5_pid value passed in
> as "0".
> 
> cd7f3a1dbf55 broke the SPARC build because it deletes M5_pid
> and uses a _pid with a default of "100" instead. This caused
> the SPARC TLB to never return any valid lookups for any
> request; the program never moved past the first instruction
> with SPARC SE in the regression tester.
> 
> The solution proposed in this changeset is to initialize
> the address space identification register with the PID value
> that is passed into the process class as a parameter from
> Python. This should return the correct responses from the TLB
> since the insertions and lookups into the page table will be
> using the same PID.
> 
> Furthermore, there are corner cases in the code which elevate
> privileges and revert to using context "0" as the context in
> the TLB. I believe that these are related to kernel level
> traps and hypervisor privilege escalations, but I'm not
> completely sure. I've tried to address the corner cases
> properly, but it would be beneficial to have someone who is
> familiar with the SPARC architecture to take a look at this
> fix.
> 
> 
> Diffs
> -
> 
>   src/arch/sparc/process.cc cd7f3a1dbf55bfa03b436c8cde51ebda515fbdbc 
>   src/arch/sparc/faults.cc cd7f3a1dbf55bfa03b436c8cde51ebda515fbdbc 
> 
> Diff: http://reviews.gem5.org/r/3806/diff/
> 
> 
> Testing
> ---
> 
> util/regress all
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

[gem5-dev] Review Request 3814: misc: Add a CONTRIBUTING document

[Jason Lowe-Power]

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

Review request for Default.


Repository: gem5


Description
---

changesets:
11849:b5f456096b51 "misc: Add a CONTRIBUTING document

This document details how to contribute to gem5 based on our new
contribution flow with git and gerrit.

Signed-off-by: Jason Lowe-Power "


Diffs
-

  CONTRIBUTING PRE-CREATION 

Diff: http://reviews.gem5.org/r/3814/diff/


Testing
---


Thanks,

Jason Lowe-Power

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

[gem5-dev] Migrating to git and gerrit

[Jason Lowe-Power]

Hi all,

We've been talking about this for a while, but now it's time! Special
thanks to Andreas Sandberg for all of his hard work for putting this
together.

We will be migrating our infrastructure from the self-hosted mercurial repo
at repo.gem5.org and reviewboard to git and gerrit hosted on Google's new
googlesource website. You can find a live version (not ready for primetime)
of this at https://gem5.googlesource.com/.

We are planning on flipping the switch on *March 1st*, unless there is an
objection from the community. Note: I'm announcing this on gem5-dev before
announcing on gem5-users and gem5-announce in case there's any details
we've missed.

I've posted a patch on reviewboard that contains a new CONTRIBUTING
document that details the new contribution process. Please review that
document so it can be pushed before we transition. (
http://reviews.gem5.org/r/3814/)

The major changes are detailed below:
1. REPOSITORIES
  * The canonical version of gem5 will now live at
https://gem5.googlesource.com/, not repo.gem5.org
  * The mercurial repository at repo.gem5.org will be a read-only mirror of
the googlesource repo.
  * We will keep the github mirror
2. CODE REVIEWS
  * All reviews will happen on https://gem5-review.googlesource.com/.
  * No new patches will be accepted on reviewboard after March 1. Any
patches still on reviewboard will be discussed/reviewed there, but
committers will have to manually commit them to the git repo (not unlike
our current situation).

Main differences for developers:
1. You will have to learn to use git, if you haven't already
2. Developers who submit patches will be able to *commit their own patches*
after review. No more waiting for me to push patches for you!
3. Continuous integration tests are coming soon, and must pass before a
patch is committed.
4. Gerrit has a different user-interface than reviewboard... sorry for the
change.
5. Many of the policies we have for commits will be *strictly enforced*
automatically by gerrit. E.g., it will no longer be possible to post a
patch that has a non-conforming commit message.
6. Instead of using postreview, patches will be posted by pushing to
special git branches on our gerrit instance.

*Please let me know if you have any questions or concerns. I'd like to iron
all of this out in the next few days so we can post the announcement on
gem5-users and gem5-announce.*

I would also like to thank the team at Google that has generously donated
their time to set up this infrastructure. This include Rahul Thakur, Jason
Buberel, Haihong Zhuo, and many others.

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

[gem5-dev] Review Request 3815: sim: fix out-of-bounds error in syscall_desc

[Brandon Potter]

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

Review request for Default.


Repository: gem5


Description
---

Changeset 11849:9152df7d2747
---
sim: fix out-of-bounds error in syscall_desc


Diffs
-

  src/sim/syscall_desc.cc f438fcbab00edbb36075e1403da75cb9a9ac7f55 

Diff: http://reviews.gem5.org/r/3815/diff/


Testing
---


Thanks,

Brandon Potter

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

Re: [gem5-dev] Review Request 3815: sim: fix out-of-bounds error in syscall_desc

[Jason Lowe-Power]

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

Ship it!


Minor thing below. Since this is confusing, I think it could use more comments.


src/sim/syscall_desc.cc (line 55)


Can you add a comment here. This is very confusing code. Maybe something 
like 

// RHS is evaluated before LHS and getSyscallArg increments index (passed 
by reference).

Note: you should also add braces, if you add this inline comment.


- Jason Lowe-Power


On Feb. 16, 2017, 5:39 p.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3815/
> ---
> 
> (Updated Feb. 16, 2017, 5:39 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11849:9152df7d2747
> ---
> sim: fix out-of-bounds error in syscall_desc
> 
> 
> Diffs
> -
> 
>   src/sim/syscall_desc.cc f438fcbab00edbb36075e1403da75cb9a9ac7f55 
> 
> Diff: http://reviews.gem5.org/r/3815/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

Re: [gem5-dev] Review Request 3806: sparc: fix bugs caused by cd7f3a1dbf55

[Brandon Potter]

> On Feb. 16, 2017, 10:44 a.m., Andreas Hansson wrote:
> > I would argue that this is a prime example why removing SPARC is a good 
> > idea.

SPARC, ALPHA, and MIPS?


- Brandon


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


On Feb. 10, 2017, 8:31 p.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3806/
> ---
> 
> (Updated Feb. 10, 2017, 8:31 p.m.)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, and Steve Reinhardt.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11803:0ab0146d777a
> ---
> sparc: fix bugs caused by cd7f3a1dbf55
> 
> Turns out that SPARC SE mode relied on M5_pid being "0" in
> all cases. The entries in the SPARC TLBs are accessed with
> M5_pid as their context. This is buggy in the sense that it
> will never work with more than one process or any
> initialization that doesn't have the M5_pid value passed in
> as "0".
> 
> cd7f3a1dbf55 broke the SPARC build because it deletes M5_pid
> and uses a _pid with a default of "100" instead. This caused
> the SPARC TLB to never return any valid lookups for any
> request; the program never moved past the first instruction
> with SPARC SE in the regression tester.
> 
> The solution proposed in this changeset is to initialize
> the address space identification register with the PID value
> that is passed into the process class as a parameter from
> Python. This should return the correct responses from the TLB
> since the insertions and lookups into the page table will be
> using the same PID.
> 
> Furthermore, there are corner cases in the code which elevate
> privileges and revert to using context "0" as the context in
> the TLB. I believe that these are related to kernel level
> traps and hypervisor privilege escalations, but I'm not
> completely sure. I've tried to address the corner cases
> properly, but it would be beneficial to have someone who is
> familiar with the SPARC architecture to take a look at this
> fix.
> 
> 
> Diffs
> -
> 
>   src/arch/sparc/process.cc cd7f3a1dbf55bfa03b436c8cde51ebda515fbdbc 
>   src/arch/sparc/faults.cc cd7f3a1dbf55bfa03b436c8cde51ebda515fbdbc 
> 
> Diff: http://reviews.gem5.org/r/3806/diff/
> 
> 
> Testing
> ---
> 
> util/regress all
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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