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); > > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list