On 06/23/2016 06:47 AM, Ján Tomko wrote:
<devices> <iommu model='intel-iommu'/> </devices>
results in:
-device intel-iommu
Might be nice to add a bit more "meat" to the commit message.
https://bugzilla.redhat.com/show_bug.cgi?id=1235580 --- src/qemu/qemu_command.c | 30 ++++++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-intel-iommu.args | 22 ++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 54 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args
One other thing that wasn't 100% from the qemu patches - is there a specific need to ensure the "-device intel-iommu" is placed on the command line prior to any PCI device being placed there as well? It's that same snippet I pulled from review of patch 1: "The device is part of the machine properties because we wanted to ensure is created before any other PCI device." But qemu patch 5/5 removes iommu from the machine command line, so I'm left wondering if there isn't some assumption that the -device option will need to be generated before any other PCI device. I only mention this because if that's the case, then I think we need to add some comments prior to calling qemuBuildIOMMUCommandLine to indicate that placement for the "-device intel-iommu" must be done before other PCI devices. I realize there's a general feeling against too many comments; however, it might avoid having some bug creep in because someone didn't know or follow a bz link or a qemu doc link from a commit message... Alternatively if the XML implementation becomes a hypervisor feature, then qemuBuildBootCommandLine could add the option. That also provides a way to have "either" 'iommu=on' or 'device intel-iommu' be generated with "-machine" or after machine is created to help hide or ensure future generations don't erroneously move things. John
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6944129..9f1d447 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6171,6 +6171,33 @@ qemuBuildBootCommandLine(virCommandPtr cmd,
static int +qemuBuildIOMMUCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + if (!def->iommu) + return 0; + + switch (def->iommu->model) { + case VIR_DOMAIN_IOMMU_MODEL_INTEL: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("IOMMU device: '%s' is not supported with " + "this QEMU binary"), + virDomainIOMMUModelTypeToString(def->iommu->model)); + return -1; + } + virCommandAddArgList(cmd, "-device", + virDomainIOMMUModelTypeToString(def->iommu->model), + NULL);
Obviously this would need adjustment if you changed the model...
+ case VIR_DOMAIN_IOMMU_MODEL_LAST: + break; + } + return 0; +} + + +static int qemuBuildGlobalControllerCommandLine(virCommandPtr cmd, const virDomainDef *def, virQEMUCapsPtr qemuCaps) @@ -9254,6 +9281,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildBootCommandLine(cmd, def, qemuCaps, &emitBootindex) < 0) goto error;
+ if (qemuBuildIOMMUCommandLine(cmd, def, qemuCaps) < 0) + goto error; + if (qemuBuildGlobalControllerCommandLine(cmd, def, qemuCaps) < 0) goto error;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args new file mode 100644 index 0000000..69e4490 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M q35 \ +-m 214 \ +-smp 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 intel-iommu \ +-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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a4b8bf4..59fa420 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2023,6 +2023,8 @@ mymain(void) QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_USB_HUB);
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);
qemuTestDriverFree(&driver);