----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3580/#review8605 -----------------------------------------------------------
I think this makes sense overall. I can definitely see use cases where I would want to prevent KVM from punching through to the backing store for some memories. I have two high-level comments: 1. Could you make sure to update the commit message to reflect that this change now only deals with how KVM maps memory? The current message describes what the patch series does and not what this individual commit does (it has nothing to do with Ruby). 2. I like the idea of using a struct to describe the backing store, this makes the code much clearer. Could you make it a bit more symmetric and make sure the conf_table_reported flag is included in the struct as well? We should check that flag as well when mapping memories in kvm, but that should probably be a separate commit since it's a bug fix that's orthogonal to this. Thanks for sorting this out! - Andreas Sandberg On Aug. 5, 2016, 10:37 p.m., David Hashe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3580/ > ----------------------------------------------------------- > > (Updated Aug. 5, 2016, 10:37 p.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 > > 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
