> 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.
> 
> Geoffrey Blake wrote:
>     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.

That doesn't sound like a bad idea, although you don't need to worry about that 
now I don't think.


> 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.
> 
> Geoffrey Blake wrote:
>     It returns a pointer to the BaseCPU type. The dynamic_cast is technically 
> necessary.

The dynamic_cast is basically necessary *if* it returns a BaseCPU pointer. Why 
does it do that instead of returning a CheckerCPU pointer? Is CheckerCPU really 
an O3 thing and not a generic class? If it's generic it makes sense to return 
it directly. If not, maybe something like O3CheckerCPU would be a better name, 
although renaming it is outside the scope of your change.


> 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?
> 
> Geoffrey Blake wrote:
>     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.

I see. This is so there's hookups to actually switch the real checker into when 
the O3 takes over.


> 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.
> 
> Geoffrey Blake wrote:
>     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.

Ok


> 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?
> 
> Geoffrey Blake wrote:
>     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.

Ok


> 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?
> 
> Geoffrey Blake wrote:
>     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.

No. In SE faults *usually* just happen (typically panicing or updating the 
TLB), but that's not a hard rule. In SPARC specifically there are faults that 
do register window spills and fills in SE mode in actual handlers injected into 
memory before the process starts.


- Gabe


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


On Feb. 10, 2012, 3:24 p.m., Geoffrey Blake wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1031/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2012, 3:24 p.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

Reply via email to