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

Alex Williamson alex.williamson at redhat.com
Mon Nov 7 20:26:36 UTC 2016


On Mon,  7 Nov 2016 14:50:25 -0500
Laine Stump <laine at laine.org> 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,

Alex

> +            }
> +
> +            if (!(pciDev = virPCIDeviceNew(hostAddr->domain,
> +                                           hostAddr->bus,
> +                                           hostAddr->slot,
> +                                           hostAddr->function))) {
> +                /* Even though libvirtd is running with privileges, we
> +                 * still couldn't read the PCI config. So either
> +                 * device doesn't currently exist on the host, or
> +                 * libvirt is running unprivileged. Since the
> +                 * overwhelming majority of assignable host devices
> +                 * are PCIe, assume that.
> +                 */
> +                return pcieFlags;
> +            }
> +
> +            isExpress = virPCIDeviceIsPCIExpress(pciDev);
> +            virPCIDeviceFree(pciDev);
> +
> +            if (isExpress)
> +                return pcieFlags;
> +            else
> +                return pciFlags;
> +        }
> +        return 0;
> +    }
>  
>      case VIR_DOMAIN_DEVICE_MEMBALLOON:
>          switch ((virDomainMemballoonModel) dev->data.memballoon->model) {




More information about the libvir-list mailing list