[libvirt] [PATCH v3 0/3] qemu: Use virtio-pci by default for mach-virt guests
Changes from [v2]: * rename qemuDomainCountVirtioMMIODevices() to qemuDomainHasVirtioMMIODevices() and make it exit as soon as the first virtio-mmio device is encountered, as suggested by Laine * tweak test suite and note no new test cases are needed * add comments and user documentation * add release notes entry * can actually be merged now that all patches it builds on have been merged :) Changes from [v1]: * use virDomainDeviceInfoIterate(), as suggested by Martin and Laine, which results in cleaner and more robust code [v1] https://www.redhat.com/archives/libvir-list/2016-October/msg00988.html [v2] https://www.redhat.com/archives/libvir-list/2016-October/msg01042.html Andrea Bolognani (3): qemu: Use virtio-pci by default for mach-virt guests docs: Document virtio-mmio by default for mach-virt guests NEWS: Update for virtio-pci by default for mach-virt guests docs/formatdomain.html.in | 8 +++- docs/news.html.in | 6 +++ src/qemu/qemu_domain_address.c | 51 ++++++++++++++++++++-- ...l2argv-aarch64-virt-2.6-virtio-pci-default.args | 14 +++--- .../qemuxml2argv-aarch64-virtio-pci-default.args | 17 +++++--- .../qemuxml2argv-aarch64-virtio-pci-default.xml | 3 -- tests/qemuxml2argvtest.c | 1 + .../qemuxml2xmlout-aarch64-virtio-pci-default.xml | 40 ++++++++++++++--- tests/qemuxml2xmltest.c | 1 + 9 files changed, 119 insertions(+), 22 deletions(-) -- 2.7.4
virtio-pci is the way forward for aarch64 guests: it's faster and less alien to people coming from other architectures. Now that guest support is finally getting there (Fedora 24, CentOS 7.3, Ubuntu 16.04 and Debian testing all support virtio-pci out of the box), we'd like to start using it by default instead of virtio-mmio. Users and applications can already opt-in by explicitly using <address type='pci'/> inside the relevant elements, but that's kind of cumbersome and requires all users and management applications to adapt, which we'd really like to avoid. What we can do instead is use virtio-mmio only if the guest already has at least one virtio-mmio device, and use virtio-pci in all other situations. That means existing virtio-mmio guests will keep using the old addressing scheme, and new guests will automatically be created using virtio-pci instead. Users can still override the default in either direction. Existing tests such as aarch64-aavmf-virtio-mmio and aarch64-virtio-pci-default already cover all possible scenarios, so no additions to the test suites are necessary. --- src/qemu/qemu_domain_address.c | 51 ++++++++++++++++++++-- ...l2argv-aarch64-virt-2.6-virtio-pci-default.args | 14 +++--- .../qemuxml2argv-aarch64-virtio-pci-default.args | 17 +++++--- .../qemuxml2argv-aarch64-virtio-pci-default.xml | 3 -- tests/qemuxml2argvtest.c | 1 + .../qemuxml2xmlout-aarch64-virtio-pci-default.xml | 40 ++++++++++++++--- tests/qemuxml2xmltest.c | 1 + 7 files changed, 106 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index be4ed23..d2f7953 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -398,6 +398,44 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def, } +static int +qemuDomainHasVirtioMMIODevicesCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr info, + void *opaque) +{ + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) { + /* We can stop iterating as soon as we find the first + * virtio-mmio device */ + *((bool *) opaque) = true; + return -1; + } + + return 0; +} + + +/** + * qemuDomainHasVirtioMMIODevices: + * @def: domain definition + * + * Scan @def looking for devices with a virtio-mmio address. + * + * Returns: true if there are any, false otherwise + */ +static bool +qemuDomainHasVirtioMMIODevices(virDomainDefPtr def) +{ + bool result = false; + + virDomainDeviceInfoIterate(def, + qemuDomainHasVirtioMMIODevicesCallback, + &result); + + return result; +} + + static void qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) @@ -410,9 +448,16 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, qemuDomainMachineIsVirt(def))) return; - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO)) { - qemuDomainPrimeVirtioDeviceAddresses( - def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO); + /* We use virtio-mmio by default on mach-virt guests only if they already + * have at least one virtio-mmio device: in all other cases, we prefer + * virtio-pci */ + if (qemuDomainMachineHasPCIeRoot(def) && + !qemuDomainHasVirtioMMIODevices(def)) { + qemuDomainPrimeVirtioDeviceAddresses(def, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI); + } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO)) { + qemuDomainPrimeVirtioDeviceAddresses(def, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO); } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args index 75db1a4..df03c6e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args @@ -21,14 +21,18 @@ QEMU_AUDIO_DRV=none \ -initrd /aarch64.initrd \ -append 'earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait' \ -dtb /aarch64.dtb \ --device virtio-serial-device,id=virtio-serial0 \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1 \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.2,addr=0x2 \ -drive file=/aarch64.raw,format=raw,if=none,id=drive-virtio-disk0 \ --device virtio-blk-device,drive=drive-virtio-disk0,id=virtio-disk0 \ --device virtio-net-device,vlan=0,id=net0,mac=52:54:00:09:a4:37 \ +-device virtio-blk-pci,bus=pci.2,addr=0x3,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-device virtio-net-pci,vlan=0,id=net0,mac=52:54:00:09:a4:37,bus=pci.2,addr=0x1 \ -net user,vlan=0,name=hostnet0 \ -serial pty \ -chardev pty,id=charconsole1 \ -device virtconsole,chardev=charconsole1,id=console1 \ --device virtio-balloon-device,id=balloon0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x4 \ -object rng-random,id=objrng0,filename=/dev/random \ --device virtio-rng-device,rng=objrng0,id=rng0 +-device virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.2,addr=0x5 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args index b5b010c..080b025 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args @@ -21,14 +21,21 @@ QEMU_AUDIO_DRV=none \ -initrd /aarch64.initrd \ -append 'earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait' \ -dtb /aarch64.dtb \ --device virtio-serial-device,id=virtio-serial0 \ +-device ioh3420,port=0x8,chassis=1,id=pci.1,bus=pcie.0,addr=0x1 \ +-device ioh3420,port=0x10,chassis=2,id=pci.2,bus=pcie.0,addr=0x2 \ +-device ioh3420,port=0x18,chassis=3,id=pci.3,bus=pcie.0,addr=0x3 \ +-device ioh3420,port=0x20,chassis=4,id=pci.4,bus=pcie.0,addr=0x4 \ +-device ioh3420,port=0x28,chassis=5,id=pci.5,bus=pcie.0,addr=0x5 \ +-device ioh3420,port=0x30,chassis=6,id=pci.6,bus=pcie.0,addr=0x6 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.2,addr=0x0 \ -drive file=/aarch64.raw,format=raw,if=none,id=drive-virtio-disk0 \ --device virtio-blk-device,drive=drive-virtio-disk0,id=virtio-disk0 \ --device virtio-net-device,vlan=0,id=net0,mac=52:54:00:09:a4:37 \ +-device virtio-blk-pci,bus=pci.3,addr=0x0,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-device virtio-net-pci,vlan=0,id=net0,mac=52:54:00:09:a4:37,bus=pci.1,addr=0x0 \ -net user,vlan=0,name=hostnet0 \ -serial pty \ -chardev pty,id=charconsole1 \ -device virtconsole,chardev=charconsole1,id=console1 \ --device virtio-balloon-device,id=balloon0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.4,addr=0x0 \ -object rng-random,id=objrng0,filename=/dev/random \ --device virtio-rng-device,rng=objrng0,id=rng0 +-device virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.5,addr=0x0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.xml b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.xml index ad34615..2a84a96 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.xml @@ -38,9 +38,6 @@ <target type='virtio' port='0'/> </console> <memballoon model='virtio'/> - <!-- - This actually doesn't work in practice because vexpress only has - 4 virtio slots available, rng makes 5 --> <rng model='virtio'> <backend model='random'>/dev/random</backend> </rng> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ab3ad08..301f83e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2135,6 +2135,7 @@ mymain(void) specified. */ DO_TEST("aarch64-virtio-pci-default", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, QEMU_CAPS_DEVICE_VIRTIO_MMIO, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM, QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE, diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml index 7c3fc19..a568f11 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml @@ -30,16 +30,46 @@ <disk type='file' device='disk'> <source file='/aarch64.raw'/> <target dev='vda' bus='virtio'/> - <address type='virtio-mmio'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> </disk> <controller type='pci' index='0' model='pcie-root'/> <controller type='virtio-serial' index='0'> - <address type='virtio-mmio'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='1' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='2' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x18'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='4' port='0x20'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='pci' index='5' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='5' port='0x28'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='pci' index='6' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='6' port='0x30'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </controller> <interface type='user'> <mac address='52:54:00:09:a4:37'/> <model type='virtio'/> - <address type='virtio-mmio'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </interface> <serial type='pty'> <target port='0'/> @@ -51,11 +81,11 @@ <target type='virtio' port='1'/> </console> <memballoon model='virtio'> - <address type='virtio-mmio'/> + <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> </memballoon> <rng model='virtio'> <backend model='random'>/dev/random</backend> - <address type='virtio-mmio'/> + <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> </rng> </devices> </domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index ddd17cb..bbd4687 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -961,6 +961,7 @@ mymain(void) QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM); DO_TEST("aarch64-virtio-pci-default", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, QEMU_CAPS_DEVICE_VIRTIO_MMIO, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM, QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE, -- 2.7.4
On 12/23/2016 01:54 PM, Andrea Bolognani wrote:
virtio-pci is the way forward for aarch64 guests: it's faster and less alien to people coming from other architectures. Now that guest support is finally getting there (Fedora 24, CentOS 7.3, Ubuntu 16.04 and Debian testing all support virtio-pci out of the box), we'd like to start using it by default instead of virtio-mmio.
Users and applications can already opt-in by explicitly using
<address type='pci'/>
inside the relevant elements, but that's kind of cumbersome and requires all users and management applications to adapt, which we'd really like to avoid.
What we can do instead is use virtio-mmio only if the guest already has at least one virtio-mmio device, and use virtio-pci in all other situations.
That means existing virtio-mmio guests will keep using the old addressing scheme, and new guests will automatically be created using virtio-pci instead. Users can still override the default in either direction.
Existing tests such as aarch64-aavmf-virtio-mmio and aarch64-virtio-pci-default already cover all possible scenarios, so no additions to the test suites are necessary. --- src/qemu/qemu_domain_address.c | 51 ++++++++++++++++++++-- ...l2argv-aarch64-virt-2.6-virtio-pci-default.args | 14 +++--- .../qemuxml2argv-aarch64-virtio-pci-default.args | 17 +++++--- .../qemuxml2argv-aarch64-virtio-pci-default.xml | 3 -- tests/qemuxml2argvtest.c | 1 + .../qemuxml2xmlout-aarch64-virtio-pci-default.xml | 40 ++++++++++++++--- tests/qemuxml2xmltest.c | 1 + 7 files changed, 106 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index be4ed23..d2f7953 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -398,6 +398,44 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def, }
+static int +qemuDomainHasVirtioMMIODevicesCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr info, + void *opaque) +{ + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) { + /* We can stop iterating as soon as we find the first + * virtio-mmio device */ + *((bool *) opaque) = true; + return -1; + } + + return 0; +} + + +/** + * qemuDomainHasVirtioMMIODevices: + * @def: domain definition + * + * Scan @def looking for devices with a virtio-mmio address. + * + * Returns: true if there are any, false otherwise + */ +static bool +qemuDomainHasVirtioMMIODevices(virDomainDefPtr def) +{ + bool result = false; + + virDomainDeviceInfoIterate(def, + qemuDomainHasVirtioMMIODevicesCallback, + &result); + + return result; +} + + static void qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) @@ -410,9 +448,16 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, qemuDomainMachineIsVirt(def))) return;
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO)) { - qemuDomainPrimeVirtioDeviceAddresses( - def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO); + /* We use virtio-mmio by default on mach-virt guests only if they already + * have at least one virtio-mmio device: in all other cases, we prefer + * virtio-pci */ + if (qemuDomainMachineHasPCIeRoot(def) && + !qemuDomainHasVirtioMMIODevices(def)) { + qemuDomainPrimeVirtioDeviceAddresses(def, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);
I'm having trouble remembering, and it's too early in the morning to go looking in the code - it seems like priming the addresses with PCI might not be necessary - as long as we simply *don't* prime them with virtio-mmio. I could be misremembering though. ACK either way. (ACK is based on the assumption that all concerned ARM parties have agreed that it's time to do this. I *think* that is the case :-)
+ } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO)) { + qemuDomainPrimeVirtioDeviceAddresses(def, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO); } }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args index 75db1a4..df03c6e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-2.6-virtio-pci-default.args @@ -21,14 +21,18 @@ QEMU_AUDIO_DRV=none \ -initrd /aarch64.initrd \ -append 'earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait' \ -dtb /aarch64.dtb \ --device virtio-serial-device,id=virtio-serial0 \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1 \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.2,addr=0x2 \ -drive file=/aarch64.raw,format=raw,if=none,id=drive-virtio-disk0 \ --device virtio-blk-device,drive=drive-virtio-disk0,id=virtio-disk0 \ --device virtio-net-device,vlan=0,id=net0,mac=52:54:00:09:a4:37 \ +-device virtio-blk-pci,bus=pci.2,addr=0x3,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-device virtio-net-pci,vlan=0,id=net0,mac=52:54:00:09:a4:37,bus=pci.2,addr=0x1 \ -net user,vlan=0,name=hostnet0 \ -serial pty \ -chardev pty,id=charconsole1 \ -device virtconsole,chardev=charconsole1,id=console1 \ --device virtio-balloon-device,id=balloon0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x4 \ -object rng-random,id=objrng0,filename=/dev/random \ --device virtio-rng-device,rng=objrng0,id=rng0 +-device virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.2,addr=0x5 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args index b5b010c..080b025 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args @@ -21,14 +21,21 @@ QEMU_AUDIO_DRV=none \ -initrd /aarch64.initrd \ -append 'earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait' \ -dtb /aarch64.dtb \ --device virtio-serial-device,id=virtio-serial0 \ +-device ioh3420,port=0x8,chassis=1,id=pci.1,bus=pcie.0,addr=0x1 \ +-device ioh3420,port=0x10,chassis=2,id=pci.2,bus=pcie.0,addr=0x2 \ +-device ioh3420,port=0x18,chassis=3,id=pci.3,bus=pcie.0,addr=0x3 \ +-device ioh3420,port=0x20,chassis=4,id=pci.4,bus=pcie.0,addr=0x4 \ +-device ioh3420,port=0x28,chassis=5,id=pci.5,bus=pcie.0,addr=0x5 \ +-device ioh3420,port=0x30,chassis=6,id=pci.6,bus=pcie.0,addr=0x6 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.2,addr=0x0 \ -drive file=/aarch64.raw,format=raw,if=none,id=drive-virtio-disk0 \ --device virtio-blk-device,drive=drive-virtio-disk0,id=virtio-disk0 \ --device virtio-net-device,vlan=0,id=net0,mac=52:54:00:09:a4:37 \ +-device virtio-blk-pci,bus=pci.3,addr=0x0,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-device virtio-net-pci,vlan=0,id=net0,mac=52:54:00:09:a4:37,bus=pci.1,addr=0x0 \ -net user,vlan=0,name=hostnet0 \ -serial pty \ -chardev pty,id=charconsole1 \ -device virtconsole,chardev=charconsole1,id=console1 \ --device virtio-balloon-device,id=balloon0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.4,addr=0x0 \ -object rng-random,id=objrng0,filename=/dev/random \ --device virtio-rng-device,rng=objrng0,id=rng0 +-device virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.5,addr=0x0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.xml b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.xml index ad34615..2a84a96 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.xml @@ -38,9 +38,6 @@ <target type='virtio' port='0'/> </console> <memballoon model='virtio'/> - <!-- - This actually doesn't work in practice because vexpress only has - 4 virtio slots available, rng makes 5 --> <rng model='virtio'> <backend model='random'>/dev/random</backend> </rng> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ab3ad08..301f83e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2135,6 +2135,7 @@ mymain(void) specified. */ DO_TEST("aarch64-virtio-pci-default", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, QEMU_CAPS_DEVICE_VIRTIO_MMIO, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM, QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE, diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml index 7c3fc19..a568f11 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml @@ -30,16 +30,46 @@ <disk type='file' device='disk'> <source file='/aarch64.raw'/> <target dev='vda' bus='virtio'/> - <address type='virtio-mmio'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> </disk> <controller type='pci' index='0' model='pcie-root'/> <controller type='virtio-serial' index='0'> - <address type='virtio-mmio'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='1' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='2' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x18'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='4' port='0x20'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='pci' index='5' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='5' port='0x28'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='pci' index='6' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='6' port='0x30'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </controller> <interface type='user'> <mac address='52:54:00:09:a4:37'/> <model type='virtio'/> - <address type='virtio-mmio'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </interface> <serial type='pty'> <target port='0'/> @@ -51,11 +81,11 @@ <target type='virtio' port='1'/> </console> <memballoon model='virtio'> - <address type='virtio-mmio'/> + <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> </memballoon> <rng model='virtio'> <backend model='random'>/dev/random</backend> - <address type='virtio-mmio'/> + <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> </rng> </devices> </domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index ddd17cb..bbd4687 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -961,6 +961,7 @@ mymain(void) QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM); DO_TEST("aarch64-virtio-pci-default", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, QEMU_CAPS_DEVICE_VIRTIO_MMIO, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM, QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE,
On Tue, 2017-01-10 at 03:38 -0500, Laine Stump wrote: [...]
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO)) { - qemuDomainPrimeVirtioDeviceAddresses( - def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO); + /* We use virtio-mmio by default on mach-virt guests only if they already + * have at least one virtio-mmio device: in all other cases, we prefer + * virtio-pci */ + if (qemuDomainMachineHasPCIeRoot(def) && + !qemuDomainHasVirtioMMIODevices(def)) { + qemuDomainPrimeVirtioDeviceAddresses(def, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI); I'm having trouble remembering, and it's too early in the morning to go looking in the code - it seems like priming the addresses with PCI might not be necessary - as long as we simply *don't* prime them with virtio-mmio. I could be misremembering though. ACK either way.
It looks like that's indeed the case from changing the code accordingly and running the test suite. However, a cursory look (it's morning here as well ;) at the code didn't give me enough confidence that it's working by design rather than pure chance, and with the freeze coming up shortly I decided to push the possibly slightly suboptimal code instead. I will go back to this and remove it if it turns out to be safe to do so after 3.0.0 is out.
(ACK is based on the assumption that all concerned ARM parties have agreed that it's time to do this. I *think* that is the case :-)
It's been agreed a while ago that the only thing stopping us from changing the default was poor guest support; as mentioned in the commit message, that's no longer a concern, so onwards to the future we go! :) Thanks for the review. -- Andrea Bolognani / Red Hat / Virtualization
--- docs/formatdomain.html.in | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f7bef51..01dd2b2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3255,7 +3255,13 @@ currently only available for some <code>armv7l</code> and <code>aarch64</code> virtual machines. virtio-mmio addresses do not have any additional attributes. - <span class="since">Since 1.1.3</span> + <span class="since">Since 1.1.3</span><br/> + If the guest architecture is <code>aarch64</code> and the machine + type is <code>virt</code>, libvirt will automatically assign PCI + addresses to devices; however, the presence of a single device + with virtio-mmio address in the guest configuration will cause + libvirt to assign virtio-mmio addresses to all further devices. + <span class="since">Since 3.0.0</span> </dd> <dt><code>isa</code></dt> <dd>ISA addresses have the following additional -- 2.7.4
Don't you want the subject to say "Document virtio-pci by default"? ACK. On 12/23/2016 01:54 PM, Andrea Bolognani wrote:
--- docs/formatdomain.html.in | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f7bef51..01dd2b2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3255,7 +3255,13 @@ currently only available for some <code>armv7l</code> and <code>aarch64</code> virtual machines. virtio-mmio addresses do not have any additional attributes. - <span class="since">Since 1.1.3</span> + <span class="since">Since 1.1.3</span><br/> + If the guest architecture is <code>aarch64</code> and the machine + type is <code>virt</code>, libvirt will automatically assign PCI + addresses to devices; however, the presence of a single device + with virtio-mmio address in the guest configuration will cause + libvirt to assign virtio-mmio addresses to all further devices. + <span class="since">Since 3.0.0</span> </dd> <dt><code>isa</code></dt> <dd>ISA addresses have the following additional
On Tue, 2017-01-10 at 03:38 -0500, Laine Stump wrote:
Don't you want the subject to say "Document virtio-pci by default"?
Of course I did :) Good catch! -- Andrea Bolognani / Red Hat / Virtualization
--- docs/news.html.in | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/news.html.in b/docs/news.html.in index 36c7d3d..e5b9513 100644 --- a/docs/news.html.in +++ b/docs/news.html.in @@ -53,6 +53,12 @@ Add a display of the <physical> size of a disk volume in the output of the volume XML </li> + <li>qemu: Use virtio-pci by default for aarch64 mach-virt guests<br/> + virtio-pci provides several advantages over virtio-mmio, such + as the ability to hotplug devices and improved performance. + While opting in to virtio-pci has been possible for a while, + newly-defined guests will now use it automatically + </li> </ul> </li> <li><strong>Bug fixes</strong> -- 2.7.4
On 12/23/2016 01:54 PM, Andrea Bolognani wrote:
--- docs/news.html.in | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/docs/news.html.in b/docs/news.html.in index 36c7d3d..e5b9513 100644 --- a/docs/news.html.in +++ b/docs/news.html.in @@ -53,6 +53,12 @@ Add a display of the <physical> size of a disk volume in the output of the volume XML </li> + <li>qemu: Use virtio-pci by default for aarch64 mach-virt guests<br/> + virtio-pci provides several advantages over virtio-mmio, such + as the ability to hotplug devices and improved performance. + While opting in to virtio-pci has been possible for a while, + newly-defined guests will now use it automatically + </li> </ul> </li> <li><strong>Bug fixes</strong>
ACK
participants (2)
-
Andrea Bolognani -
Laine Stump