> On Feb. 5, 2015, 4:16 p.m., Joel Hestness wrote: > > LGTM, also. > > > > I don't think it should be addressed in this patch, but I'm making notes > > here as a vision for how I think we should proceed with these backing store > > and protocol developments: > > > > Really what we're capturing with the backing store option is whether a Ruby > > coherence protocol requires the backing store, but *not* that the > > RubySystem needs it for any fundamental reason. In other words, requiring a > > backing store is a property of a particular coherence protocol > > implementation rather than the overall RubySystem. We've noted previously > > that protocol implementations can require backing store if they are not > > known to correctly buffer data in the cache controllers, or if they > > implement some functionality that cannot be easily modeled without a > > backing store (e.g. slim GPU atomics). The last couple changes to backing > > stores are major steps in the right direction: we've reduced the backing > > store specifier to a single option, rather than the prior structure, which > > disaggregated the variable and required that the programmer set the option > > on each of the RubySequencers (previously 'access_phys_mem'). However, the > > ultimate solution *should* be that each coherence protocol implementation specifies whether it requires the backing store. > > > > To this end, I propose that the next step for development on this should be > > to create an abstract Python wrapper object for Ruby protocol > > implementations that embeds this "requires backing store" property as a > > variable. All Ruby protocols could descend from that wrapper object, and we > > could handle the protocol as a Python object in config files rather than > > using the ugly Python exec calling convention we have in Ruby.py currently. > > If the protocol requires the backing store, Ruby should check for the > > requirement (e.g. accessor function in the abstract Python protocol object) > > and set up the backing store without the need for any command line > > parameters. If a developer decides to validate a protocol as "data > > correct", s/he would simply set the "requires backing store" variable to > > False for that protocol when the validation is completed. > > > > Andreas Hansson wrote: > I have a question: > > With this enabled, we now have two backing stores, the 1) "real" one > accessed through the interconnect, or directly through the system pointer > (system->getPhysMem()->access(...)), 2) the "fake" one that is used by the > sequencers (RubySystem->getPhysMem() ...). Does Ruby properly update (1), or > are we saying that (1) is occasionally wrong and you shouldn't trust it? > > Joel Hestness wrote: > This is a really good question. When using the "fake" Ruby backing store > (2), all sequencers need to access it at the end of every memory access to > pull the functional current version of data (i.e. the coherent latest version > of the data). This requirement means that the "fake" Ruby store contains > versions of data that would be cached in the cache objects (controllers, > buffers, or caches themselves) if the Ruby protocol were known to be data > correct. All of the memory accesses that go through the caches to the > simulated system memory controller do access the "real" backing store (1) > also, so I believe this means that when using the "fake" Ruby backing store > (2), the "real" backing store (1) still contains a correct version of the > data as seen by the simulated system memory. The data there may not be > up-to-date with versions of data in the cache hierarchy, and that's why we're > using the "fake" store (2). NOTE: The fact that the "fake" Ruby store (2) > contains the "current version" of the data means that it cannot cache stale/incoherent versions of data like some coherence protocols may allow (e.g. NVIDIA Fermi GPU L1 caches). > > I too agree with Jason that your assessment seems correct: I think we > could provide this Ruby functional data correctness with a single backing > store if we had a way to functionally access it like we're currently > accessing the "fake" Ruby backing store.
This is a good question, Andreas, and one I hadn't thought about. To reiterate what Joel said, as currently designed, Ruby's backing store acts basically like a cache for the system data. It may have more up-to-date data than the system physMem. However, it may be reasonable to remove Ruby's backing store and only use the system's physmem. I vote to leave this for future patches. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2627/#review5840 ----------------------------------------------------------- On Feb. 5, 2015, 4:02 p.m., Jason Power wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2627/ > ----------------------------------------------------------- > > (Updated Feb. 5, 2015, 4:02 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10681:b2dc4aee1a0d > --------------------------- > Ruby: Update backing store option to propagate through to all RubyPorts > > Previously, the user would have to manually set access_backing_store=True > on all RubyPorts (Sequencers) in the config files. > Now, instead there is one global option that each RubyPort checks on > initialization. > > > Diffs > ----- > > configs/ruby/Ruby.py 7639c17357dc > src/mem/ruby/system/DMASequencer.hh 7639c17357dc > src/mem/ruby/system/DMASequencer.cc 7639c17357dc > src/mem/ruby/system/RubyPort.cc 7639c17357dc > src/mem/ruby/system/RubySystem.py 7639c17357dc > src/mem/ruby/system/Sequencer.py 7639c17357dc > src/mem/ruby/system/System.hh 7639c17357dc > src/mem/ruby/system/System.cc 7639c17357dc > > Diff: http://reviews.gem5.org/r/2627/diff/ > > > Testing > ------- > > > Thanks, > > Jason Power > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
