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: [email protected] [mailto:[email protected]] 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 > On 2011-12-13 13:35:40, Geoffrey Blake wrote: > > 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? > > Ali Saidi wrote: > 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? > 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 ----------------------------------------------------------- On 2011-12-13 13:25:24, Geoffrey Blake wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > 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 [email protected] 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 [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
