On Wed, Apr 26, 2017 at 10:29:02AM +0200, Ján Tomko wrote: Hi, Ján, Thanks you for cced me. One tiny comment on irqchip mode... Here could user specify irqchip=off from libvirt side? IIUC that might be dangerous since userspace APIC should only be for debugging purpose (when kernel-irqchip=off, we'll be using userspace APIC, not kernel one, and iirc Paolo mentioned known bugs in userspace APIC). So imho we'd better not allow user to use "off", but only "on" and "split". CCing Paolo here in case he has any comment on this. Thanks, -- Peter Xu

On 04/26/2017 04:29 AM, Ján Tomko wrote: You asked in your response to my comments from the v2 posting "What would you say?" Well, that's what I was expecting someone to tell me. If I'm reading the documentation and see this, I might wonder whether I need this and what it's used for. What does on, split, and off actually do. Reading literally just what you have here makes it seem more related to IOMMU. Hence why I asked. I'm certainly "hardware terminology wording challenged" - glad that Peter/Paolo provided some insights. After thinking about it a bit, I think no matter what we end up calling it as long as we can inform the reader/consumer of our documentation that this relates to the QEMU 'kernel_irqchip' setting which is an architecture and machine model specific setting to determine where to handle interrupts. (that's about as generic as comes to mind). That should give enough context to have the curious user to search on "kernel_irqchip"... I understand there's concern about being too specific, but since you're indicating this is a QEMU only feature - I think some QEMU specifics could be added. What's still not clear to me is how this setting affects IOMMU. I'm "assuming" that "up to this point" there isn't an impact. But I do see code that specifically checks if interrupt remapping is enabled, then irqchip must be off or split. Something that could be added with the docs when patch 5/6 are added. Hopefully this helps. I'm fine with the code. If you change the name of the feature to attempt to be more generic - that's fine. John

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> ... &lt;devices&gt; - &lt;iommu model='intel'/&gt; + &lt;iommu model='intel'&gt; + &lt;driver intremap='on'/&gt; + &lt;/iommu&gt; &lt;/devices&gt; ... </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) + </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> -- 2.10.2

Add the intremap= option to QEMU command line, corresponding to the <driver intremap> attribute of the iommu device. https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- src/qemu/qemu_capabilities.c | 8 ++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 18 ++++++++ .../qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 22 +++++++--- .../qemucapabilitiesdata/caps_2.5.0.x86_64.replies | 24 +++++++---- .../qemucapabilitiesdata/caps_2.6.0.x86_64.replies | 24 +++++++---- .../qemucapabilitiesdata/caps_2.7.0.x86_64.replies | 28 +++++++++---- tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.8.0.x86_64.replies | 37 ++++++++++++---- tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.9.0.x86_64.replies | 49 ++++++++++++++++++---- tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-intel-iommu-irqchip.args | 2 +- tests/qemuxml2argvtest.c | 1 + 14 files changed, 173 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 72870b9..0c7d7b1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -365,6 +365,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "block-write-threshold", "query-named-block-nodes", "kernel-irqchip", + "intel-iommu-intremap", ); @@ -1715,6 +1716,10 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsUSBNECXHCI[] = { { "p3", QEMU_CAPS_NEC_USB_XHCI_PORTS }, }; +static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIntelIOMMU[] = { + { "intremap", QEMU_CAPS_INTEL_IOMMU_INTREMAP }, +}; + /* see documentation for virQEMUCapsQMPSchemaGetByPath for the query format */ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "blockdev-add/arg-type/options/+gluster/debug-level", QEMU_CAPS_GLUSTER_DEBUG_LEVEL}, @@ -1822,6 +1827,9 @@ static struct virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { { "nec-usb-xhci", virQEMUCapsObjectPropsUSBNECXHCI, ARRAY_CARDINALITY(virQEMUCapsObjectPropsUSBNECXHCI), -1 }, + { "intel-iommu", virQEMUCapsObjectPropsIntelIOMMU, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsIntelIOMMU), + QEMU_CAPS_DEVICE_INTEL_IOMMU}, }; struct virQEMUCapsPropTypeObjects { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 362347e..4d19691 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -402,6 +402,7 @@ typedef enum { QEMU_CAPS_BLOCK_WRITE_THRESHOLD, /* BLOCK_WRITE_THRESHOLD event */ QEMU_CAPS_QUERY_NAMED_BLOCK_NODES, /* qmp query-named-block-nodes */ QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, /* -machine kernel_irqchip */ + QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c04bf83..07c4aeb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6681,6 +6681,20 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, if (!iommu) return 0; + switch (iommu->model) { + case VIR_DOMAIN_IOMMU_MODEL_INTEL: + if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_INTREMAP)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu: interrupt remapping is not supported " + "with this QEMU binary")); + return -1; + } + break; + case VIR_DOMAIN_IOMMU_MODEL_LAST: + break; + } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU)) return 0; /* Already handled via -machine */ @@ -6701,6 +6715,10 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, return -1; } virBufferAddLit(&opts, "intel-iommu"); + if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&opts, ",intremap=%s", + virTristateSwitchTypeToString(iommu->intremap)); + } case VIR_DOMAIN_IOMMU_MODEL_LAST: break; } diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies index 6822181..9f256c4 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies @@ -3123,6 +3123,16 @@ { "return": [ { + "name": "version", + "type": "uint32" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "name": "pc-i440fx-2.4", "is-default": true, "cpu-max": 255, @@ -3246,7 +3256,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -3336,21 +3346,21 @@ "name": "qemu64" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "passthrough" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -4358,7 +4368,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -4388,7 +4398,7 @@ "capability": "events" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies index 2eeed7d..876bc2f 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies @@ -3286,6 +3286,16 @@ { "return": [ { + "name": "version", + "type": "uint32" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "name": "pc-i440fx-2.4", "cpu-max": 255 }, @@ -3417,7 +3427,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -3507,21 +3517,21 @@ "name": "qemu64" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "passthrough" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -4566,7 +4576,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -4600,7 +4610,7 @@ "capability": "x-postcopy-ram" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -12145,7 +12155,7 @@ "meta-type": "array" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies index 0c285cd..d6d0b57 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies @@ -3368,6 +3368,16 @@ { "return": [ { + "name": "version", + "type": "uint32" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "name": "pc-0.12", "cpu-max": 255 }, @@ -3475,7 +3485,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -3565,21 +3575,21 @@ "name": "qemu64" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "passthrough" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -4673,7 +4683,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -4707,7 +4717,7 @@ "capability": "postcopy-ram" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -12712,7 +12722,7 @@ "meta-type": "array" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies index 4a87237..671a958 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies @@ -3559,6 +3559,20 @@ { "return": [ { + "name": "version", + "type": "uint32" + }, + { + "name": "intremap", + "type": "bool" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "hotpluggable-cpus": true, "name": "pc-0.12", "cpu-max": 255 @@ -3702,7 +3716,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -3795,21 +3809,21 @@ "name": "qemu64" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "passthrough" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -4907,7 +4921,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -4941,7 +4955,7 @@ "capability": "postcopy-ram" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -13297,7 +13311,7 @@ "meta-type": "object" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index bee3ce0..ab0f8d2 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -203,6 +203,7 @@ <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> <flag name='kernel-irqchip'/> + <flag name='intel-iommu-intremap'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies index b3ad912..3087d76 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies @@ -3692,6 +3692,29 @@ { "return": [ { + "name": "eim", + "description": "on/off/auto", + "type": "OnOffAuto" + }, + { + "name": "x-buggy-eim", + "type": "bool" + }, + { + "name": "intremap", + "type": "bool" + }, + { + "name": "version", + "type": "uint32" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "hotpluggable-cpus": true, "name": "pc-0.12", "cpu-max": 255 @@ -3855,7 +3878,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -4061,21 +4084,21 @@ "static": false } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "passthrough" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -5198,7 +5221,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -5236,7 +5259,7 @@ "capability": "x-colo" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -14006,7 +14029,7 @@ "meta-type": "object" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml index 4e00816..85cae14 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml @@ -204,6 +204,7 @@ <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> <flag name='kernel-irqchip'/> + <flag name='intel-iommu-intremap'/> <version>2008000</version> <kvmVersion>0</kvmVersion> <package> (v2.8.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies index bae2475..3cfefb7 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies @@ -4003,6 +4003,37 @@ { "return": [ { + "name": "eim", + "description": "on/off/auto", + "type": "OnOffAuto" + }, + { + "name": "caching-mode", + "type": "bool" + }, + { + "name": "x-buggy-eim", + "type": "bool" + }, + { + "name": "intremap", + "type": "bool" + }, + { + "name": "version", + "type": "uint32" + }, + { + "name": "device-iotlb", + "type": "bool" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "hotpluggable-cpus": true, "name": "pc-0.12", "cpu-max": 255 @@ -4176,7 +4207,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -4458,21 +4489,21 @@ "migration-safe": true } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "passthrough" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -5721,7 +5752,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -5763,7 +5794,7 @@ "capability": "release-ram" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -14888,7 +14919,7 @@ "meta-type": "object" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { @@ -15067,7 +15098,7 @@ } } }, - "id": "libvirt-48" + "id": "libvirt-49" } { @@ -15308,7 +15339,7 @@ } } }, - "id": "libvirt-49" + "id": "libvirt-50" } { diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 1ca4df9..799c52a 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -208,6 +208,7 @@ <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> <flag name='kernel-irqchip'/> + <flag name='intel-iommu-intremap'/> <version>2008090</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0-rc0-142-g940a8ce)</package> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args index 899d240..6c1e96a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args @@ -16,4 +16,4 @@ QEMU_AUDIO_DRV=none \ -monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ -no-acpi \ -boot c \ --device intel-iommu +-device intel-iommu,intremap=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 80f10cc..29cd8b5 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2495,6 +2495,7 @@ mymain(void) DO_TEST("intel-iommu-irqchip", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, + QEMU_CAPS_INTEL_IOMMU_INTREMAP, QEMU_CAPS_DEVICE_INTEL_IOMMU); DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS); -- 2.10.2

On Wed, Apr 26, 2017 at 10:29:06AM +0200, Ján Tomko wrote: I don't know the detailed changes below, but... I feel like using the full term "caching-mode" might be slightly better, since "caching" is a too general name used everywhere, and that might mislead people. Yes I think even "caching-mode" is misleading, but it just seems better, and that's exactly the name that Intel defined in the spec. Anyway, please feel free to ignore this comment though! (In all cases, I blame Intel for the naming :-) Thanks, -- Peter Xu

Format the caching-mode option for the intel-iommu device, based on its <driver caching> attribute value. https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 3 +++ src/qemu/qemu_command.c | 11 ++++++++++ tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-intel-iommu-caching.args | 25 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 11 ++++++++++ 6 files changed, 54 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0c7d7b1..0e9afd2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -366,6 +366,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-named-block-nodes", "kernel-irqchip", "intel-iommu-intremap", + + "intel-iommu-caching", /* 255 */ ); @@ -1718,6 +1720,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsUSBNECXHCI[] = { static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIntelIOMMU[] = { { "intremap", QEMU_CAPS_INTEL_IOMMU_INTREMAP }, + { "caching-mode", QEMU_CAPS_INTEL_IOMMU_CACHING }, }; /* see documentation for virQEMUCapsQMPSchemaGetByPath for the query format */ diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4d19691..a3ff7e6 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -404,6 +404,9 @@ typedef enum { QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, /* -machine kernel_irqchip */ QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */ + /* 255 */ + QEMU_CAPS_INTEL_IOMMU_CACHING, /* intel-iommu.caching-mode */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 07c4aeb..566bd87 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6690,6 +6690,13 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, "with this QEMU binary")); return -1; } + if (iommu->caching != VIR_TRISTATE_SWITCH_ABSENT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_CACHING)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu: caching mode is not supported " + "with this QEMU binary")); + return -1; + } break; case VIR_DOMAIN_IOMMU_MODEL_LAST: break; @@ -6719,6 +6726,10 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, virBufferAsprintf(&opts, ",intremap=%s", virTristateSwitchTypeToString(iommu->intremap)); } + if (iommu->caching != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&opts, ",caching-mode=%s", + virTristateSwitchTypeToString(iommu->caching)); + } case VIR_DOMAIN_IOMMU_MODEL_LAST: break; } diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 799c52a..0193492 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -209,6 +209,7 @@ <flag name='query-named-block-nodes'/> <flag name='kernel-irqchip'/> <flag name='intel-iommu-intremap'/> + <flag name='intel-iommu-caching'/> <version>2008090</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0-rc0-142-g940a8ce)</package> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args new file mode 100644 index 0000000..1bec6d0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-machine q35,accel=tcg \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=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,intremap=on,caching-mode=on \ +-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 ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ +-device ich9-usb-ehci1,id=usb,bus=pci.2,addr=0x2.0x7 \ +-device rtl8139,vlan=0,id=net0,mac=52:54:00:ab:0c:5c,bus=pci.2,addr=0x1 \ +-net user,vlan=0,name=hostnet0 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 29cd8b5..aab20c4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2497,6 +2497,17 @@ mymain(void) QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, QEMU_CAPS_INTEL_IOMMU_INTREMAP, QEMU_CAPS_DEVICE_INTEL_IOMMU); + DO_TEST("intel-iommu-caching", + QEMU_CAPS_MACHINE_OPT, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_DEVICE_INTEL_IOMMU, + QEMU_CAPS_INTEL_IOMMU_INTREMAP, + QEMU_CAPS_INTEL_IOMMU_CACHING); DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS); -- 2.10.2