[libvirt] [PATCH] Reserve PCI addresses 3 and 4 on qemu-system-ppc

On PPC emulated by recent versions of qemu, PCI-bus 0 already has a macio connected in slot 3 and OHCI on slot 4. Reference: - https://bugzilla.redhat.com/show_bug.cgi?id=667345 Signed-off-by: Niels de Vos <ndevos@redhat.com> --- Imho this patch is relatively ugly. Unfortunately I did not find a better way of changing the reserved PCI-addresses on a per architecture basis. It can well be that qemuAssignDevicePCISlots() needs to be updated for other architectures as well. A more modular/dynamic approach might be more suitable. Feel free to suggest an alternative way to solve this. Thanks, Niels src/qemu/qemu_command.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f78ce71..50e3ad7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -983,6 +983,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) goto error; } + /* qemu-system-ppc has macio on slot 3 and ohci on slot 4 */ + if (STREQ(def->os.arch, "ppc")) { + if (qemuDomainPCIAddressReserveSlot(addrs, 3) < 0) + goto error; + if (qemuDomainPCIAddressReserveSlot(addrs, 4) < 0) + goto error; + } + /* Network interfaces */ for (i = 0; i < def->nnets ; i++) { if (def->nets[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) -- 1.7.4

On Sat, Feb 05, 2011 at 04:45:15PM +0000, Niels de Vos wrote:
On PPC emulated by recent versions of qemu, PCI-bus 0 already has a macio connected in slot 3 and OHCI on slot 4.
Reference: - https://bugzilla.redhat.com/show_bug.cgi?id=667345
Signed-off-by: Niels de Vos <ndevos@redhat.com> --- Imho this patch is relatively ugly. Unfortunately I did not find a better way of changing the reserved PCI-addresses on a per architecture basis. It can well be that qemuAssignDevicePCISlots() needs to be updated for other architectures as well. A more modular/dynamic approach might be more suitable.
Feel free to suggest an alternative way to solve this. Thanks, Niels
src/qemu/qemu_command.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f78ce71..50e3ad7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -983,6 +983,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) goto error; }
+ /* qemu-system-ppc has macio on slot 3 and ohci on slot 4 */ + if (STREQ(def->os.arch, "ppc")) { + if (qemuDomainPCIAddressReserveSlot(addrs, 3) < 0) + goto error; + if (qemuDomainPCIAddressReserveSlot(addrs, 4) < 0) + goto error; + } + /* Network interfaces */ for (i = 0; i < def->nnets ; i++) { if (def->nets[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
Oh fun, it is actually worse than this. On PPC slot 1 is occupied by the VGA adapter, while slot 2 is the IDE controller, so we'll need to make some more complex changes here. We're gonna need to change this for every other arch too, in fact it looks like it probably differs for each '-M' arg value too :-( I'm wondering whether we should just have a separate method for each combo rather than trying to make one method do everything. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/07/2011 05:13 AM, Daniel P. Berrange wrote:
Oh fun, it is actually worse than this. On PPC slot 1 is occupied by the VGA adapter, while slot 2 is the IDE controller, so we'll need to make some more complex changes here.
We're gonna need to change this for every other arch too, in fact it looks like it probably differs for each '-M' arg value too :-(
I'm wondering whether we should just have a separate method for each combo rather than trying to make one method do everything.
Definitely sounds like the sort of thing where we need an arch-specific callback function that can report the correct reservation information for that architecture (similar to how we already have arch-specific callbacks for computing cpu features). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Mon, Feb 7, 2011 at 5:12 PM, Eric Blake <eblake@redhat.com> wrote:
On 02/07/2011 05:13 AM, Daniel P. Berrange wrote:
Oh fun, it is actually worse than this. On PPC slot 1 is occupied by the VGA adapter, while slot 2 is the IDE controller, so we'll need to make some more complex changes here.
We're gonna need to change this for every other arch too, in fact it looks like it probably differs for each '-M' arg value too :-(
I'm wondering whether we should just have a separate method for each combo rather than trying to make one method do everything.
Definitely sounds like the sort of thing where we need an arch-specific callback function that can report the correct reservation information for that architecture (similar to how we already have arch-specific callbacks for computing cpu features).
Okay, makes sense to me. Would extending "struct qemu_arch_info" in src/qemu/qemu_capabilities.c be suitable enough? We could add a function pointer that reserves some PCI-slots where needed. Currently the table already supports different archs and machines (machines seems NULL everywhere though). Any objections if I have a look into adding this functionality there? (Note, I don't have any no timeframe, it's purely hobby for me.) Thanks, Niels

On 02/09/2011 05:55 AM, Niels de Vos wrote:
I'm wondering whether we should just have a separate method for each combo rather than trying to make one method do everything.
Definitely sounds like the sort of thing where we need an arch-specific callback function that can report the correct reservation information for that architecture (similar to how we already have arch-specific callbacks for computing cpu features).
Okay, makes sense to me. Would extending "struct qemu_arch_info" in src/qemu/qemu_capabilities.c be suitable enough?
Certainly sounds like the right place.
We could add a function pointer that reserves some PCI-slots where needed. Currently the table already supports different archs and machines (machines seems NULL everywhere though).
Any objections if I have a look into adding this functionality there? (Note, I don't have any no timeframe, it's purely hobby for me.)
Feel free to submit a patch, and to take as long as you need - the beauty of open source is that everyone scratches their own itches in the timeframe that they need. When you do post a patch, we'll be sure to review it based on its merits, and either take it as is or give you feedback for improving it further. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Niels de Vos