[libvirt] [PATCHv3 3/6] conf: add <driver intremap> to <iommu>
John Ferlan
jferlan at redhat.com
Fri Apr 28 17:34:29 UTC 2017
On 04/26/2017 04:29 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 4b92a0f..e1a4625 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -7362,7 +7362,9 @@ qemu-kvm -net nic,model=? /dev/null
> <pre>
> ...
> <devices>
> - <iommu model='intel'/>
> + <iommu model='intel'>
> + <driver intremap='on'/>
> + </iommu>
> </devices>
> ...
> </pre>
> @@ -7373,6 +7375,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)
D'oh - brain cramp obviously w/r/t thinking in my response to patch 1
that patch 5/6 enabled intremap.
In any case, this is where I'd expect the "clarification" in the
kernel_irqchip explanation to indicate that when using using intremap
that kernel_irqchip must be "off" or "split".
When I reviewed v2 it really wasn't clear whether the irqchip setting
*had* to be enabled or not. I'm fine with letting the qemu startup fail
if the user misconfigures using "intremap=on" and "irqchip=on", although
it's also possible to make that check in post processing. It's about
where it's desired to have the error come from. Don't allow the
configuration or allow it and know that it'll fail.
I disagree with your assertion that describing what this parameter does
at a very high level is out of scope. As you point out it's related to
the VT-d technology - something that could be listed/described in some
way here. It doesn't need to be detailed, just enough to allow someone
to know what the setting is related to. In this case, VT-D
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 7772a91..cc10209 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3892,6 +3892,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 73fa268..650d206 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -14136,12 +14136,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;
>
> @@ -14158,10 +14162,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;
> @@ -14314,7 +14328,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:
> @@ -18444,7 +18458,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);
> @@ -24100,8 +24114,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 61af012..61f18cf 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2213,6 +2213,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