[libvirt] [PATCHv2 3/6] conf: add <driver intremap> to <iommu>
John Ferlan
jferlan at redhat.com
Mon Apr 24 21:42:07 UTC 2017
On 04/20/2017 08:19 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 ++-
> 5 files changed, 68 insertions(+), 6 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index abf089a..f5a8e76 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -7335,7 +7335,9 @@ qemu-kvm -net nic,model=? /dev/null
> <pre>
> ...
> <devices>
> - <iommu model='intel'/>
> + <iommu model='intel'>
> + <driver intremap='on'/>
> + </iommu>
> </devices>
> ...
> </pre>
> @@ -7346,6 +7348,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)
It seems there is a relationship between this parameter and irqchip
mode? IOW: Is it a requirement that irqchip be "split" or "on"? It's
difficult to ascertain from the bz, but if so, then I'd expect a domain
conf post processing in this patch.
Reading the bz it seems though that this parameter is optional for
"certain" devices; however, for "general emulated devices" it's what is
used to enable VT-d protection, but that could also be read as if
'intel-iommu' is enabled, then VT-d protection is on by default with
'intremap' just having some other affect.
Is there something we could describe here at a very high level to
describe the usefulness/need for this parameter?
John
> + </p>
> + </dd>
> + </dl>
> + </dd>
> </dl>
>
> <h3><a name="seclabel">Security label</a></h3>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index df3143e..7930d85 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 931320e..d40d129 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -14057,12 +14057,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;
>
> @@ -14079,10 +14083,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;
> @@ -14235,7 +14249,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:
> @@ -18365,7 +18379,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);
> @@ -24019,8 +24033,24 @@ static void
> virDomainIOMMUDefFormat(virBufferPtr buf,
> const virDomainIOMMUDef *iommu)
> {
> - virBufferAsprintf(buf, "<iommu model='%s'/>\n",
> + virBuffer childBuf = VIR_BUFFER_INITIALIZER;
> +
> + virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
> +
> + if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) {
> + 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 97c4418..f95649c 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2211,6 +2211,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 cc895af..2100c08 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>
>
More information about the libvir-list
mailing list