> On July 27, 2016, 4:43 p.m., Andreas Hansson wrote: > > I would really prefer if we didn't have to add this complexity. > > > > I thought we were pretty much in a position where we could remove the > > horrible shadow memory hack. Could we not pursue that route rather? It > > would be a much better solution long term. > > David Hashe wrote: > That may be a better longterm solution, but until then this is necessary > to get KVM working with the APU model. I rewrote the patch to remove the most > complicated C++ changes. Now, instead of detecting the shadow memory using a > heuristic, which was admittedly a bit overcomplicated and overspecific to the > ruby shadow memory, we add a parameter to AbstractMemory explicitly saying > whether a memory should be mapped by KVM. > > This has several advantages. Now the only code tying this patch to the > shadow memory is in the configuration files. It is also generally useful to > have a way to stop KVM from accessing certain memories. For example, if a > hypothetical configuration script created a separate memory for a device that > could not be accessed directly by the CPU, then it would be pointless for KVM > to map that memory, and doing so could even introduce difficult-to-detect > errors if KVM were to erroneously write to that memory during acceleration. > With this patch it would be possible to avoid that by telling KVM not to map > the memory region. > > Is this acceptable?
I agree that this feature makes sense. For instance, I had some similar code for simulating a system with a DRAM cache with KVM. With at least two reasons for this I think it passes the bar for being in mainline gem5 :). - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3580/#review8551 ----------------------------------------------------------- On Aug. 5, 2016, 1:39 a.m., David Hashe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3580/ > ----------------------------------------------------------- > > (Updated Aug. 5, 2016, 1:39 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11562:7375e1f533fa > --------------------------- > cpu, mem, sim: Enable KVM support for Ruby > > Only map memories into the KVM guest address space that are > marked as usable by KVM. > > Remember whether a BackingStoreEntry should be mapped by KVM. > > Fix bug causing incomplete draining of Ruby Sequencer. > > > Diffs > ----- > > src/cpu/kvm/vm.cc 704b0198f747b766b839c577614eb2924fd1dfee > src/mem/AbstractMemory.py 704b0198f747b766b839c577614eb2924fd1dfee > src/mem/abstract_mem.hh 704b0198f747b766b839c577614eb2924fd1dfee > src/mem/abstract_mem.cc 704b0198f747b766b839c577614eb2924fd1dfee > src/mem/physical.hh 704b0198f747b766b839c577614eb2924fd1dfee > src/mem/physical.cc 704b0198f747b766b839c577614eb2924fd1dfee > src/mem/ruby/system/Sequencer.cc 704b0198f747b766b839c577614eb2924fd1dfee > > Diff: http://reviews.gem5.org/r/3580/diff/ > > > Testing > ------- > > > Thanks, > > David Hashe > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
