[libvirt] [PATCH 0/9] Support more options for intel-iommu
https://bugzilla.redhat.com/show_bug.cgi?id=1427005 Ján Tomko (9): conf: add <irqchip mode> to <features> qemu: format kernel_irqchip on the command line Split out virDomainIOMMUDefFormat conf: add <driver intremap> to <iommu> qemu: allow conditional device property probing qemu: refactor qemuBuildIOMMUCommandLine 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 | 103 +++++++++++++++++-- src/conf/domain_conf.h | 15 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 111 +++++++++++++++------ src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 53 ++++++++-- 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 | 28 ++++++ .../qemuxml2xmlout-intel-iommu-irqchip.xml | 31 ++++++ tests/qemuxml2xmltest.c | 4 + 37 files changed, 641 insertions(+), 89 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 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml create mode 100644 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 | 35 +++++++++++++++++++++- src/conf/domain_conf.h | 13 ++++++++ .../qemuxml2argv-intel-iommu-irqchip.xml | 29 ++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-irqchip.xml | 29 ++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 7 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 4a3123e..32a5f18 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1627,6 +1627,7 @@ </kvm> <pvspinlock state='on'/> <gic version='2'/> + <irqchip mode='split'/> </features> ...</pre> @@ -1788,6 +1789,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 fbedc9b..8ba3902 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4489,6 +4489,9 @@ </optional> </element> </optional> + <optional> + <ref name="irqchip"/> + </optional> </interleave> </element> </optional> @@ -4664,6 +4667,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 6bbc6a2..ffc6a68 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -139,7 +139,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", @@ -855,6 +856,13 @@ 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) @@ -17278,6 +17286,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; @@ -24337,6 +24363,13 @@ virDomainDefFormatInternal(virDomainDefPtr def, } break; + case VIR_DOMAIN_FEATURE_IRQCHIP: + if (def->irqchip) { + 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 8e6d874..762e64e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1662,6 +1662,7 @@ typedef enum { VIR_DOMAIN_FEATURE_VMPORT, VIR_DOMAIN_FEATURE_GIC, VIR_DOMAIN_FEATURE_SMM, + VIR_DOMAIN_FEATURE_IRQCHIP, VIR_DOMAIN_FEATURE_LAST } virDomainFeature; @@ -1801,6 +1802,17 @@ struct _virDomainLoaderDef { void virDomainLoaderDefFree(virDomainLoaderDefPtr loader); +typedef enum { + VIR_DOMAIN_IRQCHIP_DEFAULT = 0, + VIR_DOMAIN_IRQCHIP_OFF, + 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; @@ -2250,6 +2262,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..98e4bba --- /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</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 100644 index 0000000..98e4bba --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-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</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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 266b9c0..8afd830 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1124,6 +1124,8 @@ mymain(void) DO_TEST("intel-iommu-machine", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); + DO_TEST("intel-iommu-irqchip", + QEMU_CAPS_DEVICE_INTEL_IOMMU); DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE); -- 2.10.2
On 03/23/2017 11:26 AM, Ján Tomko wrote:
Add a new <irqchip> element with a mode attribute.
Possible values are off, split or on.
Shouldn't this just go on the "last" patch in the series. IDC really, but if you're going to add to all patches, then all patches should have it listed.
--- docs/formatdomain.html.in | 10 +++++++ docs/schemas/domaincommon.rng | 16 ++++++++++ src/conf/domain_conf.c | 35 +++++++++++++++++++++- src/conf/domain_conf.h | 13 ++++++++ .../qemuxml2argv-intel-iommu-irqchip.xml | 29 ++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-irqchip.xml | 29 ++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 7 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 4a3123e..32a5f18 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1627,6 +1627,7 @@ </kvm> <pvspinlock state='on'/> <gic version='2'/> + <irqchip mode='split'/>
</features> ...</pre> @@ -1788,6 +1789,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>
Did you somehow know this wouldn't be reviewed in time for 3.2.0? ;-) Once I got to patch 4 & 7, I began to wonder if support there was dependence upon the value being "split" or is "on" also acceptable. It would seem "off" and not present wouldn't allow intremap or caching to work. At the very least whatever it is that allows the -device intel-iommu to be present in QEMU 2.7 (if I read virQEMUCapsInitQMPMonitor correctly) instead of ",iommu=on" would also seem to be a requirement. I know we don't want to put versions there, but whatever it is that is required should be listed (at least while it's still fresh in your mind).
</dl>
<h3><a name="elementsTime">Time keeping</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index fbedc9b..8ba3902 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4489,6 +4489,9 @@ </optional> </element> </optional> + <optional> + <ref name="irqchip"/> + </optional> </interleave> </element> </optional> @@ -4664,6 +4667,19 @@ </element> </define>
+ <define name="irqchip"> + <element name="irqchip"> + <attribute name="mode"> + <choice>
should there be a "default" (or undefined - see below) to match the enum impl list? or just left empty since there's a features tristate set on parse (and should be used to control whether it gets formatted).
+ <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 6bbc6a2..ffc6a68 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -139,7 +139,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", @@ -855,6 +856,13 @@ VIR_ENUM_IMPL(virDomainLoader, "rom", "pflash")
+VIR_ENUM_IMPL(virDomainIRQChip, + VIR_DOMAIN_IRQCHIP_LAST, + "",
Should this be "undefined"? Or just go away since there's a tristate to manage the parse and (not yet) format.
+ "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) @@ -17278,6 +17286,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; @@ -24337,6 +24363,13 @@ virDomainDefFormatInternal(virDomainDefPtr def, } break;
+ case VIR_DOMAIN_FEATURE_IRQCHIP:
This is where I would expect to see a check like other features for "if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT)" (or == ON) since you set def->features[val] = VIR_TRISTATE_SWITCH_ON; above. Using the tristate removes the need to have that default/undefined or "" value.
+ if (def->irqchip) { + 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 8e6d874..762e64e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1662,6 +1662,7 @@ typedef enum { VIR_DOMAIN_FEATURE_VMPORT, VIR_DOMAIN_FEATURE_GIC, VIR_DOMAIN_FEATURE_SMM, + VIR_DOMAIN_FEATURE_IRQCHIP,
VIR_DOMAIN_FEATURE_LAST } virDomainFeature; @@ -1801,6 +1802,17 @@ struct _virDomainLoaderDef {
void virDomainLoaderDefFree(virDomainLoaderDefPtr loader);
+typedef enum { + VIR_DOMAIN_IRQCHIP_DEFAULT = 0,
I think your description would make this UNDEFINED... Of course the tristate setting makes this value unnecessary.
+ VIR_DOMAIN_IRQCHIP_OFF, + 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; @@ -2250,6 +2262,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..98e4bba --- /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</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 100644 index 0000000..98e4bba --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
Since there's no difference between this and tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml this should thus follow the example of other outdata files using a link to the argvdata file (the gic files provide examples. Impacts subsequent patches that need to change both files.
@@ -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</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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 266b9c0..8afd830 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1124,6 +1124,8 @@ mymain(void) DO_TEST("intel-iommu-machine", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); + DO_TEST("intel-iommu-irqchip", + QEMU_CAPS_DEVICE_INTEL_IOMMU);
Should this also set QEMU_CAPS_MACHINE_OPT and QEMU_CAPS_DEVICE_INTEL_IOMMU too? Although I suppose it only the caps only matter on xml2argv...
DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE);
Conditional ACK w/ suggested adjustments - I'm open to keeping the "undefined" entry, but I think it should be removed. John
On Tue, Mar 28, 2017 at 02:50:58PM -0400, John Ferlan wrote:
On 03/23/2017 11:26 AM, Ján Tomko wrote:
@@ -1788,6 +1789,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>
Did you somehow know this wouldn't be reviewed in time for 3.2.0? ;-)
Once I got to patch 4 & 7, I began to wonder if support there was dependence upon the value being "split" or is "on" also acceptable. It would seem "off" and not present wouldn't allow intremap or caching to work.
Looking at QEMU code, off also works but it seems the restriction is temporary.
At the very least whatever it is that allows the -device intel-iommu to be present in QEMU 2.7 (if I read virQEMUCapsInitQMPMonitor correctly) instead of ",iommu=on" would also seem to be a requirement. I know we don't want to put versions there, but whatever it is that is required should be listed (at least while it's still fresh in your mind).
The intremap option was introduced in QEMU 2.9, so libvirt cannot possibly support it with 2.7's -machine iommu=on. 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 a1c7624..d14b81a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -375,6 +375,7 @@ virDomainIOThreadIDAdd; virDomainIOThreadIDDefFree; virDomainIOThreadIDDel; virDomainIOThreadIDFind; +virDomainIRQChipTypeToString; virDomainKeyWrapCipherNameTypeFromString; virDomainKeyWrapCipherNameTypeToString; virDomainLeaseDefFree; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 60ed31e..05d0a91 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -363,6 +363,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "pcie-root-port", "query-cpu-definitions", /* 250 */ + "kernel-irqchip", ); @@ -2986,6 +2987,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 55777f9..b9efab8 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -399,6 +399,7 @@ typedef enum { /* 250 */ QEMU_CAPS_QUERY_CPU_DEFINITIONS, /* qmp query-cpu-definitions */ + 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 2045c2e..58af585 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7316,6 +7316,17 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, } } + if (def->irqchip) { + 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 211c0bf..6bdb5a6 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml @@ -162,6 +162,7 @@ <flag name='display'/> <flag name='vhost-scsi'/> <flag name='query-cpu-definitions'/> + <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 a348bc3..6e861da 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -183,6 +183,7 @@ <flag name='vhost-scsi'/> <flag name='drive-iotune-group'/> <flag name='query-cpu-definitions'/> + <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 f198715..634a76c 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -189,6 +189,7 @@ <flag name='vhost-scsi'/> <flag name='drive-iotune-group'/> <flag name='query-cpu-definitions'/> + <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 f45560b..4b8ddcd 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml @@ -165,6 +165,7 @@ <flag name='vhost-scsi'/> <flag name='drive-iotune-group'/> <flag name='query-cpu-definitions'/> + <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 721c97b..c708b70 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml @@ -165,6 +165,7 @@ <flag name='vhost-scsi'/> <flag name='drive-iotune-group'/> <flag name='query-cpu-definitions'/> + <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 f1c5105..aec03d6 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml @@ -160,6 +160,7 @@ <flag name='vhost-scsi'/> <flag name='drive-iotune-group'/> <flag name='query-cpu-definitions'/> + <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 74b8e9a..a67ca61 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -198,6 +198,7 @@ <flag name='vhost-scsi'/> <flag name='drive-iotune-group'/> <flag name='query-cpu-definitions'/> + <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 4c45b38..25d98cc 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml @@ -128,6 +128,7 @@ <flag name='vhost-scsi'/> <flag name='drive-iotune-group'/> <flag name='query-cpu-definitions'/> + <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 d6b589c..0bc3dcd 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -200,6 +200,7 @@ <flag name='vhost-scsi'/> <flag name='drive-iotune-group'/> <flag name='query-cpu-definitions'/> + <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 215159c..7018575 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -130,6 +130,7 @@ <flag name='drive-iotune-group'/> <flag name='query-cpu-model-expansion'/> <flag name='query-cpu-definitions'/> + <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 a4a97a7..fd0c927 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml @@ -201,6 +201,7 @@ <flag name='vhost-scsi'/> <flag name='drive-iotune-group'/> <flag name='query-cpu-definitions'/> + <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 f1adb4d..9dcb84e 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -205,6 +205,7 @@ <flag name='spice-rendernode'/> <flag name='pcie-root-port'/> <flag name='query-cpu-definitions'/> + <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..ebf9c3e --- /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 \ +-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 64e14af..11f6130 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2475,6 +2475,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 03/23/2017 11:26 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
You'll have merge conflicts here to resolve - something I'm sure you already know.
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a1c7624..d14b81a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -375,6 +375,7 @@ virDomainIOThreadIDAdd; virDomainIOThreadIDDefFree; virDomainIOThreadIDDel; virDomainIOThreadIDFind; +virDomainIRQChipTypeToString; virDomainKeyWrapCipherNameTypeFromString; virDomainKeyWrapCipherNameTypeToString; virDomainLeaseDefFree; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 60ed31e..05d0a91 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -363,6 +363,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "pcie-root-port",
"query-cpu-definitions", /* 250 */ + "kernel-irqchip", );
@@ -2986,6 +2987,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 55777f9..b9efab8 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -399,6 +399,7 @@ typedef enum {
/* 250 */ QEMU_CAPS_QUERY_CPU_DEFINITIONS, /* qmp query-cpu-definitions */ + 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 2045c2e..58af585 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7316,6 +7316,17 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, } }
+ if (def->irqchip) {
There's a relationship between irqchip setting and the features tristate that I think should be used here to remove the need for checking def->irqchip (and keeping a default/undefined value - from patch1). I think the GIC example a few lines above provides an example. Additionally does this code need to check if QEMU_CAPS_MACHINE_IOMMU was not set or preferably that QEMU_CAPS_DEVICE_INTEL_IOMMU is set in order to ensure the -device intel-iommu will be present? It's my assumption from reading patch1 html.in and the reading the comments in virQEMUCapsInitQMPMonitor regarding iommu that having ",iommu=on" may not work. It gets even more confusing in subsequent patches for intremap (support starting w/ qemu 2.4) and caching (support starting with qemu 2.9). Whether you drop a comment here or the commit message regarding what's supported could help someone in the future trying to figure this out.
+ 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/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args new file mode 100644 index 0000000..ebf9c3e --- /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 \ +-name QEMUGuest1 \ +-S \ +-machine q35,accel=tcg,kernel_irqchip=split \
If we can support with ",iommu=on", then we should probably have a second test for that which uses QEMU_CAPS_MACHINE_IOMMU Conditional ACK based on the fallout of the commandline building John
+-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 64e14af..11f6130 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2475,6 +2475,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);
Make adding subelements easier. --- src/conf/domain_conf.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ffc6a68..1245fdd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23842,6 +23842,15 @@ virDomainDefIothreadShouldFormat(virDomainDefPtr def) } +static void +virDomainIOMMUDefFormat(virBufferPtr buf, + virDomainIOMMUDefPtr iommu) +{ + virBufferAsprintf(buf, "<iommu model='%s'/>\n", + virDomainIOMMUModelTypeToString(iommu->model)); +} + + /* This internal version appends to an existing buffer * (possibly with auto-indent), rather than flattening * to string. @@ -24595,10 +24604,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto error; } - if (def->iommu) { - virBufferAsprintf(buf, "<iommu model='%s'/>\n", - virDomainIOMMUModelTypeToString(def->iommu->model)); - } + if (def->iommu) + virDomainIOMMUDefFormat(buf, def->iommu); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</devices>\n"); -- 2.10.2
On 03/23/2017 11:26 AM, Ján Tomko wrote:
Make adding subelements easier.
No bz reference here
--- src/conf/domain_conf.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ffc6a68..1245fdd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23842,6 +23842,15 @@ virDomainDefIothreadShouldFormat(virDomainDefPtr def) }
+static void +virDomainIOMMUDefFormat(virBufferPtr buf, + virDomainIOMMUDefPtr iommu)
Could also use const virDomainIOMMUDef *iommu here I think ACK regardless, John
+{ + virBufferAsprintf(buf, "<iommu model='%s'/>\n", + virDomainIOMMUModelTypeToString(iommu->model)); +} + + /* This internal version appends to an existing buffer * (possibly with auto-indent), rather than flattening * to string. @@ -24595,10 +24604,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto error; }
- if (def->iommu) { - virBufferAsprintf(buf, "<iommu model='%s'/>\n", - virDomainIOMMUModelTypeToString(def->iommu->model)); - } + if (def->iommu) + virDomainIOMMUDefFormat(buf, def->iommu);
virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</devices>\n");
Add a new attribute to control interrupt remapping. https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 22 ++++++++++++- docs/schemas/domaincommon.rng | 9 +++++ src/conf/domain_conf.c | 38 +++++++++++++++++++--- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-irqchip.xml | 4 ++- .../qemuxml2xmlout-intel-iommu-irqchip.xml | 4 ++- 6 files changed, 71 insertions(+), 7 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 32a5f18..6b196d4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7285,7 +7285,9 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> - <iommu model='intel'/> + <iommu model='intel'> + <driver intremap='on'/> + </iommu> </devices> ... </pre> @@ -7296,6 +7298,24 @@ qemu-kvm -net nic,model=? /dev/null Currently only the <code>intel</code> model is supported. </p> </dd> + <dt><code>driver</code></dt> + <dd> + <p> + The <code>driver</code> subelement can be used to configure + additional options: + </p> + <dl> + <dt><code>intremap</code></dt> + <dd> + <p> + The <code>intremap</code> attribute with possible values + <code>on</code> and <code>off</code> can be used to + turn on interrupt remapping. <span class="since">Since 3.3.0</span> + (QEMU only) + </p> + </dd> + </dl> + </dd> </dl> <h3><a name="seclabel">Security label</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 8ba3902..68f3680 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3889,6 +3889,15 @@ <attribute name="model"> <value>intel</value> </attribute> + <optional> + <element name="driver"> + <optional> + <attribute name="intremap"> + <ref name="virOnOff"/> + </attribute> + </optional> + </element> + </optional> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1245fdd..da0c9f0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13909,12 +13909,16 @@ virDomainMemoryDefParseXML(xmlNodePtr memdevNode, static virDomainIOMMUDefPtr -virDomainIOMMUDefParseXML(xmlNodePtr node) +virDomainIOMMUDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt) { virDomainIOMMUDefPtr iommu = NULL, ret = NULL; + xmlNodePtr save = ctxt->node; char *tmp = NULL; int val; + ctxt->node = node; + if (VIR_ALLOC(iommu) < 0) goto cleanup; @@ -13931,10 +13935,20 @@ virDomainIOMMUDefParseXML(xmlNodePtr node) iommu->model = val; + VIR_FREE(tmp); + if ((tmp = virXPathString("string(./driver/@intremap)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unknown intremap value: %s"), tmp); + goto cleanup; + } + iommu->intremap = val; + } + ret = iommu; iommu = NULL; cleanup: + ctxt->node = save; VIR_FREE(iommu); VIR_FREE(tmp); return ret; @@ -14087,7 +14101,7 @@ virDomainDeviceDefParse(const char *xmlStr, goto error; break; case VIR_DOMAIN_DEVICE_IOMMU: - if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node))) + if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node, ctxt))) goto error; break; case VIR_DOMAIN_DEVICE_NONE: @@ -18202,7 +18216,7 @@ virDomainDefParseXML(xmlDocPtr xml, } if (n > 0) { - if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0]))) + if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0], ctxt))) goto error; } VIR_FREE(nodes); @@ -23846,8 +23860,24 @@ static void virDomainIOMMUDefFormat(virBufferPtr buf, virDomainIOMMUDefPtr iommu) { - virBufferAsprintf(buf, "<iommu model='%s'/>\n", + virBuffer childBuf = VIR_BUFFER_INITIALIZER; + + virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2); + + if (iommu->intremap) { + virBufferAsprintf(&childBuf, "<driver intremap='%s'/>\n", + virTristateSwitchTypeToString(iommu->intremap)); + } + + virBufferAsprintf(buf, "<iommu model='%s'", virDomainIOMMUModelTypeToString(iommu->model)); + if (virBufferUse(&childBuf)) { + virBufferAddLit(buf, ">\n"); + virBufferAddBuffer(buf, &childBuf); + virBufferAddLit(buf, "</iommu>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 762e64e..ad8ae2d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2203,6 +2203,7 @@ typedef enum { struct _virDomainIOMMUDef { virDomainIOMMUModel model; + virTristateSwitch intremap; }; /* * Guest VM main configuration diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml index 98e4bba..3cef53d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml @@ -24,6 +24,8 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <memballoon model='none'/> - <iommu model='intel'/> + <iommu model='intel'> + <driver intremap='on'/> + </iommu> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml index 98e4bba..3cef53d 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml @@ -24,6 +24,8 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <memballoon model='none'/> - <iommu model='intel'/> + <iommu model='intel'> + <driver intremap='on'/> + </iommu> </devices> </domain> -- 2.10.2
On 03/23/2017 11:26 AM, Ján Tomko wrote:
Add a new attribute to control interrupt remapping.
https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 22 ++++++++++++- docs/schemas/domaincommon.rng | 9 +++++ src/conf/domain_conf.c | 38 +++++++++++++++++++--- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-irqchip.xml | 4 ++- .../qemuxml2xmlout-intel-iommu-irqchip.xml | 4 ++- 6 files changed, 71 insertions(+), 7 deletions(-)
awful name "intremap", but just following QEMU...
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 32a5f18..6b196d4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7285,7 +7285,9 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> - <iommu model='intel'/> + <iommu model='intel'> + <driver intremap='on'/> + </iommu> </devices> ... </pre> @@ -7296,6 +7298,24 @@ qemu-kvm -net nic,model=? /dev/null Currently only the <code>intel</code> model is supported. </p> </dd> + <dt><code>driver</code></dt> + <dd> + <p> + The <code>driver</code> subelement can be used to configure + additional options: + </p> + <dl> + <dt><code>intremap</code></dt> + <dd> + <p> + The <code>intremap</code> attribute with possible values + <code>on</code> and <code>off</code> can be used to + turn on interrupt remapping. <span class="since">Since 3.3.0</span> + (QEMU only) + </p> + </dd> + </dl> + </dd>
If I read patch1 correctly, support of this feature is dependent upon the 'irqchip' feature being set to (at least, it seems) "split". Whether "on" would be valid or not is not clear. In any case, since there is a dependency, it should be listed here and a doc link/reference to the irqchip feature description? Since patch7 only adds support for this when there's a "-device intel-iommu" and not when there's ",iommu=on", there's also another dependency that needs to be described.
</dl>
<h3><a name="seclabel">Security label</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 8ba3902..68f3680 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3889,6 +3889,15 @@ <attribute name="model"> <value>intel</value> </attribute> + <optional> + <element name="driver"> + <optional> + <attribute name="intremap"> + <ref name="virOnOff"/> + </attribute> + </optional> + </element> + </optional> </element> </define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1245fdd..da0c9f0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13909,12 +13909,16 @@ virDomainMemoryDefParseXML(xmlNodePtr memdevNode,
static virDomainIOMMUDefPtr -virDomainIOMMUDefParseXML(xmlNodePtr node) +virDomainIOMMUDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt) { virDomainIOMMUDefPtr iommu = NULL, ret = NULL; + xmlNodePtr save = ctxt->node; char *tmp = NULL; int val;
+ ctxt->node = node; + if (VIR_ALLOC(iommu) < 0) goto cleanup;
@@ -13931,10 +13935,20 @@ virDomainIOMMUDefParseXML(xmlNodePtr node)
iommu->model = val;
+ VIR_FREE(tmp); + if ((tmp = virXPathString("string(./driver/@intremap)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unknown intremap value: %s"), tmp); + goto cleanup; + } + iommu->intremap = val; + } + ret = iommu; iommu = NULL;
cleanup: + ctxt->node = save; VIR_FREE(iommu); VIR_FREE(tmp); return ret; @@ -14087,7 +14101,7 @@ virDomainDeviceDefParse(const char *xmlStr, goto error; break; case VIR_DOMAIN_DEVICE_IOMMU: - if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node))) + if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node, ctxt))) goto error; break; case VIR_DOMAIN_DEVICE_NONE: @@ -18202,7 +18216,7 @@ virDomainDefParseXML(xmlDocPtr xml, }
if (n > 0) { - if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0]))) + if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0], ctxt))) goto error; } VIR_FREE(nodes); @@ -23846,8 +23860,24 @@ static void virDomainIOMMUDefFormat(virBufferPtr buf, virDomainIOMMUDefPtr iommu) { - virBufferAsprintf(buf, "<iommu model='%s'/>\n", + virBuffer childBuf = VIR_BUFFER_INITIALIZER; + + virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2); + + if (iommu->intremap) {
Since intremap is not a boolean or a pointer and while it may seem superfluous ... I'd like to see != VIR_TRISTATE_SWITCH_ABSENT to make it more obvious of the comparison.
+ virBufferAsprintf(&childBuf, "<driver intremap='%s'/>\n", + virTristateSwitchTypeToString(iommu->intremap)); + } + + virBufferAsprintf(buf, "<iommu model='%s'", virDomainIOMMUModelTypeToString(iommu->model)); + if (virBufferUse(&childBuf)) { + virBufferAddLit(buf, ">\n"); + virBufferAddBuffer(buf, &childBuf); + virBufferAddLit(buf, "</iommu>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 762e64e..ad8ae2d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2203,6 +2203,7 @@ typedef enum {
struct _virDomainIOMMUDef { virDomainIOMMUModel model; + virTristateSwitch intremap; }; /* * Guest VM main configuration diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml index 98e4bba..3cef53d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml @@ -24,6 +24,8 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <memballoon model='none'/> - <iommu model='intel'/> + <iommu model='intel'> + <driver intremap='on'/> + </iommu> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml index 98e4bba..3cef53d 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml @@ -24,6 +24,8 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <memballoon model='none'/> - <iommu model='intel'/> + <iommu model='intel'> + <driver intremap='on'/> + </iommu> </devices> </domain>
FWIW: could end up with a merge conflict if you use the link from outdata to argvdata, but at least you don't change in two places. ACK w/ updated docs, John
Do not probe for devices that QEMU does not know when probing for device options. --- src/qemu/qemu_capabilities.c | 99 ++++++++++++++++++++++++++++++-------------- 1 file changed, 68 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 05d0a91..278badf 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1742,71 +1742,103 @@ struct virQEMUCapsObjectTypeProps { const char *type; struct virQEMUCapsStringFlags *props; size_t nprops; + int capsCondition; }; static struct virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { { "virtio-blk-pci", virQEMUCapsObjectPropsVirtioBlk, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBlk) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBlk), + -1 }, { "virtio-net-pci", virQEMUCapsObjectPropsVirtioNet, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioNet) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioNet), + -1 }, { "virtio-scsi-pci", virQEMUCapsObjectPropsVirtioSCSI, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioSCSI) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioSCSI), + -1 }, { "virtio-blk-ccw", virQEMUCapsObjectPropsVirtioBlk, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBlk) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBlk), + -1 }, { "virtio-net-ccw", virQEMUCapsObjectPropsVirtioNet, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioNet) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioNet), + -1 }, { "virtio-scsi-ccw", virQEMUCapsObjectPropsVirtioSCSI, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioSCSI) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioSCSI), + -1 }, { "virtio-blk-s390", virQEMUCapsObjectPropsVirtioBlk, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBlk) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBlk), + -1 }, { "virtio-net-s390", virQEMUCapsObjectPropsVirtioNet, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioNet) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioNet), + -1 }, { "pci-assign", virQEMUCapsObjectPropsPCIAssign, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsPCIAssign) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsPCIAssign), + -1 }, { "kvm-pci-assign", virQEMUCapsObjectPropsPCIAssign, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsPCIAssign) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsPCIAssign), + -1 }, { "vfio-pci", virQEMUCapsObjectPropsVfioPCI, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVfioPCI) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVfioPCI), + -1 }, { "scsi-disk", virQEMUCapsObjectPropsSCSIDisk, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsSCSIDisk) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsSCSIDisk), + -1 }, { "ide-drive", virQEMUCapsObjectPropsIDEDrive, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsIDEDrive) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsIDEDrive), + -1 }, { "PIIX4_PM", virQEMUCapsObjectPropsPiix4PM, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsPiix4PM) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsPiix4PM), + -1 }, { "usb-redir", virQEMUCapsObjectPropsUSBRedir, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsUSBRedir) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsUSBRedir), + -1 }, { "usb-host", virQEMUCapsObjectPropsUSBHost, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsUSBHost) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsUSBHost), + -1 }, { "scsi-generic", virQEMUCapsObjectPropsSCSIGeneric, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsSCSIGeneric) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsSCSIGeneric), + -1 }, { "i440FX-pcihost", virQEMUCapsObjectPropsI440FXPCIHost, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsI440FXPCIHost) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsI440FXPCIHost), + -1 }, { "q35-pcihost", virQEMUCapsObjectPropsQ35PCIHost, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsQ35PCIHost) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsQ35PCIHost), + -1 }, { "usb-storage", virQEMUCapsObjectPropsUSBStorage, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsUSBStorage) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsUSBStorage), + -1 }, { "kvm-pit", virQEMUCapsObjectPropsKVMPit, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsKVMPit) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsKVMPit), + -1 }, { "VGA", virQEMUCapsObjectPropsVGA, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVGA) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVGA), + -1 }, { "vmware-svga", virQEMUCapsObjectPropsVmwareSvga, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVmwareSvga) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVmwareSvga), + -1 }, { "qxl", virQEMUCapsObjectPropsQxl, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsQxl) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsQxl), + -1 }, { "virtio-gpu-pci", virQEMUCapsObjectPropsVirtioGpu, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioGpu) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioGpu), + -1 }, { "virtio-gpu-device", virQEMUCapsObjectPropsVirtioGpu, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioGpu) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioGpu), + -1 }, { "ICH9-LPC", virQEMUCapsObjectPropsICH9, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsICH9) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsICH9), + -1 }, { "virtio-balloon-pci", virQEMUCapsObjectPropsVirtioBalloon, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBalloon) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBalloon), + -1 }, { "virtio-balloon-ccw", virQEMUCapsObjectPropsVirtioBalloon, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBalloon) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBalloon), + -1 }, { "virtio-balloon-device", virQEMUCapsObjectPropsVirtioBalloon, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBalloon) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBalloon), + -1 }, { "nec-usb-xhci", virQEMUCapsObjectPropsUSBNECXHCI, - ARRAY_CARDINALITY(virQEMUCapsObjectPropsUSBNECXHCI) }, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsUSBNECXHCI), + -1 }, }; struct virQEMUCapsPropTypeObjects { @@ -2719,6 +2751,11 @@ virQEMUCapsProbeQMPObjects(virQEMUCapsPtr qemuCaps, for (i = 0; i < ARRAY_CARDINALITY(virQEMUCapsObjectProps); i++) { const char *type = virQEMUCapsObjectProps[i].type; + int cap = virQEMUCapsObjectProps[i].capsCondition; + + if (cap >= 0 && !virQEMUCapsGet(qemuCaps, cap)) + continue; + if ((nvalues = qemuMonitorGetObjectProps(mon, type, &values)) < 0) -- 2.10.2
On 03/23/2017 11:26 AM, Ján Tomko wrote:
Do not probe for devices that QEMU does not know when probing for device options.
No bz reference here
--- src/qemu/qemu_capabilities.c | 99 ++++++++++++++++++++++++++++++-------------- 1 file changed, 68 insertions(+), 31 deletions(-)
ACK John
Introduce a separate buffer for options and use a helper variable for def->iommu. --- src/qemu/qemu_command.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 58af585..c1c7f1a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6625,33 +6625,42 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, const virDomainDef *def, virQEMUCapsPtr qemuCaps) { - if (!def->iommu) + virBuffer opts = VIR_BUFFER_INITIALIZER; + const virDomainIOMMUDef *iommu = def->iommu; + int ret = -1; + + if (!iommu) return 0; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU)) return 0; /* Already handled via -machine */ - switch (def->iommu->model) { + switch (iommu->model) { case VIR_DOMAIN_IOMMU_MODEL_INTEL: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("IOMMU device: '%s' is not supported with " "this QEMU binary"), - virDomainIOMMUModelTypeToString(def->iommu->model)); + virDomainIOMMUModelTypeToString(iommu->model)); return -1; } if (!qemuDomainMachineIsQ35(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("IOMMU device: '%s' is only supported with " "Q35 machines"), - virDomainIOMMUModelTypeToString(def->iommu->model)); + virDomainIOMMUModelTypeToString(iommu->model)); return -1; } - virCommandAddArgList(cmd, "-device", "intel-iommu", NULL); + virBufferAddLit(&opts, "intel-iommu"); case VIR_DOMAIN_IOMMU_MODEL_LAST: break; } - return 0; + virCommandAddArg(cmd, "-device"); + virCommandAddArgBuffer(cmd, &opts); + + ret = 0; + virBufferFreeAndReset(&opts); + return ret; } -- 2.10.2
On 03/23/2017 11:26 AM, Ján Tomko wrote:
Introduce a separate buffer for options and use a helper variable for def->iommu.
No bz reference here
--- src/qemu/qemu_command.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)
ACK
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 | 11 +++++ .../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, 166 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 278badf..e9cc754 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 */ "kernel-irqchip", + "intel-iommu-intremap", ); @@ -1732,6 +1733,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}, @@ -1839,6 +1844,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 b9efab8..16db17f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -400,6 +400,7 @@ typedef enum { /* 250 */ QEMU_CAPS_QUERY_CPU_DEFINITIONS, /* qmp query-cpu-definitions */ 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 c1c7f1a..ddd889d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6652,6 +6652,16 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, return -1; } virBufferAddLit(&opts, "intel-iommu"); + if (iommu->intremap) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_INTREMAP)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu: interrupt remapping is not supported " + "with this QEMU binary")); + goto cleanup; + } + virBufferAsprintf(&opts, ",intremap=%s", + virTristateSwitchTypeToString(iommu->intremap)); + } case VIR_DOMAIN_IOMMU_MODEL_LAST: break; } @@ -6659,6 +6669,7 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, virCommandAddArgBuffer(cmd, &opts); ret = 0; + cleanup: virBufferFreeAndReset(&opts); return ret; } 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 0bc3dcd..5399afe 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -201,6 +201,7 @@ <flag name='drive-iotune-group'/> <flag name='query-cpu-definitions'/> <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 fd0c927..07dbc08 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml @@ -202,6 +202,7 @@ <flag name='drive-iotune-group'/> <flag name='query-cpu-definitions'/> <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 b6fd750..7a25ba8 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 9dcb84e..0798b7d 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -206,6 +206,7 @@ <flag name='pcie-root-port'/> <flag name='query-cpu-definitions'/> <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 ebf9c3e..d704ee4 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 11f6130..2051af9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2478,6 +2478,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 03/23/2017 11:26 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 | 11 +++++ .../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, 166 insertions(+), 44 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 278badf..e9cc754 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 */ "kernel-irqchip", + "intel-iommu-intremap", );
@@ -1732,6 +1733,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}, @@ -1839,6 +1844,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 b9efab8..16db17f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -400,6 +400,7 @@ typedef enum { /* 250 */ QEMU_CAPS_QUERY_CPU_DEFINITIONS, /* qmp query-cpu-definitions */ 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 c1c7f1a..ddd889d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6652,6 +6652,16 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, return -1; } virBufferAddLit(&opts, "intel-iommu"); + if (iommu->intremap) {
Since it's not a pointer, use VIR_TRISTATE_SWITCH_ABSENT Also what if irqchip isn't at least at split, what happens? IOW: Shouldn't there a check for the feature being available before using? What happens if this is "on", but the irqchip isn't set? Thus something that's checking if the feature bit is set and that the irqchip is at least split or on before adding. John
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_INTREMAP)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu: interrupt remapping is not supported " + "with this QEMU binary")); + goto cleanup; + } + virBufferAsprintf(&opts, ",intremap=%s", + virTristateSwitchTypeToString(iommu->intremap)); + } case VIR_DOMAIN_IOMMU_MODEL_LAST: break; } @@ -6659,6 +6669,7 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, virCommandAddArgBuffer(cmd, &opts);
ret = 0; + cleanup: virBufferFreeAndReset(&opts); return ret; }
[...]
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args index ebf9c3e..d704ee4 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 11f6130..2051af9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2478,6 +2478,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);
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 | 23 +++++++++++++++--- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-caching.xml | 28 ++++++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-caching.xml | 28 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 7 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6b196d4..f3c44ae 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7314,6 +7314,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 68f3680..49adf69 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 da0c9f0..6bcf84e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13944,6 +13944,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; @@ -23864,9 +23873,17 @@ virDomainIOMMUDefFormat(virBufferPtr buf, virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2); - if (iommu->intremap) { - virBufferAsprintf(&childBuf, "<driver intremap='%s'/>\n", - virTristateSwitchTypeToString(iommu->intremap)); + if (iommu->intremap || iommu->caching) { + virBufferAddLit(&childBuf, "<driver"); + if (iommu->intremap) { + virBufferAsprintf(&childBuf, " intremap='%s'", + virTristateSwitchTypeToString(iommu->intremap)); + } + if (iommu->caching) { + 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 ad8ae2d..af63b51 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2204,6 +2204,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..b3ce6b4 --- /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</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 100644 index 0000000..b3ce6b4 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-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</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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 8afd830..20add1b 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1126,6 +1126,8 @@ mymain(void) QEMU_CAPS_MACHINE_IOMMU); DO_TEST("intel-iommu-irqchip", QEMU_CAPS_DEVICE_INTEL_IOMMU); + DO_TEST("intel-iommu-caching", + QEMU_CAPS_DEVICE_INTEL_IOMMU); DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE); -- 2.10.2
On 03/23/2017 11:26 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 | 23 +++++++++++++++--- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-caching.xml | 28 ++++++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-caching.xml | 28 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 7 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6b196d4..f3c44ae 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7314,6 +7314,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>
Similar comment/concert regarding irqchip setting as from patch4
</dl> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 68f3680..49adf69 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 da0c9f0..6bcf84e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13944,6 +13944,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;
@@ -23864,9 +23873,17 @@ virDomainIOMMUDefFormat(virBufferPtr buf,
virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
- if (iommu->intremap) { - virBufferAsprintf(&childBuf, "<driver intremap='%s'/>\n", - virTristateSwitchTypeToString(iommu->intremap)); + if (iommu->intremap || iommu->caching) {
Same comment regarding bool/pointer... Not that it really matters technically, but it would seem to be a more proper comparison.
+ virBufferAddLit(&childBuf, "<driver"); + if (iommu->intremap) { + virBufferAsprintf(&childBuf, " intremap='%s'", + virTristateSwitchTypeToString(iommu->intremap)); + } + if (iommu->caching) { + 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 ad8ae2d..af63b51 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2204,6 +2204,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..b3ce6b4 --- /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</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'/>
Does it matter if intremap is on or off? Since this is a different test, you could go with off or even better not present to test the logic to add just caching would work...
+ </iommu> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml new file mode 100644 index 0000000..b3ce6b4
Similar comment from patch1 regarding this being a link. John
--- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-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</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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 8afd830..20add1b 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1126,6 +1126,8 @@ mymain(void) QEMU_CAPS_MACHINE_IOMMU); DO_TEST("intel-iommu-irqchip", QEMU_CAPS_DEVICE_INTEL_IOMMU); + DO_TEST("intel-iommu-caching", + QEMU_CAPS_DEVICE_INTEL_IOMMU);
DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE);
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 | 10 ++++++++++ tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-intel-iommu-caching.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 5 +++++ 6 files changed, 38 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 e9cc754..6a884d1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -365,6 +365,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-cpu-definitions", /* 250 */ "kernel-irqchip", "intel-iommu-intremap", + "intel-iommu-caching", ); @@ -1735,6 +1736,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 16db17f..8ab2ee0 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_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 ddd889d..57d4408 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6662,6 +6662,16 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, virBufferAsprintf(&opts, ",intremap=%s", virTristateSwitchTypeToString(iommu->intremap)); } + if (iommu->caching) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_CACHING)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu: caching mode is not supported " + "with this QEMU binary")); + goto cleanup; + } + 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 0798b7d..533bbcc 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='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..59cb8a1 --- /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 \ +-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 2051af9..44eed34 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2480,6 +2480,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 03/23/2017 11:26 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 | 10 ++++++++++ tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-intel-iommu-caching.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 5 +++++ 6 files changed, 38 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 e9cc754..6a884d1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -365,6 +365,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-cpu-definitions", /* 250 */ "kernel-irqchip", "intel-iommu-intremap", + "intel-iommu-caching", );
Once merge conflicts are resolved, you'll find this needs the /* 255 */
@@ -1735,6 +1736,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 16db17f..8ab2ee0 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_MACHINE_KERNEL_IRQCHIP, /* -machine kernel_irqchip */ QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */ + QEMU_CAPS_INTEL_IOMMU_CACHING, /* intel-iommu.caching-mode */
Likewise, a /* 255 */ comment before this next grouping.
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 ddd889d..57d4408 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6662,6 +6662,16 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, virBufferAsprintf(&opts, ",intremap=%s", virTristateSwitchTypeToString(iommu->intremap)); } + if (iommu->caching) {
Similar to previous - w/r/t VIR_TRISTATE_SWITCH_ABSENT and presence and setting of the irqchip value. John
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_CACHING)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu: caching mode is not supported " + "with this QEMU binary")); + goto cleanup; + } + 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 0798b7d..533bbcc 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='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..59cb8a1 --- /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 \ +-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 2051af9..44eed34 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2480,6 +2480,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