> On Aug. 5, 2016, 7:15 a.m., Andreas Hansson wrote:
> > I see how this works as a stop gap, but ultimately I would like to push for 
> > the removal of the shadow memory as the first option. Is it really that 
> > much effort?

I'm not personally familiar enough with why the shadow memory is needed to be 
able to say how much effort it would take to remove, but I believe so.


> On Aug. 5, 2016, 7:15 a.m., Andreas Hansson wrote:
> > src/mem/physical.cc, line 142
> > <http://reviews.gem5.org/r/3580/diff/3/?file=57445#file57445line142>
> >
> >     Same oddity with the configuration here. We should check each and every 
> > memory.

I've changed it to now do that. It doesn't make sense to kvmMap some members of 
an interleaved group and not others, so it is now a fatal error if the values 
don't all match.


> On Aug. 5, 2016, 7:15 a.m., Andreas Hansson wrote:
> > src/mem/physical.cc, line 85
> > <http://reviews.gem5.org/r/3580/diff/3/?file=57445#file57445line85>
> >
> >     I find the control flow here very odd. The kvm_addr_map is initialised 
> > and set on a path different to where it is used? What memory is it 
> > affecting? Why is it sticky? What if one is not kvm mapped and the others 
> > are etc?

I was confused about how the backing stores were created for memories added to 
addrMap. I've updated the patch to check each memory individually.


> On Aug. 5, 2016, 7:15 a.m., Andreas Hansson wrote:
> > src/mem/ruby/system/Sequencer.cc, line 254
> > <http://reviews.gem5.org/r/3580/diff/3/?file=57446#file57446line254>
> >
> >     This seems completely unrelated.

Removed.


- David


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


On Aug. 5, 2016, 9: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, 9: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

Reply via email to