> On Feb. 9, 2012, 1:55 a.m., Gabe Black wrote: > > configs/example/se.py, line 180 > > <http://reviews.gem5.org/r/1031/diff/1/?file=23023#file23023line180> > > > > Not that there's anything necessarily wrong with doing it, but why are > > you moving this down past the for loop here? It's not obvious how the two > > would interact.
The interaction comes from the way ports are created and connected. When addCheckerCpu is called, it adds 2 ports to the _cache_ports list in BaseCPU for the TLB walkers (if needed). When the caches are created, it calls connectAllPorts() goes through the _cache_ports list and connects all the CPU ports to the proper bus. If this happens in the wrong order then the checker ports will end up unconnected and result in a seg-fault. If desired I can go back and make these two function calls decoupled. > On Feb. 9, 2012, 1:55 a.m., Gabe Black wrote: > > src/cpu/SConscript, line 137 > > <http://reviews.gem5.org/r/1031/diff/1/?file=23028#file23028line137> > > > > Is the dummy checker needed any more now that you don't have to have a > > checker installed? Not sure what you mean here. The DummyChecker is still needed for proper port creation during configuration for when fast forwarding/checkpointing is used to switch from simple to complex cpus. > On Feb. 9, 2012, 1:55 a.m., Gabe Black wrote: > > src/cpu/checker/cpu.cc, line 95 > > <http://reviews.gem5.org/r/1031/diff/1/?file=23032#file23032line95> > > > > You shouldn't make assumptions like this. FS and SE aren't two separate > > worlds any more, and you can't assume because you have some of the > > characteristics of one that you'll have all of them. I even have plans > > specifically for making workload do something in what would have been > > called FS mode. The SimpleThread object right now assumes a division between FS and SE modes with its two constructors. So for right now its a safe assumption. When you make those changes I'll re-factor this code. > On Feb. 9, 2012, 1:55 a.m., Gabe Black wrote: > > src/cpu/checker/cpu.cc, line 110 > > <http://reviews.gem5.org/r/1031/diff/1/?file=23032#file23032line110> > > > > Does this actually need to be FullSystem only? Yes, because otherwise two SimpleThread objects will be created. If we changed the SimpleThread object to have one constructor instead of two where one is just for FS and one for SE configs (the two constructors take different args and do different things) this should go away. > On Feb. 9, 2012, 1:55 a.m., Gabe Black wrote: > > src/cpu/checker/cpu_impl.hh, line 402 > > <http://reviews.gem5.org/r/1031/diff/1/?file=23033#file23033line402> > > > > Why does this need to be FullSystem only? Yes. In SE mode faults just happen and no PC change occurs right? Only in FS will the PC be vectored to a fault handler. > On Feb. 9, 2012, 1:55 a.m., Gabe Black wrote: > > src/cpu/simple/BaseSimpleCPU.py, line 41 > > <http://reviews.gem5.org/r/1031/diff/1/?file=23053#file23053line41> > > > > A checker on the simple CPU doesn't seem to make any sense. This could > > error out all the time, and it should set the checker to NULL since that's > > the inert setting. If the function addCheckerCpu() isn't called then no checker is added to the cpu model. See the above comment as to why a dummy checker is still needed. > On Feb. 9, 2012, 1:55 a.m., Gabe Black wrote: > > src/arch/arm/isa.cc, line 291 > > <http://reviews.gem5.org/r/1031/diff/1/?file=23025#file23025line291> > > > > Is the dynamic_cast necessary? What type does getCheckerCpuPtr return? > > If you remove it here, remove it later on too. It returns a pointer to the BaseCPU type. The dynamic_cast is technically necessary. - Geoffrey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1031/#review2098 ----------------------------------------------------------- On Feb. 8, 2012, 7:45 a.m., Geoffrey Blake wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1031/ > ----------------------------------------------------------- > > (Updated Feb. 8, 2012, 7:45 a.m.) > > > Review request for Default. > > > Description > ------- > > CheckerCPU: Make CheckerCPU runtime selectable instead of compile selectable > > Enables the CheckerCPU to be enabled at runtime with the --checker option > from the configs/example/fs.py and configs/example/se.py configuration > files. Also merges with the SE/FS changes. > > > Diffs > ----- > > SConstruct 8f354c5a1634 > configs/common/Options.py 8f354c5a1634 > configs/common/Simulation.py 8f354c5a1634 > configs/example/fs.py 8f354c5a1634 > configs/example/se.py 8f354c5a1634 > src/arch/SConscript 8f354c5a1634 > src/arch/arm/isa.cc 8f354c5a1634 > src/arch/arm/utility.cc 8f354c5a1634 > src/cpu/BaseCPU.py 8f354c5a1634 > src/cpu/SConscript 8f354c5a1634 > src/cpu/base.cc 8f354c5a1634 > src/cpu/base_dyn_inst.hh 8f354c5a1634 > src/cpu/base_dyn_inst_impl.hh 8f354c5a1634 > src/cpu/checker/cpu.cc 8f354c5a1634 > src/cpu/checker/cpu_impl.hh 8f354c5a1634 > src/cpu/checker/thread_context.hh 8f354c5a1634 > src/cpu/o3/O3CPU.py 8f354c5a1634 > src/cpu/o3/SConscript 8f354c5a1634 > src/cpu/o3/commit_impl.hh 8f354c5a1634 > src/cpu/o3/cpu.hh 8f354c5a1634 > src/cpu/o3/cpu.cc 8f354c5a1634 > src/cpu/o3/cpu_builder.cc 8f354c5a1634 > src/cpu/o3/dyn_inst_impl.hh 8f354c5a1634 > src/cpu/o3/fetch_impl.hh 8f354c5a1634 > src/cpu/o3/iew_impl.hh 8f354c5a1634 > src/cpu/o3/lsq_unit_impl.hh 8f354c5a1634 > src/cpu/o3/thread_context.hh 8f354c5a1634 > src/cpu/o3/thread_context_impl.hh 8f354c5a1634 > src/cpu/ozone/OzoneCPU.py 8f354c5a1634 > src/cpu/ozone/SConscript 8f354c5a1634 > src/cpu/ozone/cpu_impl.hh 8f354c5a1634 > src/cpu/ozone/front_end_impl.hh 8f354c5a1634 > src/cpu/ozone/lw_back_end_impl.hh 8f354c5a1634 > src/cpu/ozone/lw_lsq_impl.hh 8f354c5a1634 > src/cpu/simple/BaseSimpleCPU.py 8f354c5a1634 > src/cpu/simple/base.hh 8f354c5a1634 > src/cpu/simple/base.cc 8f354c5a1634 > src/cpu/simple_thread.hh 8f354c5a1634 > src/cpu/thread_context.hh 8f354c5a1634 > > Diff: http://reviews.gem5.org/r/1031/diff/diff > > > Testing > ------- > > Compiles with ARM ISA. > Boots linux with O3 model attached to Checker in FS mode. > Runs simple HelloWorld in SE mode. > > > Thanks, > > Geoffrey Blake > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
