[libvirt] [PATCH v3 0/3] Allow PCI virtio on ARM "virt" machine

Virt machine in qemu since v2.3.0 has PCI generic host controller, and can use PCI devices. This provides performance improvement as well as vhost-net with irqfd support for virtio-net. However libvirt currently does not allow ARM virt machine to have PCI devices. This patchset adds the necessary support. This version is completely reworked and uses different approach. Changes since v2: - Correctly model PCI Express bus on the machine. It is now possible to explicitly specify <address-type='pci'> with attributes. This allows to attach not only virtio, but any other PCI device to the model. - Default is not changed and still mmio, for backwards compatibility with existing installations. PCI bus has to be explicitly specified. - Check for the capability in correct place, in v2 it actually did not work Changes since v1: - Added capability based on qemu version number - Recognize also "virt-" prefix Pavel Fedin (3): Introduce QEMU_CAPS_ARM_VIRT_PCI Add PCI-Express root to ARM virt machine Build correct command line for PCI NICs on ARM src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 3 ++- src/qemu/qemu_domain.c | 12 ++++++++---- 4 files changed, 16 insertions(+), 5 deletions(-) -- 1.9.5.msysgit.0

This capability specifies that "virt" machine on ARM has PCI controller. Enabled when qemu version is at least 2.3.0. Signed-off-by: Pavel Fedin <p.fedin@samsung.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 6 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 27686c3..0dc034f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -287,6 +287,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "aarch64-off", "vhost-user-multiqueue", /* 190 */ + "arm-virt-pci", ); @@ -3352,6 +3353,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, qemuCaps->version >= 2003000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_AARCH64_OFF); + /* 'virt' machine has PCI controller from v2.3.0 onwards */ + if (qemuCaps->version >= 2003000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_ARM_VIRT_PCI); + /* vhost-user supports multi-queue from v2.4.0 onwards, * but there is no way to query for that capability */ if (qemuCaps->version >= 2004000) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 30aa504..f4180a8 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -230,6 +230,7 @@ typedef enum { QEMU_CAPS_DEVICE_PCI_SERIAL = 188, /* -device pci-serial */ QEMU_CAPS_CPU_AARCH64_OFF = 189, /* -cpu ...,aarch64=off */ QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */ + QEMU_CAPS_ARM_VIRT_PCI = 191, /* ARM 'virt' machine has PCI bus */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; -- 1.9.5.msysgit.0

On Mon, Jul 06, 2015 at 15:59:25 +0300, Pavel Fedin wrote:
This capability specifies that "virt" machine on ARM has PCI controller. Enabled when qemu version is at least 2.3.0.
Signed-off-by: Pavel Fedin <p.fedin@samsung.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 6 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 27686c3..0dc034f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c
...
@@ -3352,6 +3353,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, qemuCaps->version >= 2003000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_AARCH64_OFF);
+ /* 'virt' machine has PCI controller from v2.3.0 onwards */ + if (qemuCaps->version >= 2003000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_ARM_VIRT_PCI);
Is there a way how we could detect this according to the actual presence of the PCI bus for the ARM machine? A certain device type that can be queried rather than relying on a version check. (see virQEMUCapsObjectTypes[] and other similar arrays in src/qemu/qemu_caps.c) Generally the approach is to use a specific device check to set capabilities for anything where it's possible since it's robust against backports of given functionality to previous versions.
+ /* vhost-user supports multi-queue from v2.4.0 onwards, * but there is no way to query for that capability */ if (qemuCaps->version >= 2004000)
Peter

Hello!
Is there a way how we could detect this according to the actual presence of the PCI bus for the ARM machine? A certain device type that can be queried rather than relying on a version check.
I looked at the code you suggested. As far as i could understand, it actually parses an output of "qemu-system-blah -device ?" plus some extras. And this command lists all devices that this qemu version knows, and not only what can be attached to a particular machine. I browsed through the history of qemu patches. Actually, "gpex-pcihost" device was implemented together with adding it to 'virt' machine. So, i could query for this flag and count it as capability. But - it only says that qemu knows this device in principle. It does not say that this device is really a part of virt machine. Will it be acceptable ? Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia

Add PCI Express root complex if the corresponding capability is present Signed-off-by: Pavel Fedin <p.fedin@samsung.com> --- src/qemu/qemu_domain.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f9bf32c..36f411d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -981,7 +981,7 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = { static int qemuDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { bool addDefaultUSB = true; bool addImplicitSATA = false; @@ -1030,12 +1030,16 @@ qemuDomainDefPostParse(virDomainDefPtr def, break; case VIR_ARCH_ARMV7L: - addDefaultUSB = false; - addDefaultMemballoon = false; - break; case VIR_ARCH_AARCH64: addDefaultUSB = false; addDefaultMemballoon = false; + if (STREQ(def->os.machine, "virt") || + STRPREFIX(def->os.machine, "virt-")) { + virQEMUDriverPtr driver = opaque; + virQEMUCapsPtr qemuCaps = + virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator); + addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_ARM_VIRT_PCI); + } break; case VIR_ARCH_PPC64: -- 1.9.5.msysgit.0

On Mon, Jul 06, 2015 at 15:59:26 +0300, Pavel Fedin wrote:
Add PCI Express root complex if the corresponding capability is present
Signed-off-by: Pavel Fedin <p.fedin@samsung.com> --- src/qemu/qemu_domain.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f9bf32c..36f411d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c
...
@@ -1030,12 +1030,16 @@ qemuDomainDefPostParse(virDomainDefPtr def, break;
case VIR_ARCH_ARMV7L: - addDefaultUSB = false; - addDefaultMemballoon = false; - break; case VIR_ARCH_AARCH64: addDefaultUSB = false; addDefaultMemballoon = false; + if (STREQ(def->os.machine, "virt") || + STRPREFIX(def->os.machine, "virt-")) { + virQEMUDriverPtr driver = opaque; + virQEMUCapsPtr qemuCaps = + virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator); + addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_ARM_VIRT_PCI);
Is there a way where we would make this configurable? Since the "virt" machine types use the MMIO bus by default we should make the PCI bus configurable since not everybody might want to add it actually.
+ } break;
case VIR_ARCH_PPC64: -- 1.9.5.msysgit.0
Peter

Hello!
Is there a way where we would make this configurable? Since the "virt" machine types use the MMIO bus by default we should make the PCI bus configurable since not everybody might want to add it actually.
What exactly do you mean? In qemu this is not configurable, it's just there. If you don't want to use it, just don't use it. I see no harm done by adding description of it to domain XML. If you just add a device, and don't specify <address type='pci'>, it will default to <address type='virtio-mmio'> as before. Nothing will change regarding old behavior and backwards compatibility. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia

On Tue, Jul 07, 2015 at 10:51:45AM +0300, Pavel Fedin wrote:
Hello!
Is there a way where we would make this configurable? Since the "virt" machine types use the MMIO bus by default we should make the PCI bus configurable since not everybody might want to add it actually.
What exactly do you mean? In qemu this is not configurable, it's just there. If you don't want to use it, just don't use it. I see no harm done by adding description of it to domain XML. If you just add a device, and don't specify <address type='pci'>, it will default to <address type='virtio-mmio'> as before. Nothing will change regarding old behavior and backwards compatibility.
The domain XML should contain the devices that are there, not just those the user requested. Backwards compatibility could break if older libvirt refused to start the domain with a pcie root. From looking at the code, that does not seem to be the case. Jan

Legacy -net option works correctly only with embedded device models, which do not require any bus specification. Therefore, we should use -device for PCI hardware Signed-off-by: Pavel Fedin <p.fedin@samsung.com> --- src/qemu/qemu_command.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 25a7bc6..983fcdb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -457,7 +457,8 @@ qemuDomainSupportsNicdev(virDomainDefPtr def, /* non-virtio ARM nics require legacy -net nic */ if (((def->os.arch == VIR_ARCH_ARMV7L) || (def->os.arch == VIR_ARCH_AARCH64)) && - net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) + net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && + net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) return false; return true; -- 1.9.5.msysgit.0
participants (3)
-
Ján Tomko
-
Pavel Fedin
-
Peter Krempa