[libvirt] [PATCH v2] qemu: fix command line building for iommu devices
The intel-iommu device has existed since QEMU 2.2.0, but it was only possible to create it with -device since QEMU 2.7.0, thanks to: commit 621d983a1f9051f4cfc3f402569b46b77d8449fc Author: Marcel Apfelbaum <marcel@redhat.com> Date: Mon Jun 27 18:38:34 2016 +0300 hw/iommu: enable iommu with -device Use the standard '-device intel-iommu' to create the IOMMU device. The legacy '-machine,iommu=on' can still be used. The libvirt capability check & command line formatting code is thus broken for all QEMU versions 2.2.0 -> 2.6.0 inclusive. This fixes it to use iommu=on instead. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 13 ++++++++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_command.c | 22 +++++++++++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 2 +- .../qemuxml2argv-intel-iommu-machine.args | 21 ++++++++++++ .../qemuxml2argv-intel-iommu-machine.xml | 37 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 10 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.xml diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2ca7803..9020247 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -342,6 +342,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "smm", "virtio-pci-disable-legacy", "query-hotpluggable-cpus", + + "machine-iommu", /* 235 */ ); @@ -3633,6 +3635,17 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (virQEMUCapsProbeQMPMigrationCapabilities(qemuCaps, mon) < 0) goto cleanup; + /* 'intel-iommu' shows up as a device since 2.2.0, but can + * not be used with -device until 2.7.0. Before that it + * requires -machine iommu=on. So we must clear the device + * capability we detected on older QEMUs + */ + if (qemuCaps->version < 2007000 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU)) { + virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU); + } + /* GIC capabilities, eg. available GIC versions */ if ((qemuCaps->arch == VIR_ARCH_AARCH64 || qemuCaps->arch == VIR_ARCH_ARMV7L) && diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index a74d39f..ce2b01a 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -376,6 +376,9 @@ typedef enum { QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, /* virtio-*pci.disable-legacy */ QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS, /* qmp command query-hotpluggable-cpus */ + /* 235 */ + QEMU_CAPS_MACHINE_IOMMU, /* -machine iommu=on */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 982c33c..f2b579d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6349,6 +6349,9 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, if (!def->iommu) return 0; + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU)) + return 0; /* Already handled via -machine */ + switch (def->iommu->model) { case VIR_DOMAIN_IOMMU_MODEL_INTEL: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU)) { @@ -7067,6 +7070,25 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, } } + /* We don't report errors on missing cap here - -device code will do that */ + if (def->iommu && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU)) { + switch (def->iommu->model) { + case VIR_DOMAIN_IOMMU_MODEL_INTEL: + if (!qemuDomainMachineIsQ35(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("IOMMU device: '%s' is only supported with " + "Q35 machines"), + virDomainIOMMUModelTypeToString(def->iommu->model)); + return -1; + } + virBufferAddLit(&buf, ",iommu=on"); + break; + case VIR_DOMAIN_IOMMU_MODEL_LAST: + break; + } + } + virCommandAddArgBuffer(cmd, &buf); } diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml index db778ef..abb9c66 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -182,9 +182,9 @@ <flag name='spice-unix'/> <flag name='drive-detect-zeroes'/> <flag name='display'/> - <flag name='intel-iommu'/> <flag name='smm'/> <flag name='virtio-pci-disable-legacy'/> + <flag name='machine-iommu'/> <version>2004000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml index fc915ad..cbf2e1e 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -187,9 +187,9 @@ <flag name='drive-detect-zeroes'/> <flag name='tls-creds-x509'/> <flag name='display'/> - <flag name='intel-iommu'/> <flag name='smm'/> <flag name='virtio-pci-disable-legacy'/> + <flag name='machine-iommu'/> <version>2005000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index 60f1fcf..116c506 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -193,9 +193,9 @@ <flag name='drive-detect-zeroes'/> <flag name='tls-creds-x509'/> <flag name='display'/> - <flag name='intel-iommu'/> <flag name='smm'/> <flag name='virtio-pci-disable-legacy'/> + <flag name='machine-iommu'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index 7a54040..b8b79ab 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -190,10 +190,10 @@ <flag name='drive-detect-zeroes'/> <flag name='tls-creds-x509'/> <flag name='display'/> - <flag name='intel-iommu'/> <flag name='smm'/> <flag name='virtio-pci-disable-legacy'/> <flag name='query-hotpluggable-cpus'/> + <flag name='machine-iommu'/> <version>2006091</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0-rc1-52-g42e0d60)</package> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.args b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.args new file mode 100644 index 0000000..9221b79 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.args @@ -0,0 +1,21 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-machine q35,accel=tcg,iommu=on \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.xml new file mode 100644 index 0000000..b5b2b51 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.xml @@ -0,0 +1,37 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</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/bin/qemu</emulator> + <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='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </memballoon> + <iommu model='intel'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 39abe72..d5e21ac 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2105,6 +2105,8 @@ mymain(void) DO_TEST("acpi-table", NONE); DO_TEST("intel-iommu", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_INTEL_IOMMU); + DO_TEST("intel-iommu-machine", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_MACHINE_OPT, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_MACHINE_IOMMU); DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS); -- 2.7.4
On Mon, Oct 03, 2016 at 02:37:57PM +0100, Daniel P. Berrange wrote:
The intel-iommu device has existed since QEMU 2.2.0, but it was only possible to create it with -device since QEMU 2.7.0, thanks to:
commit 621d983a1f9051f4cfc3f402569b46b77d8449fc Author: Marcel Apfelbaum <marcel@redhat.com> Date: Mon Jun 27 18:38:34 2016 +0300
hw/iommu: enable iommu with -device
Use the standard '-device intel-iommu' to create the IOMMU device. The legacy '-machine,iommu=on' can still be used.
The libvirt capability check & command line formatting code is thus broken for all QEMU versions 2.2.0 -> 2.6.0 inclusive.
This fixes it to use iommu=on instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 13 ++++++++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_command.c | 22 +++++++++++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 2 +- .../qemuxml2argv-intel-iommu-machine.args | 21 ++++++++++++ .../qemuxml2argv-intel-iommu-machine.xml | 37 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 10 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.xml
This patch won't apply for me: Applying: qemu: fix command line building for iommu devices error: src/qemu/qemu_capabilities.c: does not match index error: patch failed: src/qemu/qemu_capabilities.h:376 error: src/qemu/qemu_capabilities.h: patch does not apply error: patch failed: tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml:190 error: tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml: patch does not apply Patch failed at 0001 qemu: fix command line building for iommu devices
@@ -3633,6 +3635,17 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (virQEMUCapsProbeQMPMigrationCapabilities(qemuCaps, mon) < 0) goto cleanup;
+ /* 'intel-iommu' shows up as a device since 2.2.0, but can + * not be used with -device until 2.7.0. Before that it + * requires -machine iommu=on. So we must clear the device + * capability we detected on older QEMUs + */ + if (qemuCaps->version < 2007000 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU)) { + virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU);
Works for upstream QEMU, but not for distros that have backported the fix. Sadly, with 2.7.0 "iommu" still shows in the "machine" section of "query-command-line-options", despite: commit 10d01f73e39100701028c7badd6ece52990cf758 machine: remove iommu property http://git.qemu.org/?p=qemu.git;a=commitdiff;h=10d01f73 But an attempt to use it fails: # qemu-git -machine q35,iommu=on qemu-git: Property '.iommu' not found So the version check looks like the only way :(
+ } + /* GIC capabilities, eg. available GIC versions */ if ((qemuCaps->arch == VIR_ARCH_AARCH64 || qemuCaps->arch == VIR_ARCH_ARMV7L) &&
diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index 7a54040..b8b79ab 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -190,10 +190,10 @@ <flag name='drive-detect-zeroes'/> <flag name='tls-creds-x509'/> <flag name='display'/> - <flag name='intel-iommu'/>
Strange. For 2.7.0 we should have kept the original capability, according to the code above. Jan
<flag name='smm'/> <flag name='virtio-pci-disable-legacy'/> <flag name='query-hotpluggable-cpus'/> + <flag name='machine-iommu'/> <version>2006091</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0-rc1-52-g42e0d60)</package>
On Mon, Oct 03, 2016 at 05:18:40PM +0200, Ján Tomko wrote:
On Mon, Oct 03, 2016 at 02:37:57PM +0100, Daniel P. Berrange wrote:
The intel-iommu device has existed since QEMU 2.2.0, but it was only possible to create it with -device since QEMU 2.7.0, thanks to:
commit 621d983a1f9051f4cfc3f402569b46b77d8449fc Author: Marcel Apfelbaum <marcel@redhat.com> Date: Mon Jun 27 18:38:34 2016 +0300
hw/iommu: enable iommu with -device
Use the standard '-device intel-iommu' to create the IOMMU device. The legacy '-machine,iommu=on' can still be used.
The libvirt capability check & command line formatting code is thus broken for all QEMU versions 2.2.0 -> 2.6.0 inclusive.
This fixes it to use iommu=on instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 13 ++++++++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_command.c | 22 +++++++++++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 2 +- tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 2 +- .../qemuxml2argv-intel-iommu-machine.args | 21 ++++++++++++ .../qemuxml2argv-intel-iommu-machine.xml | 37 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 10 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.xml
This patch won't apply for me: Applying: qemu: fix command line building for iommu devices error: src/qemu/qemu_capabilities.c: does not match index error: patch failed: src/qemu/qemu_capabilities.h:376 error: src/qemu/qemu_capabilities.h: patch does not apply error: patch failed: tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml:190 error: tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml: patch does not apply Patch failed at 0001 qemu: fix command line building for iommu devices
Opps, I based it on the wrong branch - libvirt 2.2.0 instead of master
@@ -3633,6 +3635,17 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (virQEMUCapsProbeQMPMigrationCapabilities(qemuCaps, mon) < 0) goto cleanup;
+ /* 'intel-iommu' shows up as a device since 2.2.0, but can + * not be used with -device until 2.7.0. Before that it + * requires -machine iommu=on. So we must clear the device + * capability we detected on older QEMUs + */ + if (qemuCaps->version < 2007000 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU)) { + virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU);
Works for upstream QEMU, but not for distros that have backported the fix.
That's only a problem for distros that backported the patch that *removes* the iommu attribute against -machine. If distros backport code that removes features, then its their responsibility to fix their downstream libvirt too.
diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index 7a54040..b8b79ab 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -190,10 +190,10 @@ <flag name='drive-detect-zeroes'/> <flag name='tls-creds-x509'/> <flag name='display'/> - <flag name='intel-iommu'/>
Strange. For 2.7.0 we should have kept the original capability, according to the code above.
Opps, yes, that's not right. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
participants (2)
-
Daniel P. Berrange -
Ján Tomko