On 03/23/2017 11:26 AM, Ján Tomko wrote: > Add a new attribute to control interrupt remapping. > > https://bugzilla.redhat.com/show_bug.cgi?id=1427005 > --- > docs/formatdomain.html.in | 22 ++++++++++++- > docs/schemas/domaincommon.rng | 9 +++++ > src/conf/domain_conf.c | 38 +++++++++++++++++++--- > src/conf/domain_conf.h | 1 + > .../qemuxml2argv-intel-iommu-irqchip.xml | 4 ++- > .../qemuxml2xmlout-intel-iommu-irqchip.xml | 4 ++- > 6 files changed, 71 insertions(+), 7 deletions(-) > awful name "intremap", but just following QEMU... > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 32a5f18..6b196d4 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -7285,7 +7285,9 @@ qemu-kvm -net nic,model=? /dev/null > <pre> > ... > <devices> > - <iommu model='intel'/> > + <iommu model='intel'> > + <driver intremap='on'/> > + </iommu> > </devices> > ... > </pre> > @@ -7296,6 +7298,24 @@ qemu-kvm -net nic,model=? /dev/null > Currently only the <code>intel</code> model is supported. > </p> > </dd> > + <dt><code>driver</code></dt> > + <dd> > + <p> > + The <code>driver</code> subelement can be used to configure > + additional options: > + </p> > + <dl> > + <dt><code>intremap</code></dt> > + <dd> > + <p> > + The <code>intremap</code> attribute with possible values > + <code>on</code> and <code>off</code> can be used to > + turn on interrupt remapping. <span class="since">Since 3.3.0</span> > + (QEMU only) > + </p> > + </dd> > + </dl> > + </dd> If I read patch1 correctly, support of this feature is dependent upon the 'irqchip' feature being set to (at least, it seems) "split". Whether "on" would be valid or not is not clear. In any case, since there is a dependency, it should be listed here and a doc link/reference to the irqchip feature description? Since patch7 only adds support for this when there's a "-device intel-iommu" and not when there's ",iommu=on", there's also another dependency that needs to be described. > </dl> > > <h3><a name="seclabel">Security label</a></h3> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 8ba3902..68f3680 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -3889,6 +3889,15 @@ > <attribute name="model"> > <value>intel</value> > </attribute> > + <optional> > + <element name="driver"> > + <optional> > + <attribute name="intremap"> > + <ref name="virOnOff"/> > + </attribute> > + </optional> > + </element> > + </optional> > </element> > </define> > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 1245fdd..da0c9f0 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -13909,12 +13909,16 @@ virDomainMemoryDefParseXML(xmlNodePtr memdevNode, > > > static virDomainIOMMUDefPtr > -virDomainIOMMUDefParseXML(xmlNodePtr node) > +virDomainIOMMUDefParseXML(xmlNodePtr node, > + xmlXPathContextPtr ctxt) > { > virDomainIOMMUDefPtr iommu = NULL, ret = NULL; > + xmlNodePtr save = ctxt->node; > char *tmp = NULL; > int val; > > + ctxt->node = node; > + > if (VIR_ALLOC(iommu) < 0) > goto cleanup; > > @@ -13931,10 +13935,20 @@ virDomainIOMMUDefParseXML(xmlNodePtr node) > > iommu->model = val; > > + VIR_FREE(tmp); > + if ((tmp = virXPathString("string(./driver/@intremap)", ctxt))) { > + if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { > + virReportError(VIR_ERR_XML_ERROR, _("unknown intremap value: %s"), tmp); > + goto cleanup; > + } > + iommu->intremap = val; > + } > + > ret = iommu; > iommu = NULL; > > cleanup: > + ctxt->node = save; > VIR_FREE(iommu); > VIR_FREE(tmp); > return ret; > @@ -14087,7 +14101,7 @@ virDomainDeviceDefParse(const char *xmlStr, > goto error; > break; > case VIR_DOMAIN_DEVICE_IOMMU: > - if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node))) > + if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node, ctxt))) > goto error; > break; > case VIR_DOMAIN_DEVICE_NONE: > @@ -18202,7 +18216,7 @@ virDomainDefParseXML(xmlDocPtr xml, > } > > if (n > 0) { > - if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0]))) > + if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0], ctxt))) > goto error; > } > VIR_FREE(nodes); > @@ -23846,8 +23860,24 @@ static void > virDomainIOMMUDefFormat(virBufferPtr buf, > virDomainIOMMUDefPtr iommu) > { > - virBufferAsprintf(buf, "<iommu model='%s'/>\n", > + virBuffer childBuf = VIR_BUFFER_INITIALIZER; > + > + virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2); > + > + if (iommu->intremap) { Since intremap is not a boolean or a pointer and while it may seem superfluous ... I'd like to see != VIR_TRISTATE_SWITCH_ABSENT to make it more obvious of the comparison. > + virBufferAsprintf(&childBuf, "<driver intremap='%s'/>\n", > + virTristateSwitchTypeToString(iommu->intremap)); > + } > + > + virBufferAsprintf(buf, "<iommu model='%s'", > virDomainIOMMUModelTypeToString(iommu->model)); > + if (virBufferUse(&childBuf)) { > + virBufferAddLit(buf, ">\n"); > + virBufferAddBuffer(buf, &childBuf); > + virBufferAddLit(buf, "</iommu>\n"); > + } else { > + virBufferAddLit(buf, "/>\n"); > + } > } > > > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 762e64e..ad8ae2d 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -2203,6 +2203,7 @@ typedef enum { > > struct _virDomainIOMMUDef { > virDomainIOMMUModel model; > + virTristateSwitch intremap; > }; > /* > * Guest VM main configuration > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml > index 98e4bba..3cef53d 100644 > --- a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml > +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml > @@ -24,6 +24,8 @@ > <input type='mouse' bus='ps2'/> > <input type='keyboard' bus='ps2'/> > <memballoon model='none'/> > - <iommu model='intel'/> > + <iommu model='intel'> > + <driver intremap='on'/> > + </iommu> > </devices> > </domain> > diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml > index 98e4bba..3cef53d 100644 > --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml > +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml > @@ -24,6 +24,8 @@ > <input type='mouse' bus='ps2'/> > <input type='keyboard' bus='ps2'/> > <memballoon model='none'/> > - <iommu model='intel'/> > + <iommu model='intel'> > + <driver intremap='on'/> > + </iommu> > </devices> > </domain> > FWIW: could end up with a merge conflict if you use the link from outdata to argvdata, but at least you don't change in two places. ACK w/ updated docs, John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
