Discussion:
[gem5-dev] Review Request: CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
(too old to reply)
Geoffrey Blake
2011-11-23 23:07:45 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------

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


Summary
-------

CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5

Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.


Diffs
-----

build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
src/mem/packet.hh 62dee0c98d53
src/mem/packet.cc 62dee0c98d53

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


Testing
-------

Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.


Thanks,

Geoffrey
Gabe Black
2011-11-24 05:35:58 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1672
-----------------------------------------------------------


When you upload a new review, be sure to update the old one. That way you can see what changed between revisions.


src/cpu/base_dyn_inst.hh
<http://reviews.m5sim.org/r/910/#comment2167>

Like I said before, you shouldn't use this instResult mechanism. It's broken and obsolete and needs to be removed all together.



src/cpu/o3/thread_context.hh
<http://reviews.m5sim.org/r/910/#comment2169>

These shouldn't be squished all onto one line.



src/cpu/thread_context.hh
<http://reviews.m5sim.org/r/910/#comment2168>

I think all of these new functions could be replaced with a getCheckerCPU call on the base CPU class. Then you can get the TLB pointers from it and even factor some of the code that's duplicated for the checker into functions that take a CPU pointer.



src/mem/packet.hh
<http://reviews.m5sim.org/r/910/#comment2170>

I still don't see the point of this. The memory system should be taking care of this, and it shouldn't have anything to do with the checker.


- Gabe
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-23 15:07:45)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
src/mem/packet.hh 62dee0c98d53
src/mem/packet.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Geoffrey Blake
2011-11-28 18:29:00 UTC
Permalink
Post by Geoffrey Blake
src/cpu/base_dyn_inst.hh, line 265
<http://reviews.m5sim.org/r/910/diff/1/?file=15545#file15545line265>
Like I said before, you shouldn't use this instResult mechanism. It's broken and obsolete and needs to be removed all together.
The CheckerCPU and CPU model being checked are not required to always be in lock step. So the dyn_inst object has to save its results internally using this mechanism so the Checker can sync back up.

The cases where this happens are detailed in a comment at src/cpu/checker/cpu_impl.hh:124, and it involves the interaction between the checker and the ld/st queue.
Post by Geoffrey Blake
src/cpu/thread_context.hh, line 126
<http://reviews.m5sim.org/r/910/diff/1/?file=15568#file15568line126>
I think all of these new functions could be replaced with a getCheckerCPU call on the base CPU class. Then you can get the TLB pointers from it and even factor some of the code that's duplicated for the checker into functions that take a CPU pointer.
This keeps it rather clear what is getting accessed by these functions instead of adding another level of indirection, unless its desired to refactor parts of the thread_context code as a whole (getting rid of getDTBPtr for the regular CPUs as well). The way the Checker, O3 and thread_contexts interact is fairly difficult to understand as it is currently, but I'm not sure how to fix this anyways.


- Geoffrey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1672
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-28 10:07:14)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Ali Saidi
2011-11-25 19:40:03 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1675
-----------------------------------------------------------


The changes to the packet class fix a long standing limitation where finding a packet that partially satisfied a request would cause a panic. This code remedies that limitation, although it should be in it's own changeset.


src/mem/packet.cc
<http://reviews.m5sim.org/r/910/#comment2171>

This should be a separate change and doesn't need to be protected by a checker ifdef. The code is useful in all cases.



src/mem/packet.cc
<http://reviews.m5sim.org/r/910/#comment2172>

Same as before. The comment should be updated to note that the partial read satisfy issue no longer exists.


- Ali
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-23 15:07:45)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
src/mem/packet.hh 62dee0c98d53
src/mem/packet.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Geoffrey Blake
2011-11-28 18:07:14 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------

(Updated 2011-11-28 10:07:14.377822)


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


Summary
-------

CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5

Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.


Diffs (updated)
-----

build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53

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


Testing
-------

Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.


Thanks,

Geoffrey
Andreas Hansson
2011-11-28 18:37:35 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1694
-----------------------------------------------------------



src/mem/bus.cc
<http://reviews.m5sim.org/r/910/#comment2179>

Why? This seems odd to me. If it is a response then someone responded during the snooping. If the destination port is the same as the source we have a loop. Why should it not continue if it is the default port?

Is this not working around an issue located somewhere else?


- Andreas
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-28 10:07:14)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Geoffrey Blake
2011-11-28 20:41:00 UTC
Permalink
Post by Geoffrey Blake
src/mem/bus.cc, line 480
<http://reviews.m5sim.org/r/910/diff/2/?file=15618#file15618line480>
Why? This seems odd to me. If it is a response then someone responded during the snooping. If the destination port is the same as the source we have a loop. Why should it not continue if it is the default port?
Is this not working around an issue located somewhere else?
This workaround is due to the recursive nature of a functional access, and the presence of coherent I/O in FS mode. If a functional access goes all the way out to the membus, it will first access the snooping devices hanging off the membus, which is the iocache. It has an active address range for the ARM realview test setups I'm using as 0x0-256MB. If the iocache does not satisfy the request it will send the functional access onto the iobus and probably not find a device to satisfy the access thereby sending it to the default port which is effectively a null pointer, crashing the simulation when accessed. The Checker setup does exercise the functional memory access path heavily though. Thinking about it now, I'm surprised this hasn't been hit before in normal operation.

Ali is probably better suited to offer an opinion on a better solution to this as he's pretty familiar with the I/O setup in M5.


- Geoffrey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1694
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-28 10:07:14)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Andreas Hansson
2011-11-29 11:05:08 UTC
Permalink
Post by Geoffrey Blake
src/mem/bus.cc, line 480
<http://reviews.m5sim.org/r/910/diff/2/?file=15618#file15618line480>
Why? This seems odd to me. If it is a response then someone responded during the snooping. If the destination port is the same as the source we have a loop. Why should it not continue if it is the default port?
Is this not working around an issue located somewhere else?
This workaround is due to the recursive nature of a functional access, and the presence of coherent I/O in FS mode. If a functional access goes all the way out to the membus, it will first access the snooping devices hanging off the membus, which is the iocache. It has an active address range for the ARM realview test setups I'm using as 0x0-256MB. If the iocache does not satisfy the request it will send the functional access onto the iobus and probably not find a device to satisfy the access thereby sending it to the default port which is effectively a null pointer, crashing the simulation when accessed. The Checker setup does exercise the functional memory access path heavily though. Thinking about it now, I'm surprised this hasn't been hit before in normal operation.
Ali is probably better suited to offer an opinion on a better solution to this as he's pretty familiar with the I/O setup in M5.
Sorry, but I am still not seeing the bigger picture here. A CPU (in this case checker?) makes an access to an address that does not exist in the system? Why would that ever be right? The only reason it would end up at an unconnected default port is if the address is used incorrectly as far as I can tell. Am I missing something?


- Andreas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1694
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-28 10:07:14)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Geoffrey Blake
2011-11-29 17:21:47 UTC
Permalink
Post by Geoffrey Blake
src/mem/bus.cc, line 480
<http://reviews.m5sim.org/r/910/diff/2/?file=15618#file15618line480>
Why? This seems odd to me. If it is a response then someone responded during the snooping. If the destination port is the same as the source we have a loop. Why should it not continue if it is the default port?
Is this not working around an issue located somewhere else?
This workaround is due to the recursive nature of a functional access, and the presence of coherent I/O in FS mode. If a functional access goes all the way out to the membus, it will first access the snooping devices hanging off the membus, which is the iocache. It has an active address range for the ARM realview test setups I'm using as 0x0-256MB. If the iocache does not satisfy the request it will send the functional access onto the iobus and probably not find a device to satisfy the access thereby sending it to the default port which is effectively a null pointer, crashing the simulation when accessed. The Checker setup does exercise the functional memory access path heavily though. Thinking about it now, I'm surprised this hasn't been hit before in normal operation.
Ali is probably better suited to offer an opinion on a better solution to this as he's pretty familiar with the I/O setup in M5.
Sorry, but I am still not seeing the bigger picture here. A CPU (in this case checker?) makes an access to an address that does not exist in the system? Why would that ever be right? The only reason it would end up at an unconnected default port is if the address is used incorrectly as far as I can tell. Am I missing something?
The Checker makes functional accesses only into the memory system to see if it is seeing the same state as the O3. These functional accesses visit every memory object that could potentially hold the value the access is looking for by doing a breadth first search of the memory hierarchy tree as defined by the busses and stops the search once the request is satisfied. The fix in question is just telling the functional access to stop looking along a search path when no memory object attached to a bus can satisfy its request. So it is not making an access to a non-existent address, but the order it searched the tree of memory objects had the request searching the io devices off the iobus first instead of searching the physmem object which would have satisfied the request in this particular
case.

If an actual access, such as a timing access ends up in this state, it will cause the simulator to panic as it is an actual error.

Does this clarify what this particular piece of code is doing?


- Geoffrey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1694
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-28 10:07:14)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Steve Reinhardt
2011-12-03 20:57:30 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1720
-----------------------------------------------------------


Nice to see this getting resurrected... thanks, Geoff.


build_opts/ARM_wCHECKER_FS
<http://reviews.m5sim.org/r/910/#comment2204>

I'm not too excited about exploding the number of built-in configs further by adding these two files. Just saying USE_CHECKER=1 on the scons command line once (assuming it's sticky) doesn't seem that hard to me.




src/cpu/DummyChecker.py
<http://reviews.m5sim.org/r/910/#comment2206>

What's this DummyChecker object for?



src/cpu/thread_context.hh
<http://reviews.m5sim.org/r/910/#comment2207>

I agree with Gabe here... you're always doing things like:

if (cpu->getCheckerCpuPtr()) {
cpu->getCheckerITBPtr()->foo();
cpu->getCheckerDTBPtr()->foo();
}

so why don't you just do:

CPU *checker = cpu->getCheckerCpuPtr();
if (checker) {
checker->getITBPtr()->foo;
checker->getDTBPtr()->foo;
}

which seems just as clear (if not clearer) and avoids the need for these extra methods.




src/mem/bus.cc
<http://reviews.m5sim.org/r/910/#comment2205>

I agree with Andreas here... from Geoff's description, I think the real bug he's facing is that the ARM config he's using doesn't have a default responder on the I/O bus. There should be something out there that if nothing else sends an error response packet back to the requester. The requester can then decide if it wants to panic and terminate the simulation or not.

If blindly accessing the default port when it's not set is causing us to segfault on a null pointer, then we should require that every bus has a default responder at config time. We have at least one dummy/bad address device just for this purpose.


- Steve
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-28 10:07:14)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Geoffrey Blake
2011-12-06 16:17:13 UTC
Permalink
Post by Geoffrey Blake
build_opts/ARM_wCHECKER_FS, line 1
<http://reviews.m5sim.org/r/910/diff/2/?file=15579#file15579line1>
I'm not too excited about exploding the number of built-in configs further by adding these two files. Just saying USE_CHECKER=1 on the scons command line once (assuming it's sticky) doesn't seem that hard to me.
I'll check to make sure doing USE_CHECKER=1 on the build command line is sticky and remove the extra configs. I was not aware of this functionality in scons.
Post by Geoffrey Blake
src/cpu/DummyChecker.py, line 1
<http://reviews.m5sim.org/r/910/diff/2/?file=15591#file15591line1>
What's this DummyChecker object for?
In short, the DummyChecker allows checkpoints and fast forwarding to work by setting up the ports the real checker will need when it is switched in with the detailed model.
Post by Geoffrey Blake
src/cpu/thread_context.hh, line 127
<http://reviews.m5sim.org/r/910/diff/2/?file=15617#file15617line127>
if (cpu->getCheckerCpuPtr()) {
cpu->getCheckerITBPtr()->foo();
cpu->getCheckerDTBPtr()->foo();
}
CPU *checker = cpu->getCheckerCpuPtr();
if (checker) {
checker->getITBPtr()->foo;
checker->getDTBPtr()->foo;
}
which seems just as clear (if not clearer) and avoids the need for these extra methods.
This wouldn't be too painful of a fix just for the Checker. The checker is a special case.

But as to which is clearer, its a wash in my opinion as everywhere in the code base the ITB/DTB pointers are accessed via the ThreadContext object and not a Cpu object so the Checker will always stand out as being special.
Post by Geoffrey Blake
src/mem/bus.cc, line 480
<http://reviews.m5sim.org/r/910/diff/2/?file=15618#file15618line480>
I agree with Andreas here... from Geoff's description, I think the real bug he's facing is that the ARM config he's using doesn't have a default responder on the I/O bus. There should be something out there that if nothing else sends an error response packet back to the requester. The requester can then decide if it wants to panic and terminate the simulation or not.
If blindly accessing the default port when it's not set is causing us to segfault on a null pointer, then we should require that every bus has a default responder at config time. We have at least one dummy/bad address device just for this purpose.
The error is not that the request is bad. Its the order in which the memory objects are searched and the end conditions for a functional access. Putting a default responder on the IOBus and having it respond to a functional access will just cause the bug to occur from a different place. Would a better solution be putting a default responder that ignores functional accesses on the IOBus so they can pass through to the memory bus? That way if the address is truly bad, the badaddr_responder on the membus will cause the simulation to panic and stop.


- Geoffrey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1720
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-28 10:07:14)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Steve Reinhardt
2011-12-06 17:08:54 UTC
Permalink
Post by Geoffrey Blake
src/cpu/DummyChecker.py, line 1
<http://reviews.m5sim.org/r/910/diff/2/?file=15591#file15591line1>
What's this DummyChecker object for?
In short, the DummyChecker allows checkpoints and fast forwarding to work by setting up the ports the real checker will need when it is switched in with the detailed model.
Can you explain in a little more detail why this is needed? What happens when you try and restore a checkpoint to a system with a checker when you don't have a DummyChecker in there to begin with?
Post by Geoffrey Blake
src/cpu/thread_context.hh, line 127
<http://reviews.m5sim.org/r/910/diff/2/?file=15617#file15617line127>
if (cpu->getCheckerCpuPtr()) {
cpu->getCheckerITBPtr()->foo();
cpu->getCheckerDTBPtr()->foo();
}
CPU *checker = cpu->getCheckerCpuPtr();
if (checker) {
checker->getITBPtr()->foo;
checker->getDTBPtr()->foo;
}
which seems just as clear (if not clearer) and avoids the need for these extra methods.
This wouldn't be too painful of a fix just for the Checker. The checker is a special case.
But as to which is clearer, its a wash in my opinion as everywhere in the code base the ITB/DTB pointers are accessed via the ThreadContext object and not a Cpu object so the Checker will always stand out as being special.
I'm not clear on your comment... I used CPU in the example, but I think you can do exactly the same thing with ThreadContext, right?
Post by Geoffrey Blake
src/mem/bus.cc, line 480
<http://reviews.m5sim.org/r/910/diff/2/?file=15618#file15618line480>
I agree with Andreas here... from Geoff's description, I think the real bug he's facing is that the ARM config he's using doesn't have a default responder on the I/O bus. There should be something out there that if nothing else sends an error response packet back to the requester. The requester can then decide if it wants to panic and terminate the simulation or not.
If blindly accessing the default port when it's not set is causing us to segfault on a null pointer, then we should require that every bus has a default responder at config time. We have at least one dummy/bad address device just for this purpose.
The error is not that the request is bad. Its the order in which the memory objects are searched and the end conditions for a functional access. Putting a default responder on the IOBus and having it respond to a functional access will just cause the bug to occur from a different place. Would a better solution be putting a default responder that ignores functional accesses on the IOBus so they can pass through to the memory bus? That way if the address is truly bad, the badaddr_responder on the membus will cause the simulation to panic and stop.
OK, I think I see the problem... I think the issue is that the IOCache snoops on the membus, and can contain any address, so it always has to be probed; and caches by default forward functional requests they don't satisfy to the other side, so that (for example) a check in an L2 will cause the request to get forwarded up to its corresponding L1(s) if necessary. The trick is that normally in the latter case the L2 is the default responder; the IOCache/IOBus case is special because the IOCache is not the default responder.

If this is the problem, then what you need is to put a default responder on the IOBus, but set its address range to restrict it to the I/O address range, and then set the use_default_range flag on the IOBus. That should prevent the default responder from responding to functional requests that don't map to the I/O bus.


- Steve


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1720
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-28 10:07:14)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Geoffrey Blake
2011-12-06 17:51:59 UTC
Permalink
Post by Geoffrey Blake
src/cpu/DummyChecker.py, line 1
<http://reviews.m5sim.org/r/910/diff/2/?file=15591#file15591line1>
What's this DummyChecker object for?
In short, the DummyChecker allows checkpoints and fast forwarding to work by setting up the ports the real checker will need when it is switched in with the detailed model.
Can you explain in a little more detail why this is needed? What happens when you try and restore a checkpoint to a system with a checker when you don't have a DummyChecker in there to begin with?
There needs to be two checker objects (a dummy and the O3Checker) for the ports to be constructed and connected properly. If the dummy is not there the python code errors out stating there are ports that do not exist. On the call to takeOverFrom(), the dummy is then used to transfer its ports to the actual Checker.

I just checked if I could use the O3Checker object in place of the dummy and it does work (eliminating the extra files and code). But I do not know how desirable it is for O3 cpu code to be present in the simple cpu code tree.


- Geoffrey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1720
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-28 10:07:14)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Steve Reinhardt
2011-12-20 15:17:00 UTC
Permalink
Post by Geoffrey Blake
src/cpu/DummyChecker.py, line 1
<http://reviews.m5sim.org/r/910/diff/2/?file=15591#file15591line1>
What's this DummyChecker object for?
In short, the DummyChecker allows checkpoints and fast forwarding to work by setting up the ports the real checker will need when it is switched in with the detailed model.
Can you explain in a little more detail why this is needed? What happens when you try and restore a checkpoint to a system with a checker when you don't have a DummyChecker in there to begin with?
There needs to be two checker objects (a dummy and the O3Checker) for the ports to be constructed and connected properly. If the dummy is not there the python code errors out stating there are ports that do not exist. On the call to takeOverFrom(), the dummy is then used to transfer its ports to the actual Checker.
I just checked if I could use the O3Checker object in place of the dummy and it does work (eliminating the extra files and code). But I do not know how desirable it is for O3 cpu code to be present in the simple cpu code tree.
This seems pretty hokey to me... I'd think the answer would be to fix the switchover process to make it more robust rather than to add this mechanism with the DummyChecker or wedge O3Checker into the SimpleCPU code. In addition to the general undesirability of having this new object, I'd think that from a practical perspective you'd like to allow people to use the checker on checkpointed runs even when they didn't plan ahead and have a DummyChecker inserted into the original config.

Is there a way we can create these ports on the fly when they don't exist? How does this relate to Andreas's single system-wide ProxyPort change; would that make this problem go away?

Sorry I forgot to send this comment sooner... I've been meaning to look into this myself but haven't had the time.


- Steve


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1720
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Andreas Hansson
2011-12-20 15:44:50 UTC
Permalink
Post by Geoffrey Blake
src/cpu/DummyChecker.py, line 1
<http://reviews.m5sim.org/r/910/diff/2/?file=15591#file15591line1>
What's this DummyChecker object for?
In short, the DummyChecker allows checkpoints and fast forwarding to work by setting up the ports the real checker will need when it is switched in with the detailed model.
Can you explain in a little more detail why this is needed? What happens when you try and restore a checkpoint to a system with a checker when you don't have a DummyChecker in there to begin with?
There needs to be two checker objects (a dummy and the O3Checker) for the ports to be constructed and connected properly. If the dummy is not there the python code errors out stating there are ports that do not exist. On the call to takeOverFrom(), the dummy is then used to transfer its ports to the actual Checker.
I just checked if I could use the O3Checker object in place of the dummy and it does work (eliminating the extra files and code). But I do not know how desirable it is for O3 cpu code to be present in the simple cpu code tree.
This seems pretty hokey to me... I'd think the answer would be to fix the switchover process to make it more robust rather than to add this mechanism with the DummyChecker or wedge O3Checker into the SimpleCPU code. In addition to the general undesirability of having this new object, I'd think that from a practical perspective you'd like to allow people to use the checker on checkpointed runs even when they didn't plan ahead and have a DummyChecker inserted into the original config.
Is there a way we can create these ports on the fly when they don't exist? How does this relate to Andreas's single system-wide ProxyPort change; would that make this problem go away?
Sorry I forgot to send this comment sooner... I've been meaning to look into this myself but haven't had the time.
I believe the port proxy patch will indeed make this issue go away as the checker does not have ports on its own (it merely uses existing ports through proxies).


- Andreas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1720
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Geoffrey Blake
2011-12-20 23:40:57 UTC
Permalink
Post by Geoffrey Blake
src/cpu/DummyChecker.py, line 1
<http://reviews.m5sim.org/r/910/diff/2/?file=15591#file15591line1>
What's this DummyChecker object for?
In short, the DummyChecker allows checkpoints and fast forwarding to work by setting up the ports the real checker will need when it is switched in with the detailed model.
Can you explain in a little more detail why this is needed? What happens when you try and restore a checkpoint to a system with a checker when you don't have a DummyChecker in there to begin with?
There needs to be two checker objects (a dummy and the O3Checker) for the ports to be constructed and connected properly. If the dummy is not there the python code errors out stating there are ports that do not exist. On the call to takeOverFrom(), the dummy is then used to transfer its ports to the actual Checker.
I just checked if I could use the O3Checker object in place of the dummy and it does work (eliminating the extra files and code). But I do not know how desirable it is for O3 cpu code to be present in the simple cpu code tree.
This seems pretty hokey to me... I'd think the answer would be to fix the switchover process to make it more robust rather than to add this mechanism with the DummyChecker or wedge O3Checker into the SimpleCPU code. In addition to the general undesirability of having this new object, I'd think that from a practical perspective you'd like to allow people to use the checker on checkpointed runs even when they didn't plan ahead and have a DummyChecker inserted into the original config.
Is there a way we can create these ports on the fly when they don't exist? How does this relate to Andreas's single system-wide ProxyPort change; would that make this problem go away?
Sorry I forgot to send this comment sooner... I've been meaning to look into this myself but haven't had the time.
I believe the port proxy patch will indeed make this issue go away as the checker does not have ports on its own (it merely uses existing ports through proxies).
If ports could be created on the fly, that would eliminate the need for this object.

I guess I wasn't totally clear on the DummyChecker. It does not need to be present when taking a checkpoint. It only needs to be present when loading from a checkpoint so all the ports get created correctly at the start of simulation.

The checker does have some ports of its own, like the ports to its own TLBs that shadow the CPU object being Checked. But it does piggy back on the CPU's icache and dcache ports. Not sure how that will all fit into the upcoming changes.


- Geoffrey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1720
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Steve Reinhardt
2011-12-03 21:37:20 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1723
-----------------------------------------------------------



src/cpu/base_dyn_inst.hh
<http://reviews.m5sim.org/r/910/#comment2208>

Can we find a way to replace all this stuff with a single templated method? I think if you add some helper methods to Result like this:
union Result {
[...]
void set(uint64_t i) { integer = i; }
void set(double d) { dbl = d; }
void get(uint64_t &i) { i = integer; }
void get(double &d) { d = dbl; }
}

then you could replace these with

template <class T>
T popResult() {
[blah blah]
}

and maybe clean up some of the other existing code as well.



- Steve
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-28 10:07:14)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Geoffrey Blake
2011-12-06 16:17:06 UTC
Permalink
Post by Geoffrey Blake
src/cpu/base_dyn_inst.hh, line 568
<http://reviews.m5sim.org/r/910/diff/2/?file=15594#file15594line568>
union Result {
[...]
void set(uint64_t i) { integer = i; }
void set(double d) { dbl = d; }
void get(uint64_t &i) { i = integer; }
void get(double &d) { d = dbl; }
}
then you could replace these with
template <class T>
T popResult() {
[blah blah]
}
and maybe clean up some of the other existing code as well.
I'll see what I can do with this part of the code to make it cleaner if I can.


- Geoffrey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1723
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-11-28 10:07:14)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
build_opts/ARM_wCHECKER_FS PRE-CREATION
build_opts/ARM_wCHECKER_SE PRE-CREATION
src/arch/arm/isa.cc 62dee0c98d53
src/arch/arm/isa/insts/m5ops.isa 62dee0c98d53
src/arch/arm/isa/insts/misc.isa 62dee0c98d53
src/arch/arm/table_walker.hh 62dee0c98d53
src/arch/arm/table_walker.cc 62dee0c98d53
src/arch/arm/tlb.hh 62dee0c98d53
src/arch/arm/tlb.cc 62dee0c98d53
src/arch/arm/utility.cc 62dee0c98d53
src/cpu/BaseCPU.py 62dee0c98d53
src/cpu/CheckerCPU.py 62dee0c98d53
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 62dee0c98d53
src/cpu/base.cc 62dee0c98d53
src/cpu/base_dyn_inst.hh 62dee0c98d53
src/cpu/base_dyn_inst_impl.hh 62dee0c98d53
src/cpu/checker/cpu.hh 62dee0c98d53
src/cpu/checker/cpu.cc 62dee0c98d53
src/cpu/checker/cpu_impl.hh 62dee0c98d53
src/cpu/checker/thread_context.hh 62dee0c98d53
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 62dee0c98d53
src/cpu/o3/O3Checker.py 62dee0c98d53
src/cpu/o3/checker_builder.cc 62dee0c98d53
src/cpu/o3/commit_impl.hh 62dee0c98d53
src/cpu/o3/cpu.hh 62dee0c98d53
src/cpu/o3/cpu.cc 62dee0c98d53
src/cpu/o3/dyn_inst_impl.hh 62dee0c98d53
src/cpu/o3/fetch_impl.hh 62dee0c98d53
src/cpu/o3/iew_impl.hh 62dee0c98d53
src/cpu/o3/lsq_unit_impl.hh 62dee0c98d53
src/cpu/o3/thread_context.hh 62dee0c98d53
src/cpu/o3/thread_context_impl.hh 62dee0c98d53
src/cpu/simple/BaseSimpleCPU.py 62dee0c98d53
src/cpu/simple/base.hh 62dee0c98d53
src/cpu/simple/base.cc 62dee0c98d53
src/cpu/simple_thread.hh 62dee0c98d53
src/cpu/thread_context.hh 62dee0c98d53
src/mem/bus.cc 62dee0c98d53
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Geoffrey Blake
2011-12-13 21:25:24 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------

(Updated 2011-12-13 13:25:24.163979)


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


Summary
-------

CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5

Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.


Diffs (updated)
-----

configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805

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


Testing
-------

Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.


Thanks,

Geoffrey
Geoffrey Blake
2011-12-13 21:35:40 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1750
-----------------------------------------------------------


Cleaned up the code in base_dyn_inst.hh to use templates. Cleaned up the code in arch/arm/isa.cc to get the ITB/DTB pointers from the CheckerCPU object.

I have implemented a solution for functional accesses that go to the IOBus by adding a "reflect_functional_access" parameter to isa_fake.cc. This parameter allows accesses to pass through if they are detected to be functional via checking if the Packet object can be safely cast to a FunctionalPacket object to identify just functional accesses. I then added this io_device with the reflect param set to the IOBus as a default responder for the ARM setup in configs/common/FSConfig.py. Not sure if this is what is ultimately desired, but it fixes the problem with the Checker's functional accesses without the hack in the bus.cc code. I looked at using the default_range variable as Steve suggested, but saw in bus.cc line 359 that the code would panic if the access did not fit in the default ra
nge. Also would like some suggestions as to how to warn on detecting no default responder attached to the IOBus (any bus), should it be in the Python or C++ code?

- Geoffrey
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Ali Saidi
2011-12-14 22:24:37 UTC
Permalink
Post by Geoffrey Blake
Post by Geoffrey Blake
Cleaned up the code in base_dyn_inst.hh to use templates. Cleaned up the code in arch/arm/isa.cc to get the ITB/DTB pointers from the CheckerCPU object.
I have implemented a solution for functional accesses that go to the IOBus by adding a "reflect_functional_access" parameter to isa_fake.cc. This parameter allows accesses to pass through if they are detected to be functional via checking if the Packet object can be safely cast to a FunctionalPacket object to identify just functional accesses. I then added this io_device with the reflect param set to the IOBus as a default responder for the ARM setup in configs/common/FSConfig.py. Not sure if this is what is ultimately desired, but it fixes the problem with the Checker's functional accesses without the hack in the bus.cc code. I looked at using the default_range variable as Steve suggested, but saw in bus.cc line 359 that the code would panic if the access did not fit in the defaul
t range. Also would like some suggestions as to how to warn on detecting no default responder attached to the IOBus (any bus), should it be in the Python or C++ code?

This solution doesn't work if you've got a PCI config space device attached as the default responder for a certain range on the I/O bus. In this case you'll hit the panic that Geoff described.

Every solution i've come up with seems to fail however, e.g.:
You could say if a cache is top_level then don't forward it on, but this would prevent snooping into an lsq or something similar.

The best I can come up with is that you want to change findPort() to say if this is a request based on a recvFunctional() then return a NO_PORT if a destination isn't found rather than panicing and if NO_PORT is returned then don't call port->sendFunctional() and just do the snoop.

thoughts?


- Ali


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1750
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Ali Saidi
2011-12-16 00:05:03 UTC
Permalink
Post by Ali Saidi
Post by Geoffrey Blake
Post by Geoffrey Blake
Cleaned up the code in base_dyn_inst.hh to use templates. Cleaned
up the code in arch/arm/isa.cc to get the ITB/DTB pointers from the
CheckerCPU object.
Post by Geoffrey Blake
I have implemented a solution for functional accesses that go to
the IOBus by adding a "reflect_functional_access" parameter to
isa_fake.cc. This parameter allows accesses to pass through if they
are detected to be functional via checking if the Packet object can be
safely cast to a FunctionalPacket object to identify just functional
accesses. I then added this io_device with the reflect param set to
the IOBus as a default responder for the ARM setup in
configs/common/FSConfig.py. Not sure if this is what is ultimately
desired, but it fixes the problem with the Checker's functional
accesses without the hack in the bus.cc code. I looked at using the
default_range variable as Steve suggested, but saw in bus.cc line 359
that the code would panic if the access did not fit in the default
range. Also would like some suggestions as to how to warn on
detecting no default responder attached to the IOBus (any bus), should
it be in the Python or C++ code?
This solution doesn't work if you've got a PCI config space device
attached as the default responder for a certain range on the I/O bus.
In this case you'll hit the panic that Geoff described.
You could say if a cache is top_level then don't forward it on, but
this would prevent snooping into an lsq or something similar.
The best I can come up with is that you want to change findPort() to
say if this is a request based on a recvFunctional() then return a
NO_PORT if a destination isn't found rather than panicing and if
NO_PORT is returned then don't call port->sendFunctional() and just
do
the snoop.
thoughts?
bump.

Ali
Gabriel Michael Black
2011-12-16 08:36:33 UTC
Permalink
Post by Ali Saidi
Post by Geoffrey Blake
Cleaned up the code in base_dyn_inst.hh to use templates.
Cleaned up the code in arch/arm/isa.cc to get the ITB/DTB
pointers from the CheckerCPU object.
I have implemented a solution for functional accesses that go to
the IOBus by adding a "reflect_functional_access" parameter to
isa_fake.cc. This parameter allows accesses to pass through if
they are detected to be functional via checking if the Packet
object can be safely cast to a FunctionalPacket object to
identify just functional accesses. I then added this io_device
with the reflect param set to the IOBus as a default responder
for the ARM setup in configs/common/FSConfig.py. Not sure if
this is what is ultimately desired, but it fixes the problem with
the Checker's functional accesses without the hack in the bus.cc
code. I looked at using the default_range variable as Steve
suggested, but saw in bus.cc line 359 that the code would panic
if the access did not fit in the default range. Also would like
some suggestions as to how to warn on detecting no default
responder attached to the IOBus (any bus), should it be in the
Python or C++ code?
This solution doesn't work if you've got a PCI config space device
attached as the default responder for a certain range on the I/O bus.
In this case you'll hit the panic that Geoff described.
You could say if a cache is top_level then don't forward it on, but
this would prevent snooping into an lsq or something similar.
The best I can come up with is that you want to change findPort() to
say if this is a request based on a recvFunctional() then return a
NO_PORT if a destination isn't found rather than panicing and if
NO_PORT is returned then don't call port->sendFunctional() and just do
the snoop.
thoughts?
bump.
Ali
_______________________________________________
gem5-dev mailing list
http://m5sim.org/mailman/listinfo/gem5-dev
Sorry, I've been busy/lazy lately. I'll try to look at it this
weekend, but we have some family things both days. I do intend to look
at it soonish.

Gabe
Steve Reinhardt
2011-12-16 18:17:36 UTC
Permalink
Post by Ali Saidi
Post by Geoffrey Blake
Cleaned up the code in base_dyn_inst.hh to use templates. Cleaned up the code in arch/arm/isa.cc to get the ITB/DTB pointers from the CheckerCPU object.
I have implemented a solution for functional accesses that go to the IOBus by adding a "reflect_functional_access" parameter to isa_fake.cc. This parameter allows accesses to pass through if they are detected to be functional via checking if the Packet object can be safely cast to a FunctionalPacket object to identify just functional accesses. I then added this io_device with the reflect param set to the IOBus as a default responder for the ARM setup in configs/common/FSConfig.py. Not sure if this is what is ultimately desired, but it fixes the problem with the Checker's functional accesses without the hack in the bus.cc code. I looked at using the default_range variable as Steve suggested, but saw in bus.cc line 359 that the code would panic if the access did not fit in the defaul
t range. Also would like some suggestions as to how to warn on detecting no default responder attached to the IOBus (any bus), should it be in the Python or C++ code?
Post by Ali Saidi
This solution doesn't work if you've got a PCI config space device attached as the default responder for a certain range on the I/O bus. In this case you'll hit the panic that Geoff described.
You could say if a cache is top_level then don't forward it on, but this would prevent snooping into an lsq or something similar.
The best I can come up with is that you want to change findPort() to say if this is a request based on a recvFunctional() then return a NO_PORT if a destination isn't found rather than panicing and if NO_PORT is returned then don't call port->sendFunctional() and just do the snoop.
thoughts?
What if we just have Cache<TagStore>::functionalAccess() check forwardSnoops before forwarding functional accesses from the memSidePort to the cpuSidePort? I was trying to figure out why this is a problem with functional accesses but not with other accesses, and I think that's the key: other accesses use the forwardSnoops flag to suppress this path.

It'll require a little clever coding since functionalAccess() is currently agnostic to which direction you're headed. I suggest just adding a bool param like 'forward' to functionalAccess() and then setting it to true in CpuSidePort::recvFunctional() and setting it to mycache()->forwardSnoops in MemSidePort::recvFunctional(). Or you could just pass in true from one and false from the other, and or it with forwardSnoops in functionalAccess().

If there's some subtlety I'm missing that makes this not work, then I think Ali's general idea of tweaking bus.cc to not panic is a good plan B.


- Steve


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1750
-----------------------------------------------------------
Post by Ali Saidi
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Andreas Hansson
2011-12-16 18:24:14 UTC
Permalink
I propose we opt for Steve's suggestion as an interim solution, and in our sandbox with the memory-system changes (half way along our implementation) this is exactly what was done. The functionalAccess does not take an incoming/otherside port, but rather a Boolean fromCPUSide and then uses the members to determine where to forward (and forwardSnoops to determine if it should do it or not).

As a side note, in the "completed" memory-system updates the problem goes away as the snooping vs normal access are separated.

Andreas


-----Original Message-----
From: gem5-dev-bounces-1Gs4CP2/***@public.gmane.org [mailto:gem5-dev-bounces-1Gs4CP2/***@public.gmane.org] On Behalf Of Steve Reinhardt
Sent: 16 December 2011 18:18
To: Steve Reinhardt; Ali Saidi; Gabe Black; Nathan Binkert
Cc: Default
Subject: Re: [gem5-dev] Review Request: CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Post by Ali Saidi
Post by Geoffrey Blake
Cleaned up the code in base_dyn_inst.hh to use templates. Cleaned up the code in arch/arm/isa.cc to get the ITB/DTB pointers from the CheckerCPU object.
I have implemented a solution for functional accesses that go to the IOBus by adding a "reflect_functional_access" parameter to isa_fake.cc. This parameter allows accesses to pass through if they are detected to be functional via checking if the Packet object can be safely cast to a FunctionalPacket object to identify just functional accesses. I then added this io_device with the reflect param set to the IOBus as a default responder for the ARM setup in configs/common/FSConfig.py. Not sure if this is what is ultimately desired, but it fixes the problem with the Checker's functional accesses without the hack in the bus.cc code. I looked at using the default_range variable as Steve suggested, but saw in bus.cc line 359 that the code would panic if the access did not fit in the defaul
t range. Also would like some suggestions as to how to warn on detecting no default responder attached to the IOBus (any bus), should it be in the Python or C++ code?
Post by Ali Saidi
This solution doesn't work if you've got a PCI config space device attached as the default responder for a certain range on the I/O bus. In this case you'll hit the panic that Geoff described.
You could say if a cache is top_level then don't forward it on, but this would prevent snooping into an lsq or something similar.
The best I can come up with is that you want to change findPort() to say if this is a request based on a recvFunctional() then return a NO_PORT if a destination isn't found rather than panicing and if NO_PORT is returned then don't call port->sendFunctional() and just do the snoop.
thoughts?
What if we just have Cache<TagStore>::functionalAccess() check forwardSnoops before forwarding functional accesses from the memSidePort to the cpuSidePort? I was trying to figure out why this is a problem with functional accesses but not with other accesses, and I think that's the key: other accesses use the forwardSnoops flag to suppress this path.

It'll require a little clever coding since functionalAccess() is currently agnostic to which direction you're headed. I suggest just adding a bool param like 'forward' to functionalAccess() and then setting it to true in CpuSidePort::recvFunctional() and setting it to mycache()->forwardSnoops in MemSidePort::recvFunctional(). Or you could just pass in true from one and false from the other, and or it with forwardSnoops in functionalAccess().

If there's some subtlety I'm missing that makes this not work, then I think Ali's general idea of tweaking bus.cc to not panic is a good plan B.


- Steve


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1750
-----------------------------------------------------------
Post by Ali Saidi
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
_______________________________________________
gem5-dev mailing list
gem5-dev-1Gs4CP2/***@public.gmane.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.
Geoffrey Blake
2011-12-16 23:47:55 UTC
Permalink
I think this method of putting extra flags in the caches and trying to
make them determine whether or not it should forward a functional
request is overly complicated. I'm partial to letting the bus decide
if it can go forward as it is a much simpler solution. I was given a
quick primer on what Andreas will replace functional accesses with,
and I figure this solution can be considered a stop-gap for now until
the new mem system is pushed.

Are there any additional comments with the code in the CheckerCPU
module as it stands now?

Geoff

On Fri, Dec 16, 2011 at 12:24 PM, Andreas Hansson
Post by Andreas Hansson
I propose we opt for Steve's suggestion as an interim solution, and in our sandbox with the memory-system changes (half way along our implementation) this is exactly what was done. The functionalAccess does not take an incoming/otherside port, but rather a Boolean fromCPUSide and then uses the members to determine where to forward (and forwardSnoops to determine if it should do it or not).
As a side note, in the "completed" memory-system updates the problem goes away as the snooping vs normal access are separated.
Andreas
-----Original Message-----
Sent: 16 December 2011 18:18
To: Steve Reinhardt; Ali Saidi; Gabe Black; Nathan Binkert
Cc: Default
Subject: Re: [gem5-dev] Review Request: CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Cleaned up the code in base_dyn_inst.hh to use templates.  Cleaned up the code in arch/arm/isa.cc to get the ITB/DTB pointers from the CheckerCPU object.
I have implemented a solution for functional accesses that go to the IOBus by adding a "reflect_functional_access" parameter to isa_fake.cc.  This parameter allows accesses to pass through if they are detected to be functional via checking if the Packet object can be safely cast to a FunctionalPacket object to identify just functional accesses.  I then added this io_device with the reflect param set to the IOBus as a default responder for the ARM setup in configs/common/FSConfig.py.  Not sure if this is what is ultimately desired, but it fixes the problem with the Checker's functional accesses without the hack in the bus.cc code.  I looked at using the default_range variable as Steve suggested, but saw in bus.cc line 359 that the code would panic if the access did not fit in the default range.  Also would like some suggestions as to how to warn on detecting no default responder attached to the IOBus (any bus), should it be in the Python or C++ code?
    This solution doesn't work if you've got a PCI config space device attached as the default responder for a certain range on the I/O bus. In this case you'll hit the panic that Geoff described.
    You could say if a cache is top_level then don't forward it on, but this would prevent snooping into an lsq or something similar.
    The best I can come up with is that you want to change findPort() to say if this is a request based on a recvFunctional() then return a NO_PORT if a destination isn't found rather than panicing and if NO_PORT is returned then don't call port->sendFunctional() and just do the snoop.
    thoughts?
What if we just have Cache<TagStore>::functionalAccess() check forwardSnoops before forwarding functional accesses from the memSidePort to the cpuSidePort?  I was trying to figure out why this is a problem with functional accesses but not with other accesses, and I think that's the key: other accesses use the forwardSnoops flag to suppress this path.
It'll require a little clever coding since functionalAccess() is currently agnostic to which direction you're headed.  I suggest just adding a bool param like 'forward' to functionalAccess() and then setting it to true in CpuSidePort::recvFunctional() and setting it to mycache()->forwardSnoops in MemSidePort::recvFunctional().  Or you could just pass in true from one and false from the other, and or it with forwardSnoops in functionalAccess().
If there's some subtlety I'm missing that makes this not work, then I think Ali's general idea of tweaking bus.cc to not panic is a good plan B.
- Steve
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/#review1750
-----------------------------------------------------------
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA.  Other ISAs will potentially require modification.
Diffs
-----
  configs/common/FSConfig.py c1ab57ea8805
  src/arch/arm/isa.cc c1ab57ea8805
  src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
  src/arch/arm/isa/insts/misc.isa c1ab57ea8805
  src/arch/arm/table_walker.hh c1ab57ea8805
  src/arch/arm/table_walker.cc c1ab57ea8805
  src/arch/arm/tlb.hh c1ab57ea8805
  src/arch/arm/tlb.cc c1ab57ea8805
  src/arch/arm/utility.cc c1ab57ea8805
  src/cpu/BaseCPU.py c1ab57ea8805
  src/cpu/CheckerCPU.py c1ab57ea8805
  src/cpu/DummyChecker.py PRE-CREATION
  src/cpu/SConscript c1ab57ea8805
  src/cpu/base.cc c1ab57ea8805
  src/cpu/base_dyn_inst.hh c1ab57ea8805
  src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
  src/cpu/checker/cpu.hh c1ab57ea8805
  src/cpu/checker/cpu.cc c1ab57ea8805
  src/cpu/checker/cpu_impl.hh c1ab57ea8805
  src/cpu/checker/thread_context.hh c1ab57ea8805
  src/cpu/dummy_checker_builder.cc PRE-CREATION
  src/cpu/o3/O3CPU.py c1ab57ea8805
  src/cpu/o3/O3Checker.py c1ab57ea8805
  src/cpu/o3/checker_builder.cc c1ab57ea8805
  src/cpu/o3/commit_impl.hh c1ab57ea8805
  src/cpu/o3/cpu.hh c1ab57ea8805
  src/cpu/o3/cpu.cc c1ab57ea8805
  src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
  src/cpu/o3/fetch_impl.hh c1ab57ea8805
  src/cpu/o3/iew_impl.hh c1ab57ea8805
  src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
  src/cpu/o3/thread_context.hh c1ab57ea8805
  src/cpu/o3/thread_context_impl.hh c1ab57ea8805
  src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
  src/cpu/simple/base.hh c1ab57ea8805
  src/cpu/simple/base.cc c1ab57ea8805
  src/cpu/simple_thread.hh c1ab57ea8805
  src/cpu/thread_context.hh c1ab57ea8805
  src/dev/Device.py c1ab57ea8805
  src/dev/isa_fake.hh c1ab57ea8805
  src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode.  Successfully boots linux kernel in FS mode.  Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
_______________________________________________
gem5-dev mailing list
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.
_______________________________________________
gem5-dev mailing list
http://m5sim.org/mailman/listinfo/gem5-dev
Steve Reinhardt
2011-12-17 02:09:08 UTC
Permalink
Hi Geoff,

The flag is already there, is already set appropriately by the config
scripts, and is already used for basically the same purpose for
non-functional requests. All that's needed is to test that flag on
functional accesses too. Maybe I made it sound more complicated than it
really is... I'm guessing it's about a five-line change total.

I know it's extra work to make the change and test it, and that it's moot
in the long run, but I feel like this is actually the right solution, so
I'd much prefer to see it go in, especially since we don't know exactly how
soon all of Andreas's changes will get committed.

Thanks,

Steve
Post by Geoffrey Blake
I think this method of putting extra flags in the caches and trying to
make them determine whether or not it should forward a functional
request is overly complicated. I'm partial to letting the bus decide
if it can go forward as it is a much simpler solution. I was given a
quick primer on what Andreas will replace functional accesses with,
and I figure this solution can be considered a stop-gap for now until
the new mem system is pushed.
Are there any additional comments with the code in the CheckerCPU
module as it stands now?
Geoff
On Fri, Dec 16, 2011 at 12:24 PM, Andreas Hansson
Post by Andreas Hansson
I propose we opt for Steve's suggestion as an interim solution, and in
our sandbox with the memory-system changes (half way along our
implementation) this is exactly what was done. The functionalAccess does
not take an incoming/otherside port, but rather a Boolean fromCPUSide and
then uses the members to determine where to forward (and forwardSnoops to
determine if it should do it or not).
Post by Andreas Hansson
As a side note, in the "completed" memory-system updates the problem
goes away as the snooping vs normal access are separated.
Post by Andreas Hansson
Andreas
-----Original Message-----
Behalf Of Steve Reinhardt
Post by Andreas Hansson
Sent: 16 December 2011 18:18
To: Steve Reinhardt; Ali Saidi; Gabe Black; Nathan Binkert
Cc: Default
Subject: Re: [gem5-dev] Review Request: CheckerCPU: Re-factor CheckerCPU
to be compatible with current gem5
Post by Andreas Hansson
Post by Ali Saidi
Post by Geoffrey Blake
Cleaned up the code in base_dyn_inst.hh to use templates. Cleaned up
the code in arch/arm/isa.cc to get the ITB/DTB pointers from the CheckerCPU
object.
Post by Andreas Hansson
Post by Ali Saidi
Post by Geoffrey Blake
I have implemented a solution for functional accesses that go to the
IOBus by adding a "reflect_functional_access" parameter to isa_fake.cc.
This parameter allows accesses to pass through if they are detected to be
functional via checking if the Packet object can be safely cast to a
FunctionalPacket object to identify just functional accesses. I then added
this io_device with the reflect param set to the IOBus as a default
responder for the ARM setup in configs/common/FSConfig.py. Not sure if
this is what is ultimately desired, but it fixes the problem with the
Checker's functional accesses without the hack in the bus.cc code. I
looked at using the default_range variable as Steve suggested, but saw in
bus.cc line 359 that the code would panic if the access did not fit in the
default range. Also would like some suggestions as to how to warn on
detecting no default responder attached to the IOBus (any bus), should it
be in the Python or C++ code?
Post by Andreas Hansson
Post by Ali Saidi
This solution doesn't work if you've got a PCI config space device
attached as the default responder for a certain range on the I/O bus. In
this case you'll hit the panic that Geoff described.
Post by Andreas Hansson
Post by Ali Saidi
You could say if a cache is top_level then don't forward it on, but
this would prevent snooping into an lsq or something similar.
Post by Andreas Hansson
Post by Ali Saidi
The best I can come up with is that you want to change findPort()
to say if this is a request based on a recvFunctional() then return a
NO_PORT if a destination isn't found rather than panicing and if NO_PORT is
returned then don't call port->sendFunctional() and just do the snoop.
Post by Andreas Hansson
Post by Ali Saidi
thoughts?
What if we just have Cache<TagStore>::functionalAccess() check
forwardSnoops before forwarding functional accesses from the memSidePort to
the cpuSidePort? I was trying to figure out why this is a problem with
functional accesses but not with other accesses, and I think that's the
key: other accesses use the forwardSnoops flag to suppress this path.
Post by Andreas Hansson
It'll require a little clever coding since functionalAccess() is
currently agnostic to which direction you're headed. I suggest just adding
a bool param like 'forward' to functionalAccess() and then setting it to
true in CpuSidePort::recvFunctional() and setting it to
mycache()->forwardSnoops in MemSidePort::recvFunctional(). Or you could
just pass in true from one and false from the other, and or it with
forwardSnoops in functionalAccess().
Post by Andreas Hansson
If there's some subtlety I'm missing that makes this not work, then I
think Ali's general idea of tweaking bus.cc to not panic is a good plan B.
Post by Andreas Hansson
- Steve
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/#review1750
-----------------------------------------------------------
Post by Ali Saidi
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and
Nathan Binkert.
Post by Andreas Hansson
Post by Ali Saidi
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE
mode
Post by Andreas Hansson
Post by Ali Saidi
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in
FS mode. Works with checkpoints and fast-forwarding. Testing with buggy
O3CPUs to test checker's capabilities.
Post by Andreas Hansson
Post by Ali Saidi
Thanks,
Geoffrey
_______________________________________________
gem5-dev mailing list
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.
Post by Andreas Hansson
_______________________________________________
gem5-dev mailing list
http://m5sim.org/mailman/listinfo/gem5-dev
_______________________________________________
gem5-dev mailing list
http://m5sim.org/mailman/listinfo/gem5-dev
Gabriel Michael Black
2011-12-17 20:09:17 UTC
Permalink
Post by Geoffrey Blake
Are there any additional comments with the code in the CheckerCPU
module as it stands now?
As I said not that long ago, I'm going to look at it soon. Wait.
Post by Geoffrey Blake
Geoff
On Fri, Dec 16, 2011 at 12:24 PM, Andreas Hansson
Post by Andreas Hansson
I propose we opt for Steve's suggestion as an interim solution, and
in our sandbox with the memory-system changes (half way along our
implementation) this is exactly what was done. The functionalAccess
does not take an incoming/otherside port, but rather a Boolean
fromCPUSide and then uses the members to determine where to forward
(and forwardSnoops to determine if it should do it or not).
As a side note, in the "completed" memory-system updates the
problem goes away as the snooping vs normal access are separated.
Andreas
-----Original Message-----
On Behalf Of Steve Reinhardt
Sent: 16 December 2011 18:18
To: Steve Reinhardt; Ali Saidi; Gabe Black; Nathan Binkert
Cc: Default
Subject: Re: [gem5-dev] Review Request: CheckerCPU: Re-factor
CheckerCPU to be compatible with current gem5
Post by Geoffrey Blake
Cleaned up the code in base_dyn_inst.hh to use templates.
 Cleaned up the code in arch/arm/isa.cc to get the ITB/DTB
pointers from the CheckerCPU object.
Post by Geoffrey Blake
I have implemented a solution for functional accesses that go to
the IOBus by adding a "reflect_functional_access" parameter to
isa_fake.cc.  This parameter allows accesses to pass through if
they are detected to be functional via checking if the Packet
object can be safely cast to a FunctionalPacket object to identify
just functional accesses.  I then added this io_device with the
reflect param set to the IOBus as a default responder for the ARM
setup in configs/common/FSConfig.py.  Not sure if this is what is
ultimately desired, but it fixes the problem with the Checker's
functional accesses without the hack in the bus.cc code.  I looked
at using the default_range variable as Steve suggested, but saw in
bus.cc line 359 that the code would panic if the access did not
fit in the default range.  Also would like some suggestions as to
how to warn on detecting no default responder attached to the
IOBus (any bus), should it be in the Python or C++ code?
    This solution doesn't work if you've got a PCI config space
device attached as the default responder for a certain range on
the I/O bus. In this case you'll hit the panic that Geoff described.
    You could say if a cache is top_level then don't forward it
on, but this would prevent snooping into an lsq or something
similar.
    The best I can come up with is that you want to change
findPort() to say if this is a request based on a recvFunctional()
then return a NO_PORT if a destination isn't found rather than
panicing and if NO_PORT is returned then don't call
port->sendFunctional() and just do the snoop.
    thoughts?
What if we just have Cache<TagStore>::functionalAccess() check
forwardSnoops before forwarding functional accesses from the
memSidePort to the cpuSidePort?  I was trying to figure out why
this is a problem with functional accesses but not with other
accesses, and I think that's the key: other accesses use the
forwardSnoops flag to suppress this path.
It'll require a little clever coding since functionalAccess() is
currently agnostic to which direction you're headed.  I suggest
just adding a bool param like 'forward' to functionalAccess() and
then setting it to true in CpuSidePort::recvFunctional() and
setting it to mycache()->forwardSnoops in
MemSidePort::recvFunctional().  Or you could just pass in true from
one and false from the other, and or it with forwardSnoops in
functionalAccess().
If there's some subtlety I'm missing that makes this not work, then
I think Ali's general idea of tweaking bus.cc to not panic is a
good plan B.
- Steve
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/#review1750
-----------------------------------------------------------
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve
Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA.  Other ISAs will potentially require modification.
Diffs
-----
  configs/common/FSConfig.py c1ab57ea8805
  src/arch/arm/isa.cc c1ab57ea8805
  src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
  src/arch/arm/isa/insts/misc.isa c1ab57ea8805
  src/arch/arm/table_walker.hh c1ab57ea8805
  src/arch/arm/table_walker.cc c1ab57ea8805
  src/arch/arm/tlb.hh c1ab57ea8805
  src/arch/arm/tlb.cc c1ab57ea8805
  src/arch/arm/utility.cc c1ab57ea8805
  src/cpu/BaseCPU.py c1ab57ea8805
  src/cpu/CheckerCPU.py c1ab57ea8805
  src/cpu/DummyChecker.py PRE-CREATION
  src/cpu/SConscript c1ab57ea8805
  src/cpu/base.cc c1ab57ea8805
  src/cpu/base_dyn_inst.hh c1ab57ea8805
  src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
  src/cpu/checker/cpu.hh c1ab57ea8805
  src/cpu/checker/cpu.cc c1ab57ea8805
  src/cpu/checker/cpu_impl.hh c1ab57ea8805
  src/cpu/checker/thread_context.hh c1ab57ea8805
  src/cpu/dummy_checker_builder.cc PRE-CREATION
  src/cpu/o3/O3CPU.py c1ab57ea8805
  src/cpu/o3/O3Checker.py c1ab57ea8805
  src/cpu/o3/checker_builder.cc c1ab57ea8805
  src/cpu/o3/commit_impl.hh c1ab57ea8805
  src/cpu/o3/cpu.hh c1ab57ea8805
  src/cpu/o3/cpu.cc c1ab57ea8805
  src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
  src/cpu/o3/fetch_impl.hh c1ab57ea8805
  src/cpu/o3/iew_impl.hh c1ab57ea8805
  src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
  src/cpu/o3/thread_context.hh c1ab57ea8805
  src/cpu/o3/thread_context_impl.hh c1ab57ea8805
  src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
  src/cpu/simple/base.hh c1ab57ea8805
  src/cpu/simple/base.cc c1ab57ea8805
  src/cpu/simple_thread.hh c1ab57ea8805
  src/cpu/thread_context.hh c1ab57ea8805
  src/dev/Device.py c1ab57ea8805
  src/dev/isa_fake.hh c1ab57ea8805
  src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode.  Successfully boots linux
kernel in FS mode.  Works with checkpoints and fast-forwarding.
Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
_______________________________________________
gem5-dev mailing list
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.
_______________________________________________
gem5-dev mailing list
http://m5sim.org/mailman/listinfo/gem5-dev
_______________________________________________
gem5-dev mailing list
http://m5sim.org/mailman/listinfo/gem5-dev
Gabe Black
2011-12-20 09:16:58 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1764
-----------------------------------------------------------


This is converging to where it needs to be. There are style problems thinly scattered throughout so you'll need to fix those, and there are some smaller tweaks I'd like to see. I'm surprised to see indentation issues here and there. You should talk to Ali about setting up vim/emacs to do gem5 style automatically and then you won't have to worry about that. Other more knowledgeable people are already discussing the devices/functional access thing you're doing, so I'll just ignore that.


src/arch/arm/isa/insts/misc.isa
<http://reviews.m5sim.org/r/910/#comment2274>

This line is too long. I won't point out the others that are as well, but please fix throughout.



src/arch/arm/tlb.cc
<http://reviews.m5sim.org/r/910/#comment2272>

What's this for? I don't see any new code in this file that would need this.



src/arch/arm/tlb.cc
<http://reviews.m5sim.org/r/910/#comment2273>

Bad indentation, you don't need the {}s, and you could move it all into the assert.

assert(!(timing && functional));

At least fix the indentation, preferably just replace it with this new version.



src/cpu/BaseCPU.py
<http://reviews.m5sim.org/r/910/#comment2275>

Yeah, this assert always seemed a bit hacky to me.



src/cpu/CheckerCPU.py
<http://reviews.m5sim.org/r/910/#comment2276>

What's your reasoning for changing this? Are you worried that this is likely to happen but not signify a real bug? Have you actually seen that happen?



src/cpu/DummyChecker.py
<http://reviews.m5sim.org/r/910/#comment2277>

So much copyright for so little code :-)



src/cpu/base.cc
<http://reviews.m5sim.org/r/910/#comment2278>

It would be better to move these declarations down to where the variables are used. That way you can also merge the #if blocks.



src/cpu/base.cc
<http://reviews.m5sim.org/r/910/#comment2279>

Have you tested how the checker acts when you switch CPUs? It wouldn't be criminal if that isn't working perfectly at this stage since this change already fixing alot, but it would be nice if that worked.



src/cpu/base_dyn_inst.hh
<http://reviews.m5sim.org/r/910/#comment2280>

Bad indentation.



src/cpu/checker/cpu.cc
<http://reviews.m5sim.org/r/910/#comment2281>

A lot of this could be moved to the initializer list. It's ok to leave it here, though, since this isn't performance sensitive.



src/cpu/checker/cpu.cc
<http://reviews.m5sim.org/r/910/#comment2282>

Drop the {}s.



src/cpu/checker/cpu.cc
<http://reviews.m5sim.org/r/910/#comment2283>

I think you're only indented 3 spaces here instead of 4.



src/cpu/checker/cpu.cc
<http://reviews.m5sim.org/r/910/#comment2284>

It's really only going to rot, and it'll be in version control anyway. I'd leave a comment saying you deleted it and just get rid of it.



src/cpu/checker/cpu.cc
<http://reviews.m5sim.org/r/910/#comment2285>

Bracket should be on the preceding line.



src/cpu/checker/cpu.cc
<http://reviews.m5sim.org/r/910/#comment2286>

No {}s, indentation.



src/cpu/checker/cpu.cc
<http://reviews.m5sim.org/r/910/#comment2288>

spaces after ,



src/cpu/checker/cpu.cc
<http://reviews.m5sim.org/r/910/#comment2287>

Using ?: here makes this if harder to understand. You should declare a temporary right above this if, maybe extraDataOk. Using the ?: there would be ok since that would be simpler to start with.



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2290>

No offset involved...



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2289>

It would be better to call this handlePendingInt since it's a more ISA neutral term and is more consistent with the rest of gem5.



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2291>

Drop the {}s.



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2292>

You should declare a temporary which has the inst to check and then have one if and one panic which checks it. That gets rid of the redundancy here.



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2293>

This line is gigantic. Wrap it, and/or use a temporary.



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2294>

Drop the {}s, result.pop(); on the next line.



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2295>

ckFault => checkFault. It's not very long and there's no reason to be cryptic.



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2296>

Not here you aren't...



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2297>

Delete instead of commenting it out. I know the code you copied it from had this commented out and eventually I want to fix this, but there's no reason to propagate the problem.



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2298>

If the checker CPU is going to be able to trace things, it should have a tracer parameter. If not, pass NULL in as the tracer argument to execute. I'm 95% sure that will work, and if not you should be able to tell pretty quickly.



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2299>

I'm not sure it makes sense for the checker CPU to recognize PC based events since the main CPU is going to do that. It may, though.



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2301>

Bad indentation.



src/cpu/checker/cpu_impl.hh
<http://reviews.m5sim.org/r/910/#comment2302>

Could you clarify what you mean here? The PC mapping to an integer register should be handled by the instructions and the decoder, although I can believe there may be subtle side effects.

Also, vagarities => vagaries.



src/cpu/o3/O3Checker.py
<http://reviews.m5sim.org/r/910/#comment2303>

I remember seeing this line before so I may have already asked, but why are you changing this?



src/cpu/o3/commit_impl.hh
<http://reviews.m5sim.org/r/910/#comment2304>

Drop the {}s, and I already mentioned this function's name earlier.



src/cpu/o3/dyn_inst_impl.hh
<http://reviews.m5sim.org/r/910/#comment2305>

{}s



src/cpu/o3/fetch_impl.hh
<http://reviews.m5sim.org/r/910/#comment2306>

What are these for? If you added these for a reason please say so, but otherwise be careful not to add dead includes.



src/cpu/o3/thread_context_impl.hh
<http://reviews.m5sim.org/r/910/#comment2307>

Drop the {}s. I'll stop pointing these out (and I may have missed some), but please fix throughout.


- Gabe
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Steve Reinhardt
2011-12-20 15:11:02 UTC
Permalink
Post by Geoffrey Blake
src/cpu/checker/cpu.cc, line 332
<http://reviews.m5sim.org/r/910/diff/3/?file=16278#file16278line332>
No {}s, indentation.
I know we just had this discussion recently... as I said then, I distinctly recall a period in time where we decided that we should *always* have braces even around single-statement blocks, for consistency and to make it easier to add new lines to the block (even temporarily, e.g. for debugging) without having to add the braces. Obviously that policy didn't make it into the official style guide, and I was always somewhat ambivalent about it anyway, so I'm not trying to revive it. However, it's equally true that the converse of that policy is not in the style guide, i.e., there's no requirement to *not* have braces around single-statement blocks either. So IMO Geoff should be free to disregard those specific comments of yours. (Though there are a few places where the brace does need to
be moved to the preceding line if it's not deleted.)

If we want to have a discussion about whether or not we should amend the style guide to specifically cover single-statement blocks, we should do that in a separate thread. I'm fine with leaving it at the coder's discretion though.


- Steve


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1764
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Gabe Black
2011-12-20 21:32:03 UTC
Permalink
Post by Steve Reinhardt
src/cpu/checker/cpu.cc, line 332
<http://reviews.m5sim.org/r/910/diff/3/?file=16278#file16278line332>
No {}s, indentation.
I know we just had this discussion recently... as I said then, I distinctly recall a period in time where we decided that we should *always* have braces even around single-statement blocks, for consistency and to make it easier to add new lines to the block (even temporarily, e.g. for debugging) without having to add the braces. Obviously that policy didn't make it into the official style guide, and I was always somewhat ambivalent about it anyway, so I'm not trying to revive it. However, it's equally true that the converse of that policy is not in the style guide, i.e., there's no requirement to *not* have braces around single-statement blocks either. So IMO Geoff should be free to disregard those specific comments of yours. (Though there are a few places where the brace does ne
ed to be moved to the preceding line if it's not deleted.)
Post by Steve Reinhardt
If we want to have a discussion about whether or not we should amend the style guide to specifically cover single-statement blocks, we should do that in a separate thread. I'm fine with leaving it at the coder's discretion though.
We did, and then too I said that's not how I remember it. I can't find those emails, but we were talking about insisting the brackets were *not* used except when the code wouldn't fit on a single line. My position was that it shouldn't be mandatory to drop the brackets, and there didn't seem to be opposition to that.

As far as what we do now, I'm still fine with leaving it at the coders discretion and it isn't in the style guide. Geoff wouldn't *have* to remove the {}s, but I think it looks better.


- Gabe


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1764
-----------------------------------------------------------
Post by Steve Reinhardt
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Steve Reinhardt
2011-12-20 22:41:43 UTC
Permalink
Post by Steve Reinhardt
src/cpu/checker/cpu.cc, line 332
<http://reviews.m5sim.org/r/910/diff/3/?file=16278#file16278line332>
No {}s, indentation.
I know we just had this discussion recently... as I said then, I distinctly recall a period in time where we decided that we should *always* have braces even around single-statement blocks, for consistency and to make it easier to add new lines to the block (even temporarily, e.g. for debugging) without having to add the braces. Obviously that policy didn't make it into the official style guide, and I was always somewhat ambivalent about it anyway, so I'm not trying to revive it. However, it's equally true that the converse of that policy is not in the style guide, i.e., there's no requirement to *not* have braces around single-statement blocks either. So IMO Geoff should be free to disregard those specific comments of yours. (Though there are a few places where the brace does ne
ed to be moved to the preceding line if it's not deleted.)
Post by Steve Reinhardt
If we want to have a discussion about whether or not we should amend the style guide to specifically cover single-statement blocks, we should do that in a separate thread. I'm fine with leaving it at the coder's discretion though.
We did, and then too I said that's not how I remember it. I can't find those emails, but we were talking about insisting the brackets were *not* used except when the code wouldn't fit on a single line. My position was that it shouldn't be mandatory to drop the brackets, and there didn't seem to be opposition to that.
As far as what we do now, I'm still fine with leaving it at the coders discretion and it isn't in the style guide. Geoff wouldn't *have* to remove the {}s, but I think it looks better.
I'm not surprised you don't remember the original policy, since I believe it was before your time. Through the magic of gmail I found the thread where we originally decided to require braces on single-line blocks: Nate and Steve Raasch were opposed, Erik and Lisa were in favor, and I was in the middle, but leaned enough toward Erik & Lisa to be the deciding factor. This was September 12, 2003. Apparently none of our on-line list archives go back that far anymore.

Apparently Nate eventually forgot that he was on the losing side of that one, and by that point Erik wasn't around to hound people (and Lisa's not the hounding type), so it fell by the wayside. There are a couple of threads from late 2008 where we basically had the same discussion we're having now:
http://www.mail-archive.com/gem5-dev-***@public.gmane.org/msg01365.html
http://www.mail-archive.com/gem5-dev-***@public.gmane.org/msg02026.html

I've updated the style guide to reflect what I believe are the current rules:
http://gem5.org/Coding_Style#Braces
If we want to discuss further, we should do it on the list (or break new ground and use the wiki talk page). I probably should have put this on the list instead of typing it into reviewboard as well, but now that I've done it I'm not sure it's worth switching.


- Steve


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1764
-----------------------------------------------------------
Post by Steve Reinhardt
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Geoffrey Blake
2011-12-20 23:31:16 UTC
Permalink
Post by Geoffrey Blake
src/cpu/CheckerCPU.py, line 38
<http://reviews.m5sim.org/r/910/diff/3/?file=16271#file16271line38>
What's your reasoning for changing this? Are you worried that this is likely to happen but not signify a real bug? Have you actually seen that happen?
Yes. I see loads from the Checker complain that the values don't match often enough due to no LSQ snooping. I think setting warnOnlyOnLoadError to False should be the default. Functional LSQ snooping from the Checker should be added as a future task to figure out a way around this. In FS mode, it is also possible for loads from the Checker to see different values than the O3 because of coherent IO as well.
Post by Geoffrey Blake
src/cpu/base.cc, line 420
<http://reviews.m5sim.org/r/910/diff/3/?file=16274#file16274line420>
Have you tested how the checker acts when you switch CPUs? It wouldn't be criminal if that isn't working perfectly at this stage since this change already fixing alot, but it would be nice if that worked.
The Checker works with fast-forwarding and checkpoints and has been tested.
Post by Geoffrey Blake
src/cpu/o3/thread_context_impl.hh, line 335
<http://reviews.m5sim.org/r/910/diff/3/?file=16293#file16293line335>
Drop the {}s. I'll stop pointing these out (and I may have missed some), but please fix throughout.
I'll go through and correct the style in the upcoming days. Thanks for the review comments.


- Geoffrey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1764
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-13 13:25:24)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/dev/isa_fake.cc c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Geoffrey Blake
2011-12-22 20:07:13 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------

(Updated 2011-12-22 12:07:13.763523)


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


Summary
-------

CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5

Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.


Diffs (updated)
-----

configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/mem/bus.cc c1ab57ea8805
src/mem/cache/cache_impl.hh c1ab57ea8805
src/mem/packet.hh c1ab57ea8805

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


Testing
-------

Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.


Thanks,

Geoffrey
Geoffrey Blake
2011-12-22 20:11:40 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1783
-----------------------------------------------------------


Made requested fixes to style. Some small changes to fix a memory leak induced by adding an extra request object in src/cpu/base_dyn_inst.hh.

Implemented a solution for the bus/functional accesses problem by treating accesses that come in from the mem-side port to be "expressSnoops" and have the bus treat these accordingly if the IOCache does happen to forward snoops to a device with coherent memory.

- Geoffrey
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-22 12:07:13)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/mem/bus.cc c1ab57ea8805
src/mem/cache/cache_impl.hh c1ab57ea8805
src/mem/packet.hh c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Ali Saidi
2012-01-19 21:06:41 UTC
Permalink
Post by Geoffrey Blake
Post by Geoffrey Blake
Made requested fixes to style. Some small changes to fix a memory leak induced by adding an extra request object in src/cpu/base_dyn_inst.hh.
Implemented a solution for the bus/functional accesses problem by treating accesses that come in from the mem-side port to be "expressSnoops" and have the bus treat these accordingly if the IOCache does happen to forward snoops to a device with coherent memory.
last chance to complain before this is committed.


- Ali


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/910/#review1783
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.m5sim.org/r/910/
-----------------------------------------------------------
(Updated 2011-12-22 12:07:13)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Summary
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/mem/bus.cc c1ab57ea8805
src/mem/cache/cache_impl.hh c1ab57ea8805
src/mem/packet.hh c1ab57ea8805
Diff: http://reviews.m5sim.org/r/910/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey
Andreas Hansson
2012-01-20 08:33:07 UTC
Permalink
Post by Ali Saidi
Post by Geoffrey Blake
Made requested fixes to style. Some small changes to fix a memory leak induced by adding an extra request object in src/cpu/base_dyn_inst.hh.
Implemented a solution for the bus/functional accesses problem by treating accesses that come in from the mem-side port to be "expressSnoops" and have the bus treat these accordingly if the IOCache does happen to forward snoops to a device with coherent memory.
last chance to complain before this is committed.
Has the patch been updated since the repository surgery? I cannot see the diff.


- Andreas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/910/#review1783
-----------------------------------------------------------
Post by Ali Saidi
-----------------------------------------------------------
http://reviews.gem5.org/r/910/
-----------------------------------------------------------
(Updated Dec. 22, 2011, 12:07 p.m.)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Description
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
configs/common/FSConfig.py c1ab57ea8805
src/arch/arm/isa.cc c1ab57ea8805
src/arch/arm/isa/insts/m5ops.isa c1ab57ea8805
src/arch/arm/isa/insts/misc.isa c1ab57ea8805
src/arch/arm/table_walker.hh c1ab57ea8805
src/arch/arm/table_walker.cc c1ab57ea8805
src/arch/arm/tlb.hh c1ab57ea8805
src/arch/arm/tlb.cc c1ab57ea8805
src/arch/arm/utility.cc c1ab57ea8805
src/cpu/BaseCPU.py c1ab57ea8805
src/cpu/CheckerCPU.py c1ab57ea8805
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript c1ab57ea8805
src/cpu/base.cc c1ab57ea8805
src/cpu/base_dyn_inst.hh c1ab57ea8805
src/cpu/base_dyn_inst_impl.hh c1ab57ea8805
src/cpu/checker/cpu.hh c1ab57ea8805
src/cpu/checker/cpu.cc c1ab57ea8805
src/cpu/checker/cpu_impl.hh c1ab57ea8805
src/cpu/checker/thread_context.hh c1ab57ea8805
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py c1ab57ea8805
src/cpu/o3/O3Checker.py c1ab57ea8805
src/cpu/o3/checker_builder.cc c1ab57ea8805
src/cpu/o3/commit_impl.hh c1ab57ea8805
src/cpu/o3/cpu.hh c1ab57ea8805
src/cpu/o3/cpu.cc c1ab57ea8805
src/cpu/o3/dyn_inst_impl.hh c1ab57ea8805
src/cpu/o3/fetch_impl.hh c1ab57ea8805
src/cpu/o3/iew_impl.hh c1ab57ea8805
src/cpu/o3/lsq_unit_impl.hh c1ab57ea8805
src/cpu/o3/thread_context.hh c1ab57ea8805
src/cpu/o3/thread_context_impl.hh c1ab57ea8805
src/cpu/simple/BaseSimpleCPU.py c1ab57ea8805
src/cpu/simple/base.hh c1ab57ea8805
src/cpu/simple/base.cc c1ab57ea8805
src/cpu/simple_thread.hh c1ab57ea8805
src/cpu/thread_context.hh c1ab57ea8805
src/dev/Device.py c1ab57ea8805
src/dev/isa_fake.hh c1ab57ea8805
src/mem/bus.cc c1ab57ea8805
src/mem/cache/cache_impl.hh c1ab57ea8805
src/mem/packet.hh c1ab57ea8805
Diff: http://reviews.gem5.org/r/910/diff/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey Blake
Geoffrey Blake
2012-01-23 18:14:13 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/910/
-----------------------------------------------------------

(Updated Jan. 23, 2012, 10:14 a.m.)


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


Description
-------

CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5

Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.


Diffs (updated)
-----

src/cpu/SConscript a1d5a0e2e970
src/cpu/base.cc a1d5a0e2e970
src/cpu/CheckerCPU.py a1d5a0e2e970
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/BaseCPU.py a1d5a0e2e970
src/arch/arm/utility.cc a1d5a0e2e970
src/arch/arm/tlb.hh a1d5a0e2e970
src/arch/arm/tlb.cc a1d5a0e2e970
src/arch/arm/isa/insts/misc.isa a1d5a0e2e970
src/arch/arm/table_walker.hh a1d5a0e2e970
src/arch/arm/table_walker.cc a1d5a0e2e970
src/arch/arm/isa/insts/m5ops.isa a1d5a0e2e970
src/arch/arm/isa.cc a1d5a0e2e970
src/cpu/base_dyn_inst.hh a1d5a0e2e970
src/cpu/base_dyn_inst_impl.hh a1d5a0e2e970
src/cpu/checker/cpu.hh a1d5a0e2e970
src/cpu/checker/cpu.cc a1d5a0e2e970
src/cpu/checker/cpu_impl.hh a1d5a0e2e970
src/cpu/checker/thread_context.hh a1d5a0e2e970
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py a1d5a0e2e970
src/cpu/o3/O3Checker.py a1d5a0e2e970
src/cpu/o3/checker_builder.cc a1d5a0e2e970
src/cpu/o3/commit_impl.hh a1d5a0e2e970
src/cpu/o3/cpu.hh a1d5a0e2e970
src/cpu/o3/cpu.cc a1d5a0e2e970
src/cpu/o3/dyn_inst_impl.hh a1d5a0e2e970
src/cpu/o3/fetch_impl.hh a1d5a0e2e970
src/cpu/o3/iew_impl.hh a1d5a0e2e970
src/cpu/o3/lsq_unit_impl.hh a1d5a0e2e970
src/cpu/o3/thread_context.hh a1d5a0e2e970
src/cpu/o3/thread_context_impl.hh a1d5a0e2e970
src/cpu/simple/BaseSimpleCPU.py a1d5a0e2e970
src/cpu/simple/base.hh a1d5a0e2e970
src/cpu/simple/base.cc a1d5a0e2e970
src/cpu/simple_thread.hh a1d5a0e2e970
src/cpu/thread_context.hh a1d5a0e2e970
src/mem/bus.cc a1d5a0e2e970
src/mem/cache/cache_impl.hh a1d5a0e2e970
src/mem/packet.hh a1d5a0e2e970

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


Testing
-------

Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.


Thanks,

Geoffrey Blake
Andreas Hansson
2012-01-24 08:41:07 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/910/#review1986
-----------------------------------------------------------



src/mem/bus.cc
<http://reviews.gem5.org/r/910/#comment2485>

This should no longer be needed with the changes done to the functionalAccess in the cache now being able to respect forwardSnoops.



src/mem/cache/cache_impl.hh
<http://reviews.gem5.org/r/910/#comment2486>

This should not be needed surely? If it should not forward snoops then don't forward it?



src/mem/cache/cache_impl.hh
<http://reviews.gem5.org/r/910/#comment2487>

I don't understand why these changes are here. They make me even more confused about what an express snoop is and what it is not. What problem is this aiming to solve?



src/mem/packet.hh
<http://reviews.gem5.org/r/910/#comment2488>

A packet should never be an express snoop and at any point loose that state (at least it messes with my brain and the ability to reason about what type of packets go where).


- Andreas Hansson
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.gem5.org/r/910/
-----------------------------------------------------------
(Updated Jan. 23, 2012, 10:14 a.m.)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Description
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
src/cpu/SConscript a1d5a0e2e970
src/cpu/base.cc a1d5a0e2e970
src/cpu/CheckerCPU.py a1d5a0e2e970
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/BaseCPU.py a1d5a0e2e970
src/arch/arm/utility.cc a1d5a0e2e970
src/arch/arm/tlb.hh a1d5a0e2e970
src/arch/arm/tlb.cc a1d5a0e2e970
src/arch/arm/isa/insts/misc.isa a1d5a0e2e970
src/arch/arm/table_walker.hh a1d5a0e2e970
src/arch/arm/table_walker.cc a1d5a0e2e970
src/arch/arm/isa/insts/m5ops.isa a1d5a0e2e970
src/arch/arm/isa.cc a1d5a0e2e970
src/cpu/base_dyn_inst.hh a1d5a0e2e970
src/cpu/base_dyn_inst_impl.hh a1d5a0e2e970
src/cpu/checker/cpu.hh a1d5a0e2e970
src/cpu/checker/cpu.cc a1d5a0e2e970
src/cpu/checker/cpu_impl.hh a1d5a0e2e970
src/cpu/checker/thread_context.hh a1d5a0e2e970
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py a1d5a0e2e970
src/cpu/o3/O3Checker.py a1d5a0e2e970
src/cpu/o3/checker_builder.cc a1d5a0e2e970
src/cpu/o3/commit_impl.hh a1d5a0e2e970
src/cpu/o3/cpu.hh a1d5a0e2e970
src/cpu/o3/cpu.cc a1d5a0e2e970
src/cpu/o3/dyn_inst_impl.hh a1d5a0e2e970
src/cpu/o3/fetch_impl.hh a1d5a0e2e970
src/cpu/o3/iew_impl.hh a1d5a0e2e970
src/cpu/o3/lsq_unit_impl.hh a1d5a0e2e970
src/cpu/o3/thread_context.hh a1d5a0e2e970
src/cpu/o3/thread_context_impl.hh a1d5a0e2e970
src/cpu/simple/BaseSimpleCPU.py a1d5a0e2e970
src/cpu/simple/base.hh a1d5a0e2e970
src/cpu/simple/base.cc a1d5a0e2e970
src/cpu/simple_thread.hh a1d5a0e2e970
src/cpu/thread_context.hh a1d5a0e2e970
src/mem/bus.cc a1d5a0e2e970
src/mem/cache/cache_impl.hh a1d5a0e2e970
src/mem/packet.hh a1d5a0e2e970
Diff: http://reviews.gem5.org/r/910/diff/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey Blake
Geoffrey Blake
2012-01-24 17:30:31 UTC
Permalink
Post by Geoffrey Blake
src/mem/cache/cache_impl.hh, line 814
<http://reviews.gem5.org/r/910/diff/5/?file=21407#file21407line814>
This should not be needed surely? If it should not forward snoops then don't forward it?
Also needed here because the functionalAccess() routine has no knowledge of which port the packet came in on, so this ensures forwardSnoops is respected (requests coming upstream) but also keeps regular accesses (downstream requests) working.
Post by Geoffrey Blake
src/mem/cache/cache_impl.hh, line 1675
<http://reviews.gem5.org/r/910/diff/5/?file=21407#file21407line1675>
I don't understand why these changes are here. They make me even more confused about what an express snoop is and what it is not. What problem is this aiming to solve?
This was a design decision on my part. For the timing path, when a request comes in the MemSidePort, handleSnoop() is called which then creates a new packet with expressSnoop set. Instead of adding more code with a functionalSnoop() routine and creating additional packets like the timing path, I allow the packet to set/unset itself as a snoop when it comes up the MemSidePort to re-use the functionalAccess() code.
Post by Geoffrey Blake
src/mem/bus.cc, line 482
<http://reviews.gem5.org/r/910/diff/5/?file=21406#file21406line482>
This should no longer be needed with the changes done to the functionalAccess in the cache now being able to respect forwardSnoops.
This is needed in the case there is a coherent device that can be snooped on the bus (cache has forwardSnoops=true). The bus needs to identify if the packet is a forwarded snoop to prevent it from sending it erroneously to a destination port.


- Geoffrey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/910/#review1986
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.gem5.org/r/910/
-----------------------------------------------------------
(Updated Jan. 23, 2012, 10:14 a.m.)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Description
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
src/cpu/SConscript a1d5a0e2e970
src/cpu/base.cc a1d5a0e2e970
src/cpu/CheckerCPU.py a1d5a0e2e970
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/BaseCPU.py a1d5a0e2e970
src/arch/arm/utility.cc a1d5a0e2e970
src/arch/arm/tlb.hh a1d5a0e2e970
src/arch/arm/tlb.cc a1d5a0e2e970
src/arch/arm/isa/insts/misc.isa a1d5a0e2e970
src/arch/arm/table_walker.hh a1d5a0e2e970
src/arch/arm/table_walker.cc a1d5a0e2e970
src/arch/arm/isa/insts/m5ops.isa a1d5a0e2e970
src/arch/arm/isa.cc a1d5a0e2e970
src/cpu/base_dyn_inst.hh a1d5a0e2e970
src/cpu/base_dyn_inst_impl.hh a1d5a0e2e970
src/cpu/checker/cpu.hh a1d5a0e2e970
src/cpu/checker/cpu.cc a1d5a0e2e970
src/cpu/checker/cpu_impl.hh a1d5a0e2e970
src/cpu/checker/thread_context.hh a1d5a0e2e970
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py a1d5a0e2e970
src/cpu/o3/O3Checker.py a1d5a0e2e970
src/cpu/o3/checker_builder.cc a1d5a0e2e970
src/cpu/o3/commit_impl.hh a1d5a0e2e970
src/cpu/o3/cpu.hh a1d5a0e2e970
src/cpu/o3/cpu.cc a1d5a0e2e970
src/cpu/o3/dyn_inst_impl.hh a1d5a0e2e970
src/cpu/o3/fetch_impl.hh a1d5a0e2e970
src/cpu/o3/iew_impl.hh a1d5a0e2e970
src/cpu/o3/lsq_unit_impl.hh a1d5a0e2e970
src/cpu/o3/thread_context.hh a1d5a0e2e970
src/cpu/o3/thread_context_impl.hh a1d5a0e2e970
src/cpu/simple/BaseSimpleCPU.py a1d5a0e2e970
src/cpu/simple/base.hh a1d5a0e2e970
src/cpu/simple/base.cc a1d5a0e2e970
src/cpu/simple_thread.hh a1d5a0e2e970
src/cpu/thread_context.hh a1d5a0e2e970
src/mem/bus.cc a1d5a0e2e970
src/mem/cache/cache_impl.hh a1d5a0e2e970
src/mem/packet.hh a1d5a0e2e970
Diff: http://reviews.gem5.org/r/910/diff/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey Blake
Geoffrey Blake
2012-01-25 23:04:47 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/910/
-----------------------------------------------------------

(Updated Jan. 25, 2012, 3:04 p.m.)


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


Description
-------

CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5

Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.


Diffs (updated)
-----

src/arch/arm/isa.cc 78b08f92c290
src/arch/arm/isa/insts/m5ops.isa 78b08f92c290
src/arch/arm/isa/insts/misc.isa 78b08f92c290
src/arch/arm/table_walker.hh 78b08f92c290
src/arch/arm/table_walker.cc 78b08f92c290
src/arch/arm/tlb.hh 78b08f92c290
src/arch/arm/tlb.cc 78b08f92c290
src/arch/arm/utility.cc 78b08f92c290
src/cpu/BaseCPU.py 78b08f92c290
src/cpu/CheckerCPU.py 78b08f92c290
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 78b08f92c290
src/cpu/base.cc 78b08f92c290
src/cpu/base_dyn_inst.hh 78b08f92c290
src/cpu/base_dyn_inst_impl.hh 78b08f92c290
src/cpu/checker/cpu.hh 78b08f92c290
src/cpu/checker/cpu.cc 78b08f92c290
src/cpu/checker/cpu_impl.hh 78b08f92c290
src/cpu/checker/thread_context.hh 78b08f92c290
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 78b08f92c290
src/cpu/o3/O3Checker.py 78b08f92c290
src/cpu/o3/checker_builder.cc 78b08f92c290
src/cpu/o3/commit_impl.hh 78b08f92c290
src/cpu/o3/cpu.hh 78b08f92c290
src/cpu/o3/cpu.cc 78b08f92c290
src/cpu/o3/dyn_inst_impl.hh 78b08f92c290
src/cpu/o3/fetch_impl.hh 78b08f92c290
src/cpu/o3/iew_impl.hh 78b08f92c290
src/cpu/o3/lsq_unit_impl.hh 78b08f92c290
src/cpu/o3/thread_context.hh 78b08f92c290
src/cpu/o3/thread_context_impl.hh 78b08f92c290
src/cpu/simple/BaseSimpleCPU.py 78b08f92c290
src/cpu/simple/base.hh 78b08f92c290
src/cpu/simple/base.cc 78b08f92c290
src/cpu/simple_thread.hh 78b08f92c290
src/cpu/thread_context.hh 78b08f92c290
src/mem/bus.cc 78b08f92c290
src/mem/cache/cache_impl.hh 78b08f92c290
src/mem/packet.hh 78b08f92c290

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


Testing
-------

Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.


Thanks,

Geoffrey Blake
Geoffrey Blake
2012-01-25 23:08:14 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/910/#review1995
-----------------------------------------------------------


Re-factored patch to compile and work with Andreas's changes that were recently pushed.

Will fix the python param code issue with the Checker for SE mode in another patch to come soon.

- Geoffrey Blake
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.gem5.org/r/910/
-----------------------------------------------------------
(Updated Jan. 25, 2012, 3:04 p.m.)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Description
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
src/arch/arm/isa.cc 78b08f92c290
src/arch/arm/isa/insts/m5ops.isa 78b08f92c290
src/arch/arm/isa/insts/misc.isa 78b08f92c290
src/arch/arm/table_walker.hh 78b08f92c290
src/arch/arm/table_walker.cc 78b08f92c290
src/arch/arm/tlb.hh 78b08f92c290
src/arch/arm/tlb.cc 78b08f92c290
src/arch/arm/utility.cc 78b08f92c290
src/cpu/BaseCPU.py 78b08f92c290
src/cpu/CheckerCPU.py 78b08f92c290
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 78b08f92c290
src/cpu/base.cc 78b08f92c290
src/cpu/base_dyn_inst.hh 78b08f92c290
src/cpu/base_dyn_inst_impl.hh 78b08f92c290
src/cpu/checker/cpu.hh 78b08f92c290
src/cpu/checker/cpu.cc 78b08f92c290
src/cpu/checker/cpu_impl.hh 78b08f92c290
src/cpu/checker/thread_context.hh 78b08f92c290
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 78b08f92c290
src/cpu/o3/O3Checker.py 78b08f92c290
src/cpu/o3/checker_builder.cc 78b08f92c290
src/cpu/o3/commit_impl.hh 78b08f92c290
src/cpu/o3/cpu.hh 78b08f92c290
src/cpu/o3/cpu.cc 78b08f92c290
src/cpu/o3/dyn_inst_impl.hh 78b08f92c290
src/cpu/o3/fetch_impl.hh 78b08f92c290
src/cpu/o3/iew_impl.hh 78b08f92c290
src/cpu/o3/lsq_unit_impl.hh 78b08f92c290
src/cpu/o3/thread_context.hh 78b08f92c290
src/cpu/o3/thread_context_impl.hh 78b08f92c290
src/cpu/simple/BaseSimpleCPU.py 78b08f92c290
src/cpu/simple/base.hh 78b08f92c290
src/cpu/simple/base.cc 78b08f92c290
src/cpu/simple_thread.hh 78b08f92c290
src/cpu/thread_context.hh 78b08f92c290
src/mem/bus.cc 78b08f92c290
src/mem/cache/cache_impl.hh 78b08f92c290
src/mem/packet.hh 78b08f92c290
Diff: http://reviews.gem5.org/r/910/diff/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey Blake
Geoffrey Blake
2012-01-26 15:10:09 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/910/
-----------------------------------------------------------

(Updated Jan. 26, 2012, 7:10 a.m.)


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


Description
-------

CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5

Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.


Diffs (updated)
-----

src/arch/arm/isa.cc 78b08f92c290
src/arch/arm/isa/insts/m5ops.isa 78b08f92c290
src/arch/arm/isa/insts/misc.isa 78b08f92c290
src/arch/arm/table_walker.hh 78b08f92c290
src/arch/arm/table_walker.cc 78b08f92c290
src/arch/arm/tlb.hh 78b08f92c290
src/arch/arm/tlb.cc 78b08f92c290
src/arch/arm/utility.cc 78b08f92c290
src/cpu/BaseCPU.py 78b08f92c290
src/cpu/CheckerCPU.py 78b08f92c290
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 78b08f92c290
src/cpu/base.cc 78b08f92c290
src/cpu/base_dyn_inst.hh 78b08f92c290
src/cpu/base_dyn_inst_impl.hh 78b08f92c290
src/cpu/checker/cpu.hh 78b08f92c290
src/cpu/checker/cpu.cc 78b08f92c290
src/cpu/checker/cpu_impl.hh 78b08f92c290
src/cpu/checker/thread_context.hh 78b08f92c290
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 78b08f92c290
src/cpu/o3/O3Checker.py 78b08f92c290
src/cpu/o3/checker_builder.cc 78b08f92c290
src/cpu/o3/commit_impl.hh 78b08f92c290
src/cpu/o3/cpu.hh 78b08f92c290
src/cpu/o3/cpu.cc 78b08f92c290
src/cpu/o3/dyn_inst_impl.hh 78b08f92c290
src/cpu/o3/fetch_impl.hh 78b08f92c290
src/cpu/o3/iew_impl.hh 78b08f92c290
src/cpu/o3/lsq_unit_impl.hh 78b08f92c290
src/cpu/o3/thread_context.hh 78b08f92c290
src/cpu/o3/thread_context_impl.hh 78b08f92c290
src/cpu/simple/BaseSimpleCPU.py 78b08f92c290
src/cpu/simple/base.hh 78b08f92c290
src/cpu/simple/base.cc 78b08f92c290
src/cpu/simple_thread.hh 78b08f92c290
src/cpu/thread_context.hh 78b08f92c290
src/mem/bus.cc 78b08f92c290

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


Testing
-------

Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.


Thanks,

Geoffrey Blake
Gabe Black
2012-01-27 09:20:17 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/910/#review1997
-----------------------------------------------------------


There are only a few small things to fix now, at least as far as I can see.


src/arch/arm/isa.cc
<http://reviews.gem5.org/r/910/#comment2497>

This line is too long.



src/arch/arm/isa.cc
<http://reviews.gem5.org/r/910/#comment2498>

So is this one.



src/arch/arm/isa.cc
<http://reviews.gem5.org/r/910/#comment2499>

And this one.



src/arch/arm/isa.cc
<http://reviews.gem5.org/r/910/#comment2500>

And this one. I'll stop pointing them out, but please fix throughout.



src/cpu/checker/cpu.hh
<http://reviews.gem5.org/r/910/#comment2501>

It's not from you, but there's some whitespace at the end of this line. Since you're getting rid of commented out code, you could get rid of the whitespace too.



src/cpu/checker/cpu.cc
<http://reviews.gem5.org/r/910/#comment2502>

This sentence seems to have dropped off half way there.


- Gabe Black
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.gem5.org/r/910/
-----------------------------------------------------------
(Updated Jan. 26, 2012, 7:10 a.m.)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Description
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
src/arch/arm/isa.cc 78b08f92c290
src/arch/arm/isa/insts/m5ops.isa 78b08f92c290
src/arch/arm/isa/insts/misc.isa 78b08f92c290
src/arch/arm/table_walker.hh 78b08f92c290
src/arch/arm/table_walker.cc 78b08f92c290
src/arch/arm/tlb.hh 78b08f92c290
src/arch/arm/tlb.cc 78b08f92c290
src/arch/arm/utility.cc 78b08f92c290
src/cpu/BaseCPU.py 78b08f92c290
src/cpu/CheckerCPU.py 78b08f92c290
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 78b08f92c290
src/cpu/base.cc 78b08f92c290
src/cpu/base_dyn_inst.hh 78b08f92c290
src/cpu/base_dyn_inst_impl.hh 78b08f92c290
src/cpu/checker/cpu.hh 78b08f92c290
src/cpu/checker/cpu.cc 78b08f92c290
src/cpu/checker/cpu_impl.hh 78b08f92c290
src/cpu/checker/thread_context.hh 78b08f92c290
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 78b08f92c290
src/cpu/o3/O3Checker.py 78b08f92c290
src/cpu/o3/checker_builder.cc 78b08f92c290
src/cpu/o3/commit_impl.hh 78b08f92c290
src/cpu/o3/cpu.hh 78b08f92c290
src/cpu/o3/cpu.cc 78b08f92c290
src/cpu/o3/dyn_inst_impl.hh 78b08f92c290
src/cpu/o3/fetch_impl.hh 78b08f92c290
src/cpu/o3/iew_impl.hh 78b08f92c290
src/cpu/o3/lsq_unit_impl.hh 78b08f92c290
src/cpu/o3/thread_context.hh 78b08f92c290
src/cpu/o3/thread_context_impl.hh 78b08f92c290
src/cpu/simple/BaseSimpleCPU.py 78b08f92c290
src/cpu/simple/base.hh 78b08f92c290
src/cpu/simple/base.cc 78b08f92c290
src/cpu/simple_thread.hh 78b08f92c290
src/cpu/thread_context.hh 78b08f92c290
src/mem/bus.cc 78b08f92c290
Diff: http://reviews.gem5.org/r/910/diff/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey Blake
Geoffrey Blake
2012-01-27 14:36:09 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/910/
-----------------------------------------------------------

(Updated Jan. 27, 2012, 6:36 a.m.)


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


Description
-------

CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5

Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.


Diffs (updated)
-----

src/arch/arm/isa.cc 78b08f92c290
src/arch/arm/isa/insts/m5ops.isa 78b08f92c290
src/arch/arm/isa/insts/misc.isa 78b08f92c290
src/arch/arm/table_walker.hh 78b08f92c290
src/arch/arm/table_walker.cc 78b08f92c290
src/arch/arm/tlb.hh 78b08f92c290
src/arch/arm/tlb.cc 78b08f92c290
src/arch/arm/utility.cc 78b08f92c290
src/cpu/BaseCPU.py 78b08f92c290
src/cpu/CheckerCPU.py 78b08f92c290
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 78b08f92c290
src/cpu/base.cc 78b08f92c290
src/cpu/base_dyn_inst.hh 78b08f92c290
src/cpu/base_dyn_inst_impl.hh 78b08f92c290
src/cpu/checker/cpu.hh 78b08f92c290
src/cpu/checker/cpu.cc 78b08f92c290
src/cpu/checker/cpu_impl.hh 78b08f92c290
src/cpu/checker/thread_context.hh 78b08f92c290
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 78b08f92c290
src/cpu/o3/O3Checker.py 78b08f92c290
src/cpu/o3/checker_builder.cc 78b08f92c290
src/cpu/o3/commit_impl.hh 78b08f92c290
src/cpu/o3/cpu.hh 78b08f92c290
src/cpu/o3/cpu.cc 78b08f92c290
src/cpu/o3/dyn_inst_impl.hh 78b08f92c290
src/cpu/o3/fetch_impl.hh 78b08f92c290
src/cpu/o3/iew_impl.hh 78b08f92c290
src/cpu/o3/lsq_unit_impl.hh 78b08f92c290
src/cpu/o3/thread_context.hh 78b08f92c290
src/cpu/o3/thread_context_impl.hh 78b08f92c290
src/cpu/simple/BaseSimpleCPU.py 78b08f92c290
src/cpu/simple/base.hh 78b08f92c290
src/cpu/simple/base.cc 78b08f92c290
src/cpu/simple_thread.hh 78b08f92c290
src/cpu/thread_context.hh 78b08f92c290
src/mem/bus.cc 78b08f92c290

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


Testing
-------

Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.


Thanks,

Geoffrey Blake
Gabe Black
2012-01-27 20:28:58 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/910/#review2004
-----------------------------------------------------------

Ship it!


Ship It!

- Gabe Black
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.gem5.org/r/910/
-----------------------------------------------------------
(Updated Jan. 27, 2012, 6:36 a.m.)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Description
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
src/arch/arm/isa.cc 78b08f92c290
src/arch/arm/isa/insts/m5ops.isa 78b08f92c290
src/arch/arm/isa/insts/misc.isa 78b08f92c290
src/arch/arm/table_walker.hh 78b08f92c290
src/arch/arm/table_walker.cc 78b08f92c290
src/arch/arm/tlb.hh 78b08f92c290
src/arch/arm/tlb.cc 78b08f92c290
src/arch/arm/utility.cc 78b08f92c290
src/cpu/BaseCPU.py 78b08f92c290
src/cpu/CheckerCPU.py 78b08f92c290
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/SConscript 78b08f92c290
src/cpu/base.cc 78b08f92c290
src/cpu/base_dyn_inst.hh 78b08f92c290
src/cpu/base_dyn_inst_impl.hh 78b08f92c290
src/cpu/checker/cpu.hh 78b08f92c290
src/cpu/checker/cpu.cc 78b08f92c290
src/cpu/checker/cpu_impl.hh 78b08f92c290
src/cpu/checker/thread_context.hh 78b08f92c290
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py 78b08f92c290
src/cpu/o3/O3Checker.py 78b08f92c290
src/cpu/o3/checker_builder.cc 78b08f92c290
src/cpu/o3/commit_impl.hh 78b08f92c290
src/cpu/o3/cpu.hh 78b08f92c290
src/cpu/o3/cpu.cc 78b08f92c290
src/cpu/o3/dyn_inst_impl.hh 78b08f92c290
src/cpu/o3/fetch_impl.hh 78b08f92c290
src/cpu/o3/iew_impl.hh 78b08f92c290
src/cpu/o3/lsq_unit_impl.hh 78b08f92c290
src/cpu/o3/thread_context.hh 78b08f92c290
src/cpu/o3/thread_context_impl.hh 78b08f92c290
src/cpu/simple/BaseSimpleCPU.py 78b08f92c290
src/cpu/simple/base.hh 78b08f92c290
src/cpu/simple/base.cc 78b08f92c290
src/cpu/simple_thread.hh 78b08f92c290
src/cpu/thread_context.hh 78b08f92c290
src/mem/bus.cc 78b08f92c290
Diff: http://reviews.gem5.org/r/910/diff/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey Blake
Geoffrey Blake
2012-01-23 18:21:41 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/910/#review1983
-----------------------------------------------------------


Re-posted the diff.


src/cpu/checker/cpu.cc
<http://reviews.gem5.org/r/910/#comment2482>

Related to the question below. Any ideas how to clean this up? This limits an SE run to only 1 core effectively right now.



src/cpu/o3/O3CPU.py
<http://reviews.gem5.org/r/910/#comment2481>

I have a question for Nate or someone familiar with the guts of the python setup code for a simulation. I had to put this array de-reference to get the python to work properly. Without it, the python code complains about an unknown type, even if the workload array is 1 element, as python will try to pass this: [[workload1]]. An array of arrays to the C++ code. How should I fix this?


- Geoffrey Blake
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.gem5.org/r/910/
-----------------------------------------------------------
(Updated Jan. 23, 2012, 10:14 a.m.)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Description
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
src/cpu/SConscript a1d5a0e2e970
src/cpu/base.cc a1d5a0e2e970
src/cpu/CheckerCPU.py a1d5a0e2e970
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/BaseCPU.py a1d5a0e2e970
src/arch/arm/utility.cc a1d5a0e2e970
src/arch/arm/tlb.hh a1d5a0e2e970
src/arch/arm/tlb.cc a1d5a0e2e970
src/arch/arm/isa/insts/misc.isa a1d5a0e2e970
src/arch/arm/table_walker.hh a1d5a0e2e970
src/arch/arm/table_walker.cc a1d5a0e2e970
src/arch/arm/isa/insts/m5ops.isa a1d5a0e2e970
src/arch/arm/isa.cc a1d5a0e2e970
src/cpu/base_dyn_inst.hh a1d5a0e2e970
src/cpu/base_dyn_inst_impl.hh a1d5a0e2e970
src/cpu/checker/cpu.hh a1d5a0e2e970
src/cpu/checker/cpu.cc a1d5a0e2e970
src/cpu/checker/cpu_impl.hh a1d5a0e2e970
src/cpu/checker/thread_context.hh a1d5a0e2e970
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py a1d5a0e2e970
src/cpu/o3/O3Checker.py a1d5a0e2e970
src/cpu/o3/checker_builder.cc a1d5a0e2e970
src/cpu/o3/commit_impl.hh a1d5a0e2e970
src/cpu/o3/cpu.hh a1d5a0e2e970
src/cpu/o3/cpu.cc a1d5a0e2e970
src/cpu/o3/dyn_inst_impl.hh a1d5a0e2e970
src/cpu/o3/fetch_impl.hh a1d5a0e2e970
src/cpu/o3/iew_impl.hh a1d5a0e2e970
src/cpu/o3/lsq_unit_impl.hh a1d5a0e2e970
src/cpu/o3/thread_context.hh a1d5a0e2e970
src/cpu/o3/thread_context_impl.hh a1d5a0e2e970
src/cpu/simple/BaseSimpleCPU.py a1d5a0e2e970
src/cpu/simple/base.hh a1d5a0e2e970
src/cpu/simple/base.cc a1d5a0e2e970
src/cpu/simple_thread.hh a1d5a0e2e970
src/cpu/thread_context.hh a1d5a0e2e970
src/mem/bus.cc a1d5a0e2e970
src/mem/cache/cache_impl.hh a1d5a0e2e970
src/mem/packet.hh a1d5a0e2e970
Diff: http://reviews.gem5.org/r/910/diff/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey Blake
Steve Reinhardt
2012-01-24 05:23:25 UTC
Permalink
Post by Geoffrey Blake
src/cpu/o3/O3CPU.py, line 45
<http://reviews.gem5.org/r/910/diff/5/?file=21389#file21389line45>
I have a question for Nate or someone familiar with the guts of the python setup code for a simulation. I had to put this array de-reference to get the python to work properly. Without it, the python code complains about an unknown type, even if the workload array is 1 element, as python will try to pass this: [[workload1]]. An array of arrays to the C++ code. How should I fix this?
Good question... this may be a bug in the internal param code. When you assign a scalar value to a VectorParam like "workload" it will automatically convert the scalar value to a one-element array. It almost looks like this is happening twice instead of once. You could possibly be the first person to ever use the Parent. proxy mechanism with a VectorParam.

You can try putting some print statements in VectorParamDesc.convert() in puthon/m5/params.py to see if you can track down what's going on... unfortunately I don't have time to look at it myself. Let me know what you see and I can try and help figure it out.


- Steve


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/910/#review1983
-----------------------------------------------------------
Post by Geoffrey Blake
-----------------------------------------------------------
http://reviews.gem5.org/r/910/
-----------------------------------------------------------
(Updated Jan. 23, 2012, 10:14 a.m.)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert.
Description
-------
CheckerCPU: Re-factor CheckerCPU to be compatible with current gem5
Brings the CheckerCPU back to a functioning state allowing FS and SE mode
checking of the O3CPU. These changes have only been tested with the
ARM ISA. Other ISAs will potentially require modification.
Diffs
-----
src/cpu/SConscript a1d5a0e2e970
src/cpu/base.cc a1d5a0e2e970
src/cpu/CheckerCPU.py a1d5a0e2e970
src/cpu/DummyChecker.py PRE-CREATION
src/cpu/BaseCPU.py a1d5a0e2e970
src/arch/arm/utility.cc a1d5a0e2e970
src/arch/arm/tlb.hh a1d5a0e2e970
src/arch/arm/tlb.cc a1d5a0e2e970
src/arch/arm/isa/insts/misc.isa a1d5a0e2e970
src/arch/arm/table_walker.hh a1d5a0e2e970
src/arch/arm/table_walker.cc a1d5a0e2e970
src/arch/arm/isa/insts/m5ops.isa a1d5a0e2e970
src/arch/arm/isa.cc a1d5a0e2e970
src/cpu/base_dyn_inst.hh a1d5a0e2e970
src/cpu/base_dyn_inst_impl.hh a1d5a0e2e970
src/cpu/checker/cpu.hh a1d5a0e2e970
src/cpu/checker/cpu.cc a1d5a0e2e970
src/cpu/checker/cpu_impl.hh a1d5a0e2e970
src/cpu/checker/thread_context.hh a1d5a0e2e970
src/cpu/dummy_checker_builder.cc PRE-CREATION
src/cpu/o3/O3CPU.py a1d5a0e2e970
src/cpu/o3/O3Checker.py a1d5a0e2e970
src/cpu/o3/checker_builder.cc a1d5a0e2e970
src/cpu/o3/commit_impl.hh a1d5a0e2e970
src/cpu/o3/cpu.hh a1d5a0e2e970
src/cpu/o3/cpu.cc a1d5a0e2e970
src/cpu/o3/dyn_inst_impl.hh a1d5a0e2e970
src/cpu/o3/fetch_impl.hh a1d5a0e2e970
src/cpu/o3/iew_impl.hh a1d5a0e2e970
src/cpu/o3/lsq_unit_impl.hh a1d5a0e2e970
src/cpu/o3/thread_context.hh a1d5a0e2e970
src/cpu/o3/thread_context_impl.hh a1d5a0e2e970
src/cpu/simple/BaseSimpleCPU.py a1d5a0e2e970
src/cpu/simple/base.hh a1d5a0e2e970
src/cpu/simple/base.cc a1d5a0e2e970
src/cpu/simple_thread.hh a1d5a0e2e970
src/cpu/thread_context.hh a1d5a0e2e970
src/mem/bus.cc a1d5a0e2e970
src/mem/cache/cache_impl.hh a1d5a0e2e970
src/mem/packet.hh a1d5a0e2e970
Diff: http://reviews.gem5.org/r/910/diff/diff
Testing
-------
Successfully runs gzip in SE mode. Successfully boots linux kernel in FS mode. Works with checkpoints and fast-forwarding. Testing with buggy O3CPUs to test checker's capabilities.
Thanks,
Geoffrey Blake
Continue reading on narkive:
Loading...