> On Nov. 19, 2014, 4:39 p.m., Andreas Sandberg wrote:
> > src/cpu/kvm/vm.hh, line 354
> > <http://reviews.gem5.org/r/2510/diff/1/?file=42635#file42635line354>
> >
> >     Would it make sense to rename this to mapMemSlot? In my opinion, that'd 
> > be more descriptive.

No, I don't think so. A slot isn't mapped or unmapped, it's what keeps track of 
a mapping between the guest and the host. The guest region is mapped, but I 
don't want to change this to mapMemRegion because the thing being operated on 
is the slot, not the region. With that name, the slot parameter seems 
extraneous.


> On Nov. 19, 2014, 4:39 p.m., Andreas Sandberg wrote:
> > src/cpu/kvm/vm.hh, line 359
> > <http://reviews.gem5.org/r/2510/diff/1/?file=42635#file42635line359>
> >
> >     What's this group comment doing here and where is it terminated?

A left over from earlier edits. I'll get rid of it.


> On Nov. 19, 2014, 4:39 p.m., Andreas Sandberg wrote:
> > src/cpu/kvm/vm.hh, line 361
> > <http://reviews.gem5.org/r/2510/diff/1/?file=42635#file42635line361>
> >
> >     How about renaming this to unmapMemSlot?

See above.


> On Nov. 19, 2014, 4:39 p.m., Andreas Sandberg wrote:
> > src/cpu/kvm/vm.hh, line 344
> > <http://reviews.gem5.org/r/2510/diff/1/?file=42635#file42635line344>
> >
> >     The returned slot ID should be typedef:ed, or preferably a struct since 
> > that would make type checking more reliable.

I considered it briefly, but then I decided not to worry about it then and 
forgot about it. It wouldn't be a bad idea.


- Gabe


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


On Nov. 18, 2014, 1:29 a.m., Gabe Black wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/2510/
> -----------------------------------------------------------
> 
> (Updated Nov. 18, 2014, 1:29 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 10546:b4c9aa186307
> ---------------------------
> Let other objects set up memory like regions in a KVM VM.
> 
> 
> Diffs
> -----
> 
>   src/cpu/kvm/vm.hh f66948658a36b6874e84ee5da37e70d351287cb4 
>   src/cpu/kvm/vm.cc f66948658a36b6874e84ee5da37e70d351287cb4 
> 
> Diff: http://reviews.gem5.org/r/2510/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gabe Black
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to