[libvirt] [PATCHv2 0/6] Support more options for intel-iommu

For https://bugzilla.redhat.com/show_bug.cgi?id=1427005 v2: * use != VIR_TRISTATE_SWITCH_ABSENT more often * use symlinks for identical XMLs * move capability checks in qemuBuildIOMMUCommandLine to the beginning, to be shared with qemuBuildIOMMUCommandLine Documentation changes reflecting current QEMU implementation limits are not included. Ján Tomko (6): conf: add <irqchip mode> to <features> qemu: format kernel_irqchip on the command line conf: add <driver intremap> to <iommu> qemu: format intremap= on intel-iommu command line conf: add caching attribute to iommu device qemu: format caching-mode on iommu command line docs/formatdomain.html.in | 41 +++++++++- docs/schemas/domaincommon.rng | 30 ++++++++ src/conf/domain_conf.c | 90 ++++++++++++++++++++-- src/conf/domain_conf.h | 14 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 12 +++ src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 40 ++++++++++ tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 22 ++++-- tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.5.0.x86_64.replies | 24 ++++-- tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + .../qemucapabilitiesdata/caps_2.6.0.x86_64.replies | 24 ++++-- tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_2.7.0.x86_64.replies | 28 +++++-- tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 2 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_2.8.0.x86_64.replies | 37 +++++++-- tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 2 + .../qemucapabilitiesdata/caps_2.9.0.x86_64.replies | 49 +++++++++--- tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 3 + .../qemuxml2argv-intel-iommu-caching.args | 19 +++++ .../qemuxml2argv-intel-iommu-caching.xml | 28 +++++++ .../qemuxml2argv-intel-iommu-irqchip.args | 19 +++++ .../qemuxml2argv-intel-iommu-irqchip.xml | 31 ++++++++ tests/qemuxml2argvtest.c | 10 +++ .../qemuxml2xmlout-intel-iommu-caching.xml | 1 + .../qemuxml2xmlout-intel-iommu-irqchip.xml | 1 + tests/qemuxml2xmltest.c | 2 + 37 files changed, 496 insertions(+), 49 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml -- 2.10.2

Add a new <irqchip> element with a mode attribute. Possible values are off, split or on. https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 10 +++++++ docs/schemas/domaincommon.rng | 16 ++++++++++ src/conf/domain_conf.c | 34 +++++++++++++++++++++- src/conf/domain_conf.h | 12 ++++++++ .../qemuxml2argv-intel-iommu-irqchip.xml | 29 ++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-irqchip.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b1e38f0..abf089a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1643,6 +1643,7 @@ </kvm> <pvspinlock state='on'/> <gic version='2'/> + <irqchip mode='split'/> </features> ...</pre> @@ -1804,6 +1805,15 @@ for hypervisor to decide. <span class="since">Since 2.1.0</span> </dd> + <dt><code>irqchip</code></dt> + <dd>Tune the in-kernel irqchip. Possible values for the + <code>mode</code> attribute are: + <code>on</code>, <code>split</code> and <code>off</code>. + <code>split</code> is useful for using interrupt remapping + with the <a href="#elementsIommu">IOMMU device</a>. + The default is left for hypervisor to decide. + <span class="since">Since 3.3.0</span> (QEMU only) + </dd> </dl> <h3><a name="elementsTime">Time keeping</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index edc225f..df3143e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4511,6 +4511,9 @@ </optional> </element> </optional> + <optional> + <ref name="irqchip"/> + </optional> </interleave> </element> </optional> @@ -4686,6 +4689,19 @@ </element> </define> + <define name="irqchip"> + <element name="irqchip"> + <attribute name="mode"> + <choice> + <value>off</value> + <value>split</value> + <value>on</value> + </choice> + </attribute> + <empty/> + </element> + </define> + <define name="address"> <element name="address"> <choice> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 705deb3..931320e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -140,7 +140,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "pmu", "vmport", "gic", - "smm") + "smm", + "irqchip") VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST, "default", @@ -857,6 +858,12 @@ VIR_ENUM_IMPL(virDomainLoader, "rom", "pflash") +VIR_ENUM_IMPL(virDomainIRQChip, + VIR_DOMAIN_IRQCHIP_LAST, + "off", + "split", + "on") + /* Internal mapping: subset of block job types that can be present in * <mirror> XML (remaining types are not two-phase). */ VIR_ENUM_DECL(virDomainBlockJob) @@ -17442,6 +17449,24 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt->node = node; break; + case VIR_DOMAIN_FEATURE_IRQCHIP: + node = ctxt->node; + ctxt->node = nodes[i]; + tmp = virXPathString("string(./@mode)", ctxt); + if (tmp) { + int value = virDomainIRQChipTypeFromString(tmp); + if (value < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown irqchip mode: %s"), + tmp); + goto error; + } + def->irqchip = value; + def->features[val] = VIR_TRISTATE_SWITCH_ON; + } + ctxt->node = node; + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_FEATURE_LAST: break; @@ -24520,6 +24545,13 @@ virDomainDefFormatInternal(virDomainDefPtr def, } break; + case VIR_DOMAIN_FEATURE_IRQCHIP: + if (def->features[i] == VIR_TRISTATE_SWITCH_ON) { + virBufferAsprintf(buf, "<irqchip mode='%s'/>\n", + virDomainIRQChipTypeToString(def->irqchip)); + } + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_FEATURE_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7da554f..97c4418 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1671,6 +1671,7 @@ typedef enum { VIR_DOMAIN_FEATURE_VMPORT, VIR_DOMAIN_FEATURE_GIC, VIR_DOMAIN_FEATURE_SMM, + VIR_DOMAIN_FEATURE_IRQCHIP, VIR_DOMAIN_FEATURE_LAST } virDomainFeature; @@ -1810,6 +1811,16 @@ struct _virDomainLoaderDef { void virDomainLoaderDefFree(virDomainLoaderDefPtr loader); +typedef enum { + VIR_DOMAIN_IRQCHIP_OFF = 0, + VIR_DOMAIN_IRQCHIP_SPLIT, + VIR_DOMAIN_IRQCHIP_ON, + + VIR_DOMAIN_IRQCHIP_LAST +} virDomainIRQChip; + +VIR_ENUM_DECL(virDomainIRQChip); + /* Operating system configuration data & machine / arch */ typedef struct _virDomainOSDef virDomainOSDef; typedef virDomainOSDef *virDomainOSDefPtr; @@ -2259,6 +2270,7 @@ struct _virDomainDef { unsigned int hyperv_spinlocks; virGICVersion gic_version; char *hyperv_vendor_id; + virDomainIRQChip irqchip; /* These options are of type virTristateSwitch: ON = keep, OFF = drop */ int caps_features[VIR_DOMAIN_CAPS_FEATURE_LAST]; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml new file mode 100644 index 0000000..cc895af --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml @@ -0,0 +1,29 @@ +<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> + <features> + <irqchip mode='split'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <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='none'/> + <iommu model='intel'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml new file mode 120000 index 0000000..58a0199 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e4b510f..c7d4788 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1121,6 +1121,7 @@ mymain(void) DO_TEST("intel-iommu-machine", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); + DO_TEST("intel-iommu-irqchip", NONE); DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE); -- 2.10.2

On 04/20/2017 08:19 AM, Ján Tomko wrote:
Add a new <irqchip> element with a mode attribute.
Possible values are off, split or on.
https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 10 +++++++ docs/schemas/domaincommon.rng | 16 ++++++++++ src/conf/domain_conf.c | 34 +++++++++++++++++++++- src/conf/domain_conf.h | 12 ++++++++ .../qemuxml2argv-intel-iommu-irqchip.xml | 29 ++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-irqchip.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b1e38f0..abf089a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1643,6 +1643,7 @@ </kvm> <pvspinlock state='on'/> <gic version='2'/> + <irqchip mode='split'/>
</features> ...</pre> @@ -1804,6 +1805,15 @@ for hypervisor to decide. <span class="since">Since 2.1.0</span> </dd> + <dt><code>irqchip</code></dt> + <dd>Tune the in-kernel irqchip. Possible values for the + <code>mode</code> attribute are: + <code>on</code>, <code>split</code> and <code>off</code>. + <code>split</code> is useful for using interrupt remapping + with the <a href="#elementsIommu">IOMMU device</a>.
Something that isn't implemented until the subsequent patch, but I'm not against describing this feature a bit more here... I think most importantly what setting this feature will "do" would be useful. How does someone know they need this? And secondarily what would it be required for? What does "on" really do? IOW: What the difference between split and on.
+ The default is left for hypervisor to decide. + <span class="since">Since 3.3.0</span> (QEMU only) + </dd> </dl>
<h3><a name="elementsTime">Time keeping</a></h3>
...
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml new file mode 100644 index 0000000..cc895af --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml @@ -0,0 +1,29 @@ +<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> + <features> + <irqchip mode='split'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <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='none'/> + <iommu model='intel'/> + </devices> +</domain>
Could a few devices be added so that future patches will actually generate a command line that would should the code conforms to the requirement that the "-device intel-iommu" appears in the command list before the "rest of the devices". John
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml new file mode 120000 index 0000000..58a0199 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e4b510f..c7d4788 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1121,6 +1121,7 @@ mymain(void) DO_TEST("intel-iommu-machine", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); + DO_TEST("intel-iommu-irqchip", NONE);
DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE);

On Mon, Apr 24, 2017 at 05:40:07PM -0400, John Ferlan wrote:
On 04/20/2017 08:19 AM, Ján Tomko wrote:
Add a new <irqchip> element with a mode attribute.
Possible values are off, split or on.
https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 10 +++++++ docs/schemas/domaincommon.rng | 16 ++++++++++ src/conf/domain_conf.c | 34 +++++++++++++++++++++- src/conf/domain_conf.h | 12 ++++++++ .../qemuxml2argv-intel-iommu-irqchip.xml | 29 ++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-irqchip.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b1e38f0..abf089a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1643,6 +1643,7 @@ </kvm> <pvspinlock state='on'/> <gic version='2'/> + <irqchip mode='split'/>
</features> ...</pre> @@ -1804,6 +1805,15 @@ for hypervisor to decide. <span class="since">Since 2.1.0</span> </dd> + <dt><code>irqchip</code></dt> + <dd>Tune the in-kernel irqchip. Possible values for the + <code>mode</code> attribute are: + <code>on</code>, <code>split</code> and <code>off</code>. + <code>split</code> is useful for using interrupt remapping + with the <a href="#elementsIommu">IOMMU device</a>.
Something that isn't implemented until the subsequent patch, but I'm not against describing this feature a bit more here...
What would you say?
I think most importantly what setting this feature will "do" would be useful. How does someone know they need this?
It is needed if they want interrupt remapping for assigned devices. They can find out from a guide like: http://wiki.qemu.org/Features/VT-d#References or the linked BZ, or from the error message QEMU reports when they try to use interrupt remapping from libvirt without setting this to "split".
And secondarily what would it be required for? What does "on" really do? IOW: What the difference between split and on.
IIUC options other than "split" aren't that useful. Jan

Add kernel_irqchip=off/split/on to the QEMU command line and a capability that looks for it in query-command-line-options output. https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 11 +++++++++++ tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-intel-iommu-irqchip.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++++ 21 files changed, 53 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 181e178..f903c2c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -381,6 +381,7 @@ virDomainIOThreadIDAdd; virDomainIOThreadIDDefFree; virDomainIOThreadIDDel; virDomainIOThreadIDFind; +virDomainIRQChipTypeToString; virDomainKeyWrapCipherNameTypeFromString; virDomainKeyWrapCipherNameTypeToString; virDomainLeaseDefFree; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cc3e1f8..72870b9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -364,6 +364,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-cpu-definitions", /* 250 */ "block-write-threshold", "query-named-block-nodes", + "kernel-irqchip", ); @@ -3124,6 +3125,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "drive", "throttling.bps-total-max-length", QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH }, { "drive", "throttling.group", QEMU_CAPS_DRIVE_IOTUNE_GROUP }, { "spice", "rendernode", QEMU_CAPS_SPICE_RENDERNODE }, + { "machine", "kernel_irqchip", QEMU_CAPS_MACHINE_KERNEL_IRQCHIP }, }; static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index a7eec52..362347e 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -401,6 +401,7 @@ typedef enum { QEMU_CAPS_QUERY_CPU_DEFINITIONS, /* qmp query-cpu-definitions */ 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_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b2e76ca..1918933 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7379,6 +7379,17 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, } } + if (def->features[VIR_DOMAIN_FEATURE_IRQCHIP] == VIR_TRISTATE_SWITCH_ON) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("kernel-irqchip option is not supported by this " + "QEMU binary")); + goto cleanup; + } + virBufferAsprintf(&buf, ",kernel_irqchip=%s", + virDomainIRQChipTypeToString(def->irqchip)); + } + virCommandAddArgBuffer(cmd, &buf); } diff --git a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml index a68c13b..14f34b2 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml @@ -140,6 +140,7 @@ <flag name='display'/> <flag name='vhost-scsi'/> <flag name='query-cpu-definitions'/> + <flag name='kernel-irqchip'/> <version>1005003</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml index 365b3a6..8fc23d6 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml @@ -145,6 +145,7 @@ <flag name='display'/> <flag name='vhost-scsi'/> <flag name='query-cpu-definitions'/> + <flag name='kernel-irqchip'/> <version>1006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml index 689fbf8..47c8956 100644 --- a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml @@ -147,6 +147,7 @@ <flag name='display'/> <flag name='vhost-scsi'/> <flag name='query-cpu-definitions'/> + <flag name='kernel-irqchip'/> <version>1007000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml index e092dd2..afe7d53 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml @@ -163,6 +163,7 @@ <flag name='vhost-scsi'/> <flag name='query-cpu-definitions'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2001001</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml index ea03f2e..5bdc1a2 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -185,6 +185,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2004000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml index 164605f..36bc134 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -191,6 +191,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2005000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml index af3a8e7..46e77b7 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml @@ -168,6 +168,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml index 4402ffa..0c15b92 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml @@ -168,6 +168,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml index 3f05169..c2ee94d 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml @@ -162,6 +162,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index 1d823ea..0583d6a 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -200,6 +200,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml index 38d36b3..49baca8 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml @@ -130,6 +130,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index 24d7cb4..bee3ce0 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -202,6 +202,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml index 42c92c5..4fc20df 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -132,6 +132,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2007093</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml index 0bc1368..4e00816 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml @@ -203,6 +203,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2008000</version> <kvmVersion>0</kvmVersion> <package> (v2.8.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 6386c4e..1ca4df9 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -207,6 +207,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <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 new file mode 100644 index 0000000..899d240 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args @@ -0,0 +1,19 @@ +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,kernel_irqchip=split \ +-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 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c1b014b..80f10cc 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2492,6 +2492,10 @@ mymain(void) DO_TEST("intel-iommu-machine", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); + DO_TEST("intel-iommu-irqchip", + QEMU_CAPS_MACHINE_OPT, + QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, + QEMU_CAPS_DEVICE_INTEL_IOMMU); DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS); -- 2.10.2

On 04/20/2017 08:19 AM, Ján Tomko wrote:
Add kernel_irqchip=off/split/on to the QEMU command line and a capability that looks for it in query-command-line-options output.
https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 11 +++++++++++ tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-intel-iommu-irqchip.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++++ 21 files changed, 53 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args
ACK, 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 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) + </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> -- 2.10.2

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>

On Mon, Apr 24, 2017 at 05:42:07PM -0400, John Ferlan wrote:
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.
Per http://wiki.qemu.org/Features/VT-d: Currently, interrupt remapping does not support full kernel irqchip, only "split" and "off" are supported. We should not put current QEMU implementation limits into XML post processing.
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.
Interrupt remapping is part of the VT-d feature.
Is there something we could describe here at a very high level to describe the usefulness/need for this parameter?
IMO that is out of scope of the element description. Jan

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 1918933..1f97bd0 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 04/20/2017 08:19 AM, Ján Tomko wrote:
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(-)
ACK - John

Add a new attribute to control the caching mode. https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 9 +++++++ docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 24 ++++++++++++++++--- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-caching.xml | 28 ++++++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-caching.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f5a8e76..dbca316 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7364,6 +7364,15 @@ qemu-kvm -net nic,model=? /dev/null (QEMU only) </p> </dd> + <dt><code>caching</code></dt> + <dd> + <p> + The <code>caching</code> attribute with possible values + <code>on</code> and <code>off</code> can be used to + turn on the caching mode. <span class="since">Since 3.3.0</span> + (QEMU only) + </p> + </dd> </dl> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7930d85..b237140 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3896,6 +3896,11 @@ <ref name="virOnOff"/> </attribute> </optional> + <optional> + <attribute name="caching"> + <ref name="virOnOff"/> + </attribute> + </optional> </element> </optional> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d40d129..cdeb60b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14092,6 +14092,15 @@ virDomainIOMMUDefParseXML(xmlNodePtr node, iommu->intremap = val; } + VIR_FREE(tmp); + if ((tmp = virXPathString("string(./driver/@caching)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unknown caching value: %s"), tmp); + goto cleanup; + } + iommu->caching = val; + } + ret = iommu; iommu = NULL; @@ -24037,9 +24046,18 @@ virDomainIOMMUDefFormat(virBufferPtr buf, virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2); - if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) { - virBufferAsprintf(&childBuf, "<driver intremap='%s'/>\n", - virTristateSwitchTypeToString(iommu->intremap)); + if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT || + iommu->caching != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAddLit(&childBuf, "<driver"); + if (iommu->intremap) { + virBufferAsprintf(&childBuf, " intremap='%s'", + virTristateSwitchTypeToString(iommu->intremap)); + } + if (iommu->caching != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&childBuf, " caching='%s'", + virTristateSwitchTypeToString(iommu->caching)); + } + virBufferAddLit(&childBuf, "/>\n"); } virBufferAsprintf(buf, "<iommu model='%s'", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f95649c..1afbaff 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2212,6 +2212,7 @@ typedef enum { struct _virDomainIOMMUDef { virDomainIOMMUModel model; virTristateSwitch intremap; + virTristateSwitch caching; }; /* * Guest VM main configuration diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml new file mode 100644 index 0000000..bab4fcb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml @@ -0,0 +1,28 @@ +<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-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <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='none'/> + <iommu model='intel'> + <driver intremap='on' caching='on'/> + </iommu> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml new file mode 120000 index 0000000..6935d93 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c7d4788..a4d5e97 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1122,6 +1122,7 @@ mymain(void) QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); DO_TEST("intel-iommu-irqchip", NONE); + DO_TEST("intel-iommu-caching", NONE); DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE); -- 2.10.2

On 04/20/2017 08:19 AM, Ján Tomko wrote:
Add a new attribute to control the caching mode.
https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 9 +++++++ docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 24 ++++++++++++++++--- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-caching.xml | 28 ++++++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-caching.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f5a8e76..dbca316 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7364,6 +7364,15 @@ qemu-kvm -net nic,model=? /dev/null (QEMU only) </p> </dd> + <dt><code>caching</code></dt> + <dd> + <p> + The <code>caching</code> attribute with possible values + <code>on</code> and <code>off</code> can be used to + turn on the caching mode. <span class="since">Since 3.3.0</span> + (QEMU only)
Useful for what? Required for what? A bit more description about the relationship with intremap might be nice as well. Is there a relationship with the irqmode settings? If so, then I would think there should be checks in domain post processing that ensure that if this is on, then the irqmode setting is adjusted appropriately.
+ </p> + </dd> </dl> </dd> </dl>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml new file mode 100644 index 0000000..bab4fcb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml @@ -0,0 +1,28 @@ +<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-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <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='none'/> + <iommu model='intel'> + <driver intremap='on' caching='on'/> + </iommu> + </devices> +</domain>
Maybe this is the XML that gets more devices added that prove the ordering is correct. IDC which one, but we should "ensure" the ordering is right.. John

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 | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 11 +++++++++++ tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-intel-iommu-caching.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 5 +++++ 6 files changed, 39 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..88ea306 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -366,6 +366,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-named-block-nodes", "kernel-irqchip", "intel-iommu-intremap", + "intel-iommu-caching", ); @@ -1718,6 +1719,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..2409377 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -403,6 +403,7 @@ typedef enum { 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_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 1f97bd0..55800c7 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..dee63f4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args @@ -0,0 +1,19 @@ +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 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 29cd8b5..9950f7f 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2497,6 +2497,11 @@ 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_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

On 04/20/2017 08:19 AM, Ján Tomko wrote:
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 | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 11 +++++++++++ tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-intel-iommu-caching.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 5 +++++ 6 files changed, 39 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..88ea306 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -366,6 +366,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-named-block-nodes", "kernel-irqchip", "intel-iommu-intremap",
This is the 255th capability so it'll need separation like every other 5th element with a comment after the ", /* 255 */
+ "intel-iommu-caching", );
@@ -1718,6 +1719,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..2409377 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -403,6 +403,7 @@ typedef enum { 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 */
Likewise, /* 255 */ Needs to be added.
+ 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 1f97bd0..55800c7 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..dee63f4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args @@ -0,0 +1,19 @@ +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
Hmm.... so this proves that kernel-irqchip=split (or =on) isn't required then? It's just not clear, so I'm wondering. Could be adjustments to the .args. based on patch5, but I think they're obvious... ACK with at least the comments added like other capabilities. John
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 29cd8b5..9950f7f 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2497,6 +2497,11 @@ 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_INTEL_IOMMU, + QEMU_CAPS_INTEL_IOMMU_INTREMAP, + QEMU_CAPS_INTEL_IOMMU_CACHING);
DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS);
participants (2)
-
John Ferlan
-
Ján Tomko