Re: [libvirt] [PATCH] qemu: Permit PCI-free aarch64 mach-virt guests

On 06/17/2016 08:43 AM, Andrea Bolognani wrote: > There has been some progress lately in enabling virtio-pci on > aarch64 guests; however, guest OS support is still spotty at best, > so most guests are going to be using virtio-mmio instead. >  > Currently, mach-virt guests are closely modeled after q35 guests, > and that includes always adding a dmi-to-pci-bridge that's just > impossible to get rid of.   Yeah. Sorry my simple patches from yesterday didn't get you as far as  you needed to go.

>            * other than the pcie-root. This is so that there will be hot-pluggable > -         * PCI slots available > +         * PCI slots available. > +         * > +         * We skip this step for aarch64 mach-virt guests, where we want to > +         * be able to have a pure virtio-mmio topology >            */ >           if (virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1) < 0 && > +            !qemuDomainMachineIsVirt(def) &&   You're assuming that the only virt* machinetypes will be aarch64, which  may be reasonable now, but not in the future (periodically someone from  qemu will mention the idea of a "virt" machinetype for x86, which is  legacy-free and accepts only virtio devices). Wouldn't a more specific  comparison be better here (and in the other places in this patch)?

>               !virDomainDefAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1, >                                          VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE)) >               goto cleanup;   Okay, so pcie-root is still always "there", which isn't a problem since  the presence of pcie-root in the config doesn't actually change anything  in the qemu commandline or resulting virtual machine (it's a default  device in qemu that can't be removed).

> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c > index ca3adc0..883264a 100644 > --- a/src/qemu/qemu_domain_address.c > +++ b/src/qemu/qemu_domain_address.c > @@ -1483,9 +1483,15 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, >           } >    >           /* Reserve 1 extra slot for a (potential) bridge only if buses > -         * are not fully reserved yet > +         * are not fully reserved yet. > +         * > +         * We don't reserve the extra slot for aarch64 mach-virt guests > +         * either because we want to be able to have pure virtio-mmio > +         * guests, and reserving this slot would force us to add at least > +         * a dmi-to-pci-bridge to an otherwise PCI-free topology >            */ >           if (!buses_reserved && > +            !qemuDomainMachineIsVirt(def) && >               virDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0) >               goto cleanup;   You could save some time by just putting the whole thing from "for (i =  0; i < addrs->nbuses; i++)" down to that "goto cleanup" inside an "if  (!qemuDomainMachineIsVirt(def)) { ... }". (maybe replacing  qemuDomainMachineIsVirt() with something more specific, as noted above).

As we've discussed in IRC, eventually there should be a way to disable  this for *every* platform, not just aarch64/virt. Possibly an  "openPCISlots" attribute somewhere in the domain that tells how many  slots should be left open for hotplug (with default being 1 for  x86/440fx, and up for discussion on other platforms).

>         <target dev='sda' bus='scsi'/> >         <address type='drive' controller='0' bus='0' target='0' unit='0'/> >       </disk> > +    <controller type='pci' index='0' model='pcie-root'/>   This surprised me for a minute, since you're now explicitly adding  pcie-root even though it's still implicitly added for you (and, for  example, qemuxml2argvtest completes successfully without it). But I  guess it doesn't hurt anything to have it in the file, as it makes it  obvious that the dmi-to-pci-bridge isn't just being added on top of nothing.

(Hopefully all of this will soon be a thing of the past - for *all*  arches/machinetypes if you put in a device with <address type='pci'/>  (or a device that can only be connected via PCI), then all the necessary  pci controllers should just magically appear, otherwise not).

ACK.