Re: [PATCH v3 11/18] qemu: assign virtio devices to PCIe slot when appropriate

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2016-09-20 at 15:14 -0400, Laine Stump wrote:
> libvirt previously assigned nearly all devices to a "hotpluggable"
> legacy PCI slot even on machines with a PCIe root bus (and even though
> most such machines don't even support hotplug on legacy PCI slots!)
> Forcing all devices onto legacy PCI slots means that the domain will
> need a dmi-to-pci-bridge (to convert from PCIe to legacy PCI) and a
> pci-bridge (to provide hotpluggable legacy PCI slots which, again,
> usually aren't hotpluggable anyway).
> 
> To help reduce the need for these legacy controllers, this patch tries
> to assign virtio-1.0-capable devices to PCIe slots whenever possible,
> by setting appropriate connectFlags in
> virDomainDeviceConnectFlagsInternal(). Happily, when that function was
> written (just a few commits ago) it was created with a "virtioFlags"
> argument, set by both of its callers, which is the proper connectFlags
> to set for any virtio-*-pci device - depending on the arch/machinetype
> of the domain, and whether or not the qemu binary supports virtio-1.0,
> that flag will have either been set to PCI or PCIE. This patch merely
> enables the functionality by setting the flags for the device to
> whatever is in virtioFlags if the device is a virtio-*-pci device.
> 
> NB: since the slot must be hotpluggable, and pcie-root (the PCIe root
> complex) does *not* support hotplug, this means that suitable
> controllers must also be in the config (i.e. either pcie-root-port, or
> pcie-downstream-port). For now, libvirt doesn't add those
> automatically, so if you put virtio devices in a config for a qemu
> that has PCIe-capable virtio devices, you'll need to add extra
> pcie-root-ports yourself. That requirement will be eliminated in a
> future patch, but for now, it's simple to do this:
> 
>    <controller type='pci' model='pcie-root-port'/>
>    <controller type='pci' model='pcie-root-port'/>
>    <controller type='pci' model='pcie-root-port'/>
>    ...
> 
> Partially Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1330024

[...]
> @@ -475,20 +482,26 @@ qemuDomainDeviceConnectFlagsInternal(virDomainDeviceDefPtr dev,
>          break;
>  
>      case VIR_DOMAIN_DEVICE_DISK:
> -        if (dev->data.disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
> -            flags = 0; /* only virtio disks use PCI */
> +        if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)
> +            flags =  virtioFlags;

Double space.

[...]
> @@ -1692,6 +1692,51 @@ mymain(void)
>              QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1,
>              QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
>              QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
> +    /* verify that devices with pcie capability are assigned to a pcie slot */
> +    DO_TEST("q35-pcie",
> +            QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY,
> +            QEMU_CAPS_DEVICE_VIRTIO_RNG,
> +            QEMU_CAPS_OBJECT_RNG_RANDOM,
> +            QEMU_CAPS_NETDEV,
> +            QEMU_CAPS_DEVICE_VIRTIO_NET,
> +            QEMU_CAPS_DEVICE_VIRTIO_GPU,
> +            QEMU_CAPS_DEVICE_VIRTIO_GPU_VIRGL,
> +            QEMU_CAPS_VIRTIO_KEYBOARD,
> +            QEMU_CAPS_VIRTIO_MOUSE,
> +            QEMU_CAPS_VIRTIO_TABLET,
> +            QEMU_CAPS_VIRTIO_INPUT_HOST,
> +            QEMU_CAPS_VIRTIO_SCSI,
> +            QEMU_CAPS_FSDEV,
> +            QEMU_CAPS_FSDEV_WRITEOUT,
> +            QEMU_CAPS_DEVICE_PCI_BRIDGE,
> +            QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
> +            QEMU_CAPS_DEVICE_IOH3420,
> +            QEMU_CAPS_ICH9_AHCI,
> +            QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1,
> +            QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
> +    /* same XML as q35-pcie, but don't set QEMU_CAPS_VIRTIO_PCI_LEGACY,

s/_PCI_LEGACY/_PCI_DISABLE_LEGACY/

Same in qemuxml2xmltest.

[...]
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml
> new file mode 100644
> index 0000000..60e29b5
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pcie.xml
> @@ -0,0 +1,149 @@
> +<domain type='qemu'>
> +  <name>q35-test</name>
> +  <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid>
> +  <memory unit='KiB'>2097152</memory>
> +  <currentMemory unit='KiB'>2097152</currentMemory>
> +  <vcpu placement='static' cpuset='0-1'>2</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='q35'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/libexec/qemu-kvm</emulator>
> +    <disk type='block' device='disk'>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='vdb' bus='virtio'/>
> +      <address type='pci' domain='0x0000' bus='0x07' slot='0x00' function='0x0'/>
> +    </disk>
> +    <controller type='pci' index='0' model='pcie-root'/>
> +    <controller type='pci' index='1' model='dmi-to-pci-bridge'>
> +      <model name='i82801b11-bridge'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='2' model='pci-bridge'>
> +      <model name='pci-bridge'/>
> +      <target chassisNr='2'/>
> +      <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='3' model='pcie-root-port'>
> +      <model name='ioh3420'/>
> +      <target chassis='3' port='0x10'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='4' model='pcie-root-port'>
> +      <model name='ioh3420'/>
> +      <target chassis='4' port='0x18'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='5' model='pcie-root-port'>
> +      <model name='ioh3420'/>
> +      <target chassis='5' port='0x20'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='6' model='pcie-root-port'>
> +      <model name='ioh3420'/>
> +      <target chassis='6' port='0x28'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='7' model='pcie-root-port'>
> +      <model name='ioh3420'/>
> +      <target chassis='7' port='0x30'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='8' model='pcie-root-port'>
> +      <model name='ioh3420'/>
> +      <target chassis='8' port='0x38'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='9' model='pcie-root-port'>
> +      <model name='ioh3420'/>
> +      <target chassis='9' port='0x40'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='10' model='pcie-root-port'>
> +      <model name='ioh3420'/>
> +      <target chassis='10' port='0x48'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='11' model='pcie-root-port'>
> +      <model name='ioh3420'/>
> +      <target chassis='11' port='0x50'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='12' model='pcie-root-port'>
> +      <model name='ioh3420'/>
> +      <target chassis='12' port='0x58'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='13' model='pcie-root-port'>
> +      <model name='ioh3420'/>
> +      <target chassis='13' port='0x60'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/>
> +    </controller>
> +    <controller type='virtio-serial' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/>
> +    </controller>
> +    <controller type='scsi' index='0' model='virtio-scsi'>
> +      <address type='pci' domain='0x0000' bus='0x06' slot='0x00' function='0x0'/>
> +    </controller>
> +    <controller type='usb' index='0' model='ich9-ehci1'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x7'/>
> +    </controller>
> +    <controller type='usb' index='0' model='ich9-uhci1'>
> +      <master startport='0'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x0' multifunction='on'/>
> +    </controller>
> +    <controller type='usb' index='0' model='ich9-uhci2'>
> +      <master startport='2'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x1'/>
> +    </controller>
> +    <controller type='usb' index='0' model='ich9-uhci3'>
> +      <master startport='4'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x2'/>
> +    </controller>
> +    <controller type='sata' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
> +    </controller>
> +    <filesystem type='mount' accessmode='passthrough'>
> +      <source dir='/export/to/guest'/>
> +      <target dir='/import/from/host'/>
> +      <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/>
> +    </filesystem>
> +    <interface type='user'>
> +      <mac address='00:11:22:33:44:55'/>
> +      <model type='virtio'/>
> +      <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/>
> +    </interface>
> +    <input type='passthrough' bus='virtio'>
> +      <source evdev='/dev/input/event1234'/>
> +      <address type='pci' domain='0x0000' bus='0x0a' slot='0x00' function='0x0'/>
> +    </input>
> +    <input type='mouse' bus='virtio'>
> +      <address type='pci' domain='0x0000' bus='0x0b' slot='0x00' function='0x0'/>
> +    </input>
> +    <input type='keyboard' bus='virtio'>
> +      <address type='pci' domain='0x0000' bus='0x0c' slot='0x00' function='0x0'/>
> +    </input>
> +    <input type='tablet' bus='virtio'>
> +      <address type='pci' domain='0x0000' bus='0x0d' slot='0x00' function='0x0'/>
> +    </input>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <video>
> +      <model type='virtio' heads='1' primary='yes'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>

I was initially baffled by this, because I expected it to
be assigned to one of the available pcie-root-ports just
like all the other virtio devices.

However, according to qemuDomainValidateDevicePCISlotsQ35()
this is intentional, so I guess we're good :)

One improvement I can recommend to this test case is to add
one more pcie-root-port, so that it becomes quite clear
that virtio-vga has been assigned to pcie-root on purpose
and *not* due to a lack of pcie-root-ports.

ACK

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]