On 06/23/2016 06:47 AM, Ján Tomko wrote: > A device with an attribude 'model'. Attribute > > intel-iommu is accepted so far: > > <devices> > ... > <iommu model='intel-iommu'/> > </devices> > The text reads like it already exists, but we're adding the XML here. > https://bugzilla.redhat.com/show_bug.cgi?id=1235580 > --- > docs/schemas/domaincommon.rng | 11 +++++++ > src/conf/domain_conf.c | 37 ++++++++++++++++++++++ > src/conf/domain_conf.h | 14 ++++++++ > src/libvirt_private.syms | 2 ++ > .../qemuxml2argvdata/qemuxml2argv-intel-iommu.xml | 37 ++++++++++++++++++++++ > .../qemuxml2xmlout-intel-iommu.xml | 37 ++++++++++++++++++++++ > tests/qemuxml2xmltest.c | 4 +++ > 7 files changed, 142 insertions(+) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.xml > create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.xml > FYI: There is a qemu v5 of the series now: https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07686.html w/r/t this patch: There's no description in formatdomain.html.in I think you need to add ABI checks too. Although the implementation is to add a "-device intel-iommu' to the command line, it would seem to me based purely on the former/current means to enable this via "-machine,iommu=on" and the comments from the qemu commit: "The device is part of the machine properties because we wanted to ensure is created before any other PCI device." is this then perhaps more of a hypervisor feature since IIUC in the long run it's the Intel VT-d and AMD IOV "support" that allow it to work. It seems though if it were to be used prior to qemu 2.7, then providing "machine,iommu=on" would be required. In any case, probably need to wait for QEMU fixes to be finally accepted as well... > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 162c2e0..c8a8ecb 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -3709,6 +3709,14 @@ > </optional> > </define> > > + <define name="iommu"> > + <element name="iommu"> > + <attribute name="model"> > + <value>intel-iommu</value> Isn't this redundant? <iommu model='intel-iommu'> Is the future command option going to be "-device amd-iommu" or "-device ppc-iommu" or "-device arm-iommu"? Seems we'd want to indicate "<iommu model='intel'/> and then generate the command using '%s'-iommu, using *TypeToString. > + </attribute> > + </element> > + </define> > + > <define name="input"> > <element name="input"> > <choice> > @@ -4198,6 +4206,9 @@ > <zeroOrMore> > <ref name="panic"/> > </zeroOrMore> > + <optional> > + <ref name="iommu"/> > + </optional> > </interleave> > </element> > </define> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 9443281..07889ff 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -802,6 +802,9 @@ VIR_ENUM_IMPL(virDomainTPMModel, VIR_DOMAIN_TPM_MODEL_LAST, > VIR_ENUM_IMPL(virDomainTPMBackend, VIR_DOMAIN_TPM_TYPE_LAST, > "passthrough") > > +VIR_ENUM_IMPL(virDomainIOMMUModel, VIR_DOMAIN_IOMMU_MODEL_LAST, > + "intel-iommu") > + > VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST, > "default", > "unmap", > @@ -2615,6 +2618,8 @@ void virDomainDefFree(virDomainDefPtr def) > virDomainPanicDefFree(def->panics[i]); > VIR_FREE(def->panics); > > + VIR_FREE(def->iommu); > + > VIR_FREE(def->idmap.uidmap); > VIR_FREE(def->idmap.gidmap); > > @@ -17239,6 +17244,33 @@ virDomainDefParseXML(xmlDocPtr xml, > } > VIR_FREE(nodes); > > + if ((n = virXPathNodeSet("./devices/iommu", ctxt, &nodes)) < 0) > + goto error; > + > + if (n > 1) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("only a single IOMMU device is supported")); > + goto error; > + } > + > + if (n > 0) { > + int val; > + if (VIR_ALLOC(def->iommu) < 0) > + goto error; > + > + if (!(tmp = virXMLPropString(nodes[0], "model"))) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("missing model for IOMMU device")); > + goto error; > + } > + if ((val = virDomainIOMMUModelTypeFromString(tmp)) < 0) { You won't need val, if you go with if ((def->iommu->model = ... IDC either way. > + virReportError(VIR_ERR_XML_ERROR, _("unknown IOMMU model: %s"), tmp); > + goto error; > + } > + def->iommu->model = val; > + } > + VIR_FREE(nodes); > + > /* analysis of the user namespace mapping */ > if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0) > goto error; > @@ -23561,6 +23593,11 @@ virDomainDefFormatInternal(virDomainDefPtr def, > goto error; > } > > + if (def->iommu) { > + virBufferAsprintf(buf, "<iommu model='%s'/>\n", > + virDomainIOMMUModelTypeToString(def->iommu->model)); > + } > + > virBufferAdjustIndent(buf, -2); > virBufferAddLit(buf, "</devices>\n"); > > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 6e81e52..918c185 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -150,6 +150,9 @@ typedef virDomainShmemDef *virDomainShmemDefPtr; > typedef struct _virDomainTPMDef virDomainTPMDef; > typedef virDomainTPMDef *virDomainTPMDefPtr; > > +typedef struct _virDomainIOMMUDef virDomainIOMMUDef; > +typedef virDomainIOMMUDef *virDomainIOMMUDefPtr; > + > /* Flags for the 'type' field in virDomainDeviceDef */ > typedef enum { > VIR_DOMAIN_DEVICE_NONE = 0, > @@ -2112,6 +2115,15 @@ struct _virDomainKeyWrapDef { > int dea; /* enum virTristateSwitch */ > }; > > +typedef enum { > + VIR_DOMAIN_IOMMU_MODEL_INTEL, > + > + VIR_DOMAIN_IOMMU_MODEL_LAST > +} virDomainIOMMUModel; > + > +struct _virDomainIOMMUDef { > + virDomainIOMMUModel model; > +}; > /* > * Guest VM main configuration > * > @@ -2249,6 +2261,7 @@ struct _virDomainDef { > virCPUDefPtr cpu; > virSysinfoDefPtr sysinfo; > virDomainRedirFilterDefPtr redirfilter; > + virDomainIOMMUDefPtr iommu; > > void *namespaceData; > virDomainXMLNamespace ns; > @@ -3006,6 +3019,7 @@ VIR_ENUM_DECL(virDomainTPMModel) > VIR_ENUM_DECL(virDomainTPMBackend) > VIR_ENUM_DECL(virDomainMemoryModel) > VIR_ENUM_DECL(virDomainMemoryBackingModel) > +VIR_ENUM_DECL(virDomainIOMMUModel) > /* from libvirt.h */ > VIR_ENUM_DECL(virDomainState) > VIR_ENUM_DECL(virDomainNostateReason) > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index 501c23e..73cecfd 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -343,6 +343,8 @@ virDomainHubTypeToString; > virDomainHypervTypeFromString; > virDomainHypervTypeToString; > virDomainInputDefFree; > +virDomainIOMMUModelTypeFromString; > +virDomainIOMMUModelTypeToString; > virDomainIOThreadIDAdd; > virDomainIOThreadIDDefFree; > virDomainIOThreadIDDel; > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.xml > new file mode 100644 > index 0000000..8d95383 > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.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-iommu'/> > + </devices> > +</domain> > diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.xml > new file mode 100644 Since I recently went through this with the secret/LUKS changes and the fact that the input/output files don't differ, if you create a soft link in the output directory to the argvdata directory for it's file instead of a whole new file, then the xml2xml test will use that. John > index 0000000..8d95383 > --- /dev/null > +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.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-iommu'/> > + </devices> > +</domain> > diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c > index 7db9cb7..855eb40 100644 > --- a/tests/qemuxml2xmltest.c > +++ b/tests/qemuxml2xmltest.c > @@ -829,6 +829,10 @@ mymain(void) > DO_TEST("video-qxl-heads"); > DO_TEST("video-qxl-noheads"); > > + DO_TEST_FULL("intel-iommu", WHEN_ACTIVE, GIC_NONE, > + QEMU_CAPS_DEVICE_PCI_BRIDGE, > + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE); > + > qemuTestDriverFree(&driver); > > return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list