Re: [m5-dev] Review Request: ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.

2011-03-31 Thread Ali Saidi
Everyone, this change alters the way that the O3 cpu switches registers from 
the atomic cpu. If you use checkpoint/switchover and m5 please test this 
(specifically the change to src/cpu/o3/thread_context_impl.hh)

Thanks,
Ali



On Mar 30, 2011, at 4:55 PM, Ali Saidi wrote:

 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.m5sim.org/r/620/
 ---
 
 Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
 Nathan Binkert.
 
 
 Summary
 ---
 
 ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.
 
 This change fixes a small bug in the arm copyRegs() code where some registers
 wouldn't be copied if the processor was in a mode other than MODE_USER.
 Additionally, this change simplifies the way the O3 switchCpu code works by
 utilizing TheISA::copyRegs() to copy the required context information
 rather than the adhoc copying that goes on in the CPU model. The current code
 makes assumptions about the visibility of int and float registers that aren't
 true for all architectures in FS mode.
 
 
 Diffs
 -
 
  src/arch/arm/isa.cc d54b7775a6b0 
  src/arch/arm/miscregs.hh d54b7775a6b0 
  src/arch/arm/utility.cc d54b7775a6b0 
  src/cpu/o3/thread_context_impl.hh d54b7775a6b0 
 
 Diff: http://reviews.m5sim.org/r/620/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ali
 
 ___
 m5-dev mailing list
 m5-dev@m5sim.org
 http://m5sim.org/mailman/listinfo/m5-dev
 

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


[m5-dev] Review Request: ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.

2011-03-30 Thread Ali Saidi

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

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.

This change fixes a small bug in the arm copyRegs() code where some registers
wouldn't be copied if the processor was in a mode other than MODE_USER.
Additionally, this change simplifies the way the O3 switchCpu code works by
utilizing TheISA::copyRegs() to copy the required context information
rather than the adhoc copying that goes on in the CPU model. The current code
makes assumptions about the visibility of int and float registers that aren't
true for all architectures in FS mode.


Diffs
-

  src/arch/arm/isa.cc d54b7775a6b0 
  src/arch/arm/miscregs.hh d54b7775a6b0 
  src/arch/arm/utility.cc d54b7775a6b0 
  src/cpu/o3/thread_context_impl.hh d54b7775a6b0 

Diff: http://reviews.m5sim.org/r/620/diff


Testing
---


Thanks,

Ali

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


Re: [m5-dev] Review Request: ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.

2011-03-30 Thread Gabe Black

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/620/#review1049
---


I'm not sure this is right yet. Won't it only copy the USR registers now and 
leave out all the other modes? Also, is there anything wrong with reading the 
CPSR, changing the mode, and then writing it back?


src/arch/arm/isa.cc
http://reviews.m5sim.org/r/620/#comment1420

Random blank line.


- Gabe


On 2011-03-30 14:55:05, Ali Saidi wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.m5sim.org/r/620/
 ---
 
 (Updated 2011-03-30 14:55:05)
 
 
 Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
 Nathan Binkert.
 
 
 Summary
 ---
 
 ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.
 
 This change fixes a small bug in the arm copyRegs() code where some registers
 wouldn't be copied if the processor was in a mode other than MODE_USER.
 Additionally, this change simplifies the way the O3 switchCpu code works by
 utilizing TheISA::copyRegs() to copy the required context information
 rather than the adhoc copying that goes on in the CPU model. The current code
 makes assumptions about the visibility of int and float registers that aren't
 true for all architectures in FS mode.
 
 
 Diffs
 -
 
   src/arch/arm/isa.cc d54b7775a6b0 
   src/arch/arm/miscregs.hh d54b7775a6b0 
   src/arch/arm/utility.cc d54b7775a6b0 
   src/cpu/o3/thread_context_impl.hh d54b7775a6b0 
 
 Diff: http://reviews.m5sim.org/r/620/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ali
 


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


Re: [m5-dev] Review Request: ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.

2011-03-30 Thread Ali Saidi


 On 2011-03-30 15:38:14, Gabe Black wrote:
  I'm not sure this is right yet. Won't it only copy the USR registers now 
  and leave out all the other modes? Also, is there anything wrong with 
  reading the CPSR, changing the mode, and then writing it back?

No, NumIntRegs is all the registers in the system, not just the user ones. 
However, if some other register mapping is in effect, the mapping hides the 
user registers so they can't be accessed. Starting in user mode solves the 
problem. As far as the CPSR goes, I only want the updateRegMap() functionality, 
so the no effect versions can't be used and the effect version can change other 
things (E.g. if you were in thumb mode and cpsr mode was written you the 
pcstate would be updated). This seems much cleaner. 


- Ali


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/620/#review1049
---


On 2011-03-30 14:55:05, Ali Saidi wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.m5sim.org/r/620/
 ---
 
 (Updated 2011-03-30 14:55:05)
 
 
 Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
 Nathan Binkert.
 
 
 Summary
 ---
 
 ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.
 
 This change fixes a small bug in the arm copyRegs() code where some registers
 wouldn't be copied if the processor was in a mode other than MODE_USER.
 Additionally, this change simplifies the way the O3 switchCpu code works by
 utilizing TheISA::copyRegs() to copy the required context information
 rather than the adhoc copying that goes on in the CPU model. The current code
 makes assumptions about the visibility of int and float registers that aren't
 true for all architectures in FS mode.
 
 
 Diffs
 -
 
   src/arch/arm/isa.cc d54b7775a6b0 
   src/arch/arm/miscregs.hh d54b7775a6b0 
   src/arch/arm/utility.cc d54b7775a6b0 
   src/cpu/o3/thread_context_impl.hh d54b7775a6b0 
 
 Diff: http://reviews.m5sim.org/r/620/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ali
 


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


Re: [m5-dev] Review Request: ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.

2011-03-30 Thread Gabe Black


 On 2011-03-30 15:38:14, Gabe Black wrote:
  I'm not sure this is right yet. Won't it only copy the USR registers now 
  and leave out all the other modes? Also, is there anything wrong with 
  reading the CPSR, changing the mode, and then writing it back?
 
 Ali Saidi wrote:
 No, NumIntRegs is all the registers in the system, not just the user 
 ones. However, if some other register mapping is in effect, the mapping hides 
 the user registers so they can't be accessed. Starting in user mode solves 
 the problem. As far as the CPSR goes, I only want the updateRegMap() 
 functionality, so the no effect versions can't be used and the effect version 
 can change other things (E.g. if you were in thumb mode and cpsr mode was 
 written you the pcstate would be updated). This seems much cleaner.

Ah, ok, so everything that isn't put through the map is flattened to the same 
thing. That seems a little fragile since if flattening changes it'll break, and 
it relies on USR mode being identically mapped. I'm not sure exactly what to do 
about it, though.


- Gabe


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/620/#review1049
---


On 2011-03-30 14:55:05, Ali Saidi wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.m5sim.org/r/620/
 ---
 
 (Updated 2011-03-30 14:55:05)
 
 
 Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
 Nathan Binkert.
 
 
 Summary
 ---
 
 ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.
 
 This change fixes a small bug in the arm copyRegs() code where some registers
 wouldn't be copied if the processor was in a mode other than MODE_USER.
 Additionally, this change simplifies the way the O3 switchCpu code works by
 utilizing TheISA::copyRegs() to copy the required context information
 rather than the adhoc copying that goes on in the CPU model. The current code
 makes assumptions about the visibility of int and float registers that aren't
 true for all architectures in FS mode.
 
 
 Diffs
 -
 
   src/arch/arm/isa.cc d54b7775a6b0 
   src/arch/arm/miscregs.hh d54b7775a6b0 
   src/arch/arm/utility.cc d54b7775a6b0 
   src/cpu/o3/thread_context_impl.hh d54b7775a6b0 
 
 Diff: http://reviews.m5sim.org/r/620/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ali
 


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


Re: [m5-dev] Review Request: ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.

2011-03-30 Thread Ali Saidi


 On 2011-03-30 15:38:14, Gabe Black wrote:
  I'm not sure this is right yet. Won't it only copy the USR registers now 
  and leave out all the other modes? Also, is there anything wrong with 
  reading the CPSR, changing the mode, and then writing it back?
 
 Ali Saidi wrote:
 No, NumIntRegs is all the registers in the system, not just the user 
 ones. However, if some other register mapping is in effect, the mapping hides 
 the user registers so they can't be accessed. Starting in user mode solves 
 the problem. As far as the CPSR goes, I only want the updateRegMap() 
 functionality, so the no effect versions can't be used and the effect version 
 can change other things (E.g. if you were in thumb mode and cpsr mode was 
 written you the pcstate would be updated). This seems much cleaner.
 
 Gabe Black wrote:
 Ah, ok, so everything that isn't put through the map is flattened to the 
 same thing. That seems a little fragile since if flattening changes it'll 
 break, and it relies on USR mode being identically mapped. I'm not sure 
 exactly what to do about it, though.

I can't come up with a better solution, but you wrote the initial code that 
operated that way ;). 


- Ali


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/620/#review1049
---


On 2011-03-30 14:55:05, Ali Saidi wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.m5sim.org/r/620/
 ---
 
 (Updated 2011-03-30 14:55:05)
 
 
 Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
 Nathan Binkert.
 
 
 Summary
 ---
 
 ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.
 
 This change fixes a small bug in the arm copyRegs() code where some registers
 wouldn't be copied if the processor was in a mode other than MODE_USER.
 Additionally, this change simplifies the way the O3 switchCpu code works by
 utilizing TheISA::copyRegs() to copy the required context information
 rather than the adhoc copying that goes on in the CPU model. The current code
 makes assumptions about the visibility of int and float registers that aren't
 true for all architectures in FS mode.
 
 
 Diffs
 -
 
   src/arch/arm/isa.cc d54b7775a6b0 
   src/arch/arm/miscregs.hh d54b7775a6b0 
   src/arch/arm/utility.cc d54b7775a6b0 
   src/cpu/o3/thread_context_impl.hh d54b7775a6b0 
 
 Diff: http://reviews.m5sim.org/r/620/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ali
 


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


Re: [m5-dev] Review Request: ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.

2011-03-30 Thread Gabe Black


 On 2011-03-30 15:38:14, Gabe Black wrote:
  I'm not sure this is right yet. Won't it only copy the USR registers now 
  and leave out all the other modes? Also, is there anything wrong with 
  reading the CPSR, changing the mode, and then writing it back?
 
 Ali Saidi wrote:
 No, NumIntRegs is all the registers in the system, not just the user 
 ones. However, if some other register mapping is in effect, the mapping hides 
 the user registers so they can't be accessed. Starting in user mode solves 
 the problem. As far as the CPSR goes, I only want the updateRegMap() 
 functionality, so the no effect versions can't be used and the effect version 
 can change other things (E.g. if you were in thumb mode and cpsr mode was 
 written you the pcstate would be updated). This seems much cleaner.
 
 Gabe Black wrote:
 Ah, ok, so everything that isn't put through the map is flattened to the 
 same thing. That seems a little fragile since if flattening changes it'll 
 break, and it relies on USR mode being identically mapped. I'm not sure 
 exactly what to do about it, though.
 
 Ali Saidi wrote:
 I can't come up with a better solution, but you wrote the initial code 
 that operated that way ;).

Honestly I probably copied and pasted that or left it from the original 
implementation, and since I usually don't pay much attention to the 
checkpointing/CPU switching stuff just left it that way while I got the other 
parts working. So you could say it's my fault but not my design.


- Gabe


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/620/#review1049
---


On 2011-03-30 14:55:05, Ali Saidi wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://reviews.m5sim.org/r/620/
 ---
 
 (Updated 2011-03-30 14:55:05)
 
 
 Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
 Nathan Binkert.
 
 
 Summary
 ---
 
 ARM: Fix checkpoint restoration into O3 CPU and the way O3 switchCpu works.
 
 This change fixes a small bug in the arm copyRegs() code where some registers
 wouldn't be copied if the processor was in a mode other than MODE_USER.
 Additionally, this change simplifies the way the O3 switchCpu code works by
 utilizing TheISA::copyRegs() to copy the required context information
 rather than the adhoc copying that goes on in the CPU model. The current code
 makes assumptions about the visibility of int and float registers that aren't
 true for all architectures in FS mode.
 
 
 Diffs
 -
 
   src/arch/arm/isa.cc d54b7775a6b0 
   src/arch/arm/miscregs.hh d54b7775a6b0 
   src/arch/arm/utility.cc d54b7775a6b0 
   src/cpu/o3/thread_context_impl.hh d54b7775a6b0 
 
 Diff: http://reviews.m5sim.org/r/620/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ali
 


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