Re: [libvirt] [PATCH v6 17/17] qemu: assign vfio devices to PCIe addresses when appropriate

On Mon, 7 Nov 2016 14:50:25 -0500 Laine Stump <laine(a)laine.org&gt; wrote: > Although nearly all host devices that are assigned to guests using > vfio ("<hostdev>" devices in libvirt) are physically PCI Express > devices, until now libvirt's PCI address assignment has always > assigned them addresses on legacy PCI controllers. > > This patch tries to assign them to an address on a PCIe controller > instead. First we do some preliminary checks that might allow setting > the flags without doing any extra work, and if those conditions aren't > met (and if libvirt is running privileged so that it has proper > permissions), we perform the (relatively) time consuming task of > reading the device's PCI config to see if it is an Express device. If > this is successful, the connect flags are set based on the result, but > if we aren't able to read the PCI config (most likely due to the > device not being present on the system at the time of the check) we > assume it is (or will be) and Express device, since that is almost > always the case anyway. > --- > src/qemu/qemu_domain_address.c | 66 ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 63 insertions(+), 3 deletions(-) > > diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c > index 65753c5..86984fb 100644 > --- a/src/qemu/qemu_domain_address.c > +++ b/src/qemu/qemu_domain_address.c > @@ -428,7 +428,7 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, > */ > static virDomainPCIConnectFlags > qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, > - virQEMUDriverPtr driver ATTRIBUTE_UNUSED, > + virQEMUDriverPtr driver, > virDomainPCIConnectFlags pcieFlags, > virDomainPCIConnectFlags virtioFlags) > { > @@ -558,8 +558,68 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, > return 0; > } > > - case VIR_DOMAIN_DEVICE_HOSTDEV: > - return pciFlags; > + case VIR_DOMAIN_DEVICE_HOSTDEV: { > + virDomainHostdevDefPtr hostdev = dev->data.hostdev; > + > + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && > + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { > + bool isExpress = false; > + virPCIDevicePtr pciDev; > + virPCIDeviceAddressPtr hostAddr = &hostdev->source.subsys.u.pci.addr; > + > + if (pciFlags == pcieFlags) { > + /* This arch/qemu only supports legacy PCI, so there > + * is no point in checking if the device is an Express > + * device. > + */ > + return pciFlags; > + } > + > + if (virDeviceInfoPCIAddressPresent(hostdev->info)) { > + /* A guest-side address has already been assigned, so > + * we can avoid reading the PCI config, and just use > + * pcieFlags, since the pciConnectFlags checking is > + * more relaxed when an address is already assigned > + * than it is when we're looking for a new address (so > + * validation will pass regardless of whether we set > + * the flags to PCI or PCIE). > + */ > + return pcieFlags; > + } > + > + if (!driver->privileged) { > + /* unprivileged libvirtd is unable to read a device's > + * PCI config, so instead of trying and failing, we > + * will just assume what is by far the most likely > + * version of reality: this is a PCIE device. > + */ > + return pcieFlags; Don't you still have permission to stat the config file? You can tell pretty well by whether the config file is 4096 bytes that the device is either PCIe or PCIx (and given the rarity of PCIx in most systems assume that it's PCIe). Conventional PCI will be 256 bytes. Thanks,