Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.
On Dec. 8, 2014, 2:36 p.m., Andreas Hansson wrote: src/cpu/kvm/vm.cc, line 374 http://reviews.gem5.org/r/2510/diff/2/?file=42734#file42734line374 I think this causes problems with some of the officially supported compilers. It's just a hunch, but please check. Gabe Black wrote: Ugh. Yes, I think you're right. That's standard C but not C++. I changed it to just initialize the elements one by one. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/#review5653 --- On Nov. 23, 2014, 2:51 p.m., Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/ --- (Updated Nov. 23, 2014, 2:51 p.m.) Review request for Default. Repository: gem5 Description --- Changeset 10551:7767dc21318d --- Let other objects set up memory like regions in a KVM VM. Diffs - src/cpu/kvm/vm.hh f9fb64a72259a2514080151b5250a04c575d443a src/cpu/kvm/vm.cc f9fb64a72259a2514080151b5250a04c575d443a Diff: http://reviews.gem5.org/r/2510/diff/ Testing --- Thanks, Gabe Black ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/#review5652 --- Ship it! I'm still not happy with the setupMemSlot()/disableMemSlot() names, but I can't think of any better names atm, so let's go with them for now. - Andreas Sandberg On Nov. 23, 2014, 2:51 p.m., Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/ --- (Updated Nov. 23, 2014, 2:51 p.m.) Review request for Default. Repository: gem5 Description --- Changeset 10551:7767dc21318d --- Let other objects set up memory like regions in a KVM VM. Diffs - src/cpu/kvm/vm.hh f9fb64a72259a2514080151b5250a04c575d443a src/cpu/kvm/vm.cc f9fb64a72259a2514080151b5250a04c575d443a Diff: http://reviews.gem5.org/r/2510/diff/ Testing --- Thanks, Gabe Black ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/#review5653 --- src/cpu/kvm/vm.cc http://reviews.gem5.org/r/2510/#comment5046 I think this causes problems with some of the officially supported compilers. It's just a hunch, but please check. - Andreas Hansson On Nov. 23, 2014, 2:51 p.m., Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/ --- (Updated Nov. 23, 2014, 2:51 p.m.) Review request for Default. Repository: gem5 Description --- Changeset 10551:7767dc21318d --- Let other objects set up memory like regions in a KVM VM. Diffs - src/cpu/kvm/vm.hh f9fb64a72259a2514080151b5250a04c575d443a src/cpu/kvm/vm.cc f9fb64a72259a2514080151b5250a04c575d443a Diff: http://reviews.gem5.org/r/2510/diff/ Testing --- Thanks, Gabe Black ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.
On Dec. 8, 2014, 2:36 p.m., Andreas Hansson wrote: src/cpu/kvm/vm.cc, line 374 http://reviews.gem5.org/r/2510/diff/2/?file=42734#file42734line374 I think this causes problems with some of the officially supported compilers. It's just a hunch, but please check. Ugh. Yes, I think you're right. That's standard C but not C++. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/#review5653 --- On Nov. 23, 2014, 2:51 p.m., Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/ --- (Updated Nov. 23, 2014, 2:51 p.m.) Review request for Default. Repository: gem5 Description --- Changeset 10551:7767dc21318d --- Let other objects set up memory like regions in a KVM VM. Diffs - src/cpu/kvm/vm.hh f9fb64a72259a2514080151b5250a04c575d443a src/cpu/kvm/vm.cc f9fb64a72259a2514080151b5250a04c575d443a Diff: http://reviews.gem5.org/r/2510/diff/ Testing --- Thanks, Gabe Black ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.
I think it’s fine that a device wants to do this, I’d just like it to use an thin interface on the System object as a matter of clean interfaces in the object hierarchy, so unrelated objects don’t have to know about each other. Ali On Dec 3, 2014, at 11:54 AM, Gabe Black via gem5-dev gem5-dev@gem5.org wrote: On Dec. 3, 2014, 11:42 a.m., Andreas Hansson wrote: As this is quite invasive, how broadly would this be used? Also, is there any chance of rather using devices that do not have memory of their own (and rely on the normal system memory that is already mapped)? The one example I'm aware of is the cirrus graphics device on x86, and unfortunately the existing driver assumes there's memory on the card. Our team is reluctant to spend a significant amount of effort either writing a new driver or specializing the image to switch to a different video device. QEMU uses cirrus already, and since we use that for VM based testing we already have support for it in place. It's all a matter of opinion of course, but this doesn't seem that invasive to me. It's making the memory slot management a little bit more robust, and exposing it to external consumers. KVM is already a generalized/standardized interface to hardware virtualization, so it's not reaching behind the curtain all that much. There is some impact, but it seems like a reasonable trade off to me. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/#review5613 --- On Nov. 23, 2014, 2:51 p.m., Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/ --- (Updated Nov. 23, 2014, 2:51 p.m.) Review request for Default. Repository: gem5 Description --- Changeset 10551:7767dc21318d --- Let other objects set up memory like regions in a KVM VM. Diffs - src/cpu/kvm/vm.hh f9fb64a72259a2514080151b5250a04c575d443a src/cpu/kvm/vm.cc f9fb64a72259a2514080151b5250a04c575d443a Diff: http://reviews.gem5.org/r/2510/diff/ Testing --- Thanks, Gabe Black ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/#review5611 --- Ping - Gabe Black On Nov. 23, 2014, 2:51 p.m., Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/ --- (Updated Nov. 23, 2014, 2:51 p.m.) Review request for Default. Repository: gem5 Description --- Changeset 10551:7767dc21318d --- Let other objects set up memory like regions in a KVM VM. Diffs - src/cpu/kvm/vm.hh f9fb64a72259a2514080151b5250a04c575d443a src/cpu/kvm/vm.cc f9fb64a72259a2514080151b5250a04c575d443a Diff: http://reviews.gem5.org/r/2510/diff/ Testing --- Thanks, Gabe Black ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/#review5613 --- As this is quite invasive, how broadly would this be used? Also, is there any chance of rather using devices that do not have memory of their own (and rely on the normal system memory that is already mapped)? - Andreas Hansson On Nov. 23, 2014, 2:51 p.m., Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/ --- (Updated Nov. 23, 2014, 2:51 p.m.) Review request for Default. Repository: gem5 Description --- Changeset 10551:7767dc21318d --- Let other objects set up memory like regions in a KVM VM. Diffs - src/cpu/kvm/vm.hh f9fb64a72259a2514080151b5250a04c575d443a src/cpu/kvm/vm.cc f9fb64a72259a2514080151b5250a04c575d443a Diff: http://reviews.gem5.org/r/2510/diff/ Testing --- Thanks, Gabe Black ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.
On Dec. 3, 2014, 11:42 a.m., Andreas Hansson wrote: As this is quite invasive, how broadly would this be used? Also, is there any chance of rather using devices that do not have memory of their own (and rely on the normal system memory that is already mapped)? The one example I'm aware of is the cirrus graphics device on x86, and unfortunately the existing driver assumes there's memory on the card. Our team is reluctant to spend a significant amount of effort either writing a new driver or specializing the image to switch to a different video device. QEMU uses cirrus already, and since we use that for VM based testing we already have support for it in place. It's all a matter of opinion of course, but this doesn't seem that invasive to me. It's making the memory slot management a little bit more robust, and exposing it to external consumers. KVM is already a generalized/standardized interface to hardware virtualization, so it's not reaching behind the curtain all that much. There is some impact, but it seems like a reasonable trade off to me. - Gabe --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/#review5613 --- On Nov. 23, 2014, 2:51 p.m., Gabe Black wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/ --- (Updated Nov. 23, 2014, 2:51 p.m.) Review request for Default. Repository: gem5 Description --- Changeset 10551:7767dc21318d --- Let other objects set up memory like regions in a KVM VM. Diffs - src/cpu/kvm/vm.hh f9fb64a72259a2514080151b5250a04c575d443a src/cpu/kvm/vm.cc f9fb64a72259a2514080151b5250a04c575d443a Diff: http://reviews.gem5.org/r/2510/diff/ Testing --- Thanks, Gabe Black ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.
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 gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/ --- (Updated Nov. 23, 2014, 2:51 p.m.) Review request for Default. Repository: gem5 Description (updated) --- Changeset 10551:7767dc21318d --- Let other objects set up memory like regions in a KVM VM. Diffs (updated) - src/cpu/kvm/vm.hh f9fb64a72259a2514080151b5250a04c575d443a src/cpu/kvm/vm.cc f9fb64a72259a2514080151b5250a04c575d443a Diff: http://reviews.gem5.org/r/2510/diff/ Testing --- Thanks, Gabe Black ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
Re: [gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/#review5489 --- src/cpu/kvm/vm.hh http://reviews.gem5.org/r/2510/#comment4940 The returned slot ID should be typedef:ed, or preferably a struct since that would make type checking more reliable. src/cpu/kvm/vm.hh http://reviews.gem5.org/r/2510/#comment4944 Would it make sense to rename this to mapMemSlot? In my opinion, that'd be more descriptive. src/cpu/kvm/vm.hh http://reviews.gem5.org/r/2510/#comment4942 Indentation is inconsistent with the rest of the file. src/cpu/kvm/vm.hh http://reviews.gem5.org/r/2510/#comment4941 What's this group comment doing here and where is it terminated? src/cpu/kvm/vm.hh http://reviews.gem5.org/r/2510/#comment4945 How about renaming this to unmapMemSlot? src/cpu/kvm/vm.cc http://reviews.gem5.org/r/2510/#comment4943 Inconsistent indentation. Overall, I'd prefer this to be an internal API and have some way of notifying the VM through the System instead of allowing objects to poke around directly. See my reply in the email thread for RB #2513. - Andreas Sandberg 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 gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Review Request 2510: Let other objects set up memory like regions in a KVM VM.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2510/ --- 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 gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
