[libvirt] [PATCHv3 0/6] Support more options for intel-iommu
[Adding Peter Xu to cc to see if he has any opinion on the docs] https://bugzilla.redhat.com/show_bug.cgi?id=1427005 v1: https://www.redhat.com/archives/libvir-list/2017-March/msg01072.html v2: https://www.redhat.com/archives/libvir-list/2017-April/msg00932.html new in v3: * add more devices to qemuxml2argv-intel-iommu-caching.xml to check the ordering (intel-iommu should come before other devices) * fix the capability clustering Ján Tomko (6): conf: add <irqchip mode> to <features> qemu: format kernel_irqchip on the command line conf: add <driver intremap> to <iommu> qemu: format intremap= on intel-iommu command line conf: add caching attribute to iommu device qemu: format caching-mode on iommu command line docs/formatdomain.html.in | 41 +++++++++- docs/schemas/domaincommon.rng | 30 ++++++++ src/conf/domain_conf.c | 90 ++++++++++++++++++++-- src/conf/domain_conf.h | 14 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 13 ++++ src/qemu/qemu_capabilities.h | 5 ++ src/qemu/qemu_command.c | 40 ++++++++++ tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 22 ++++-- tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.5.0.x86_64.replies | 24 ++++-- tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + .../qemucapabilitiesdata/caps_2.6.0.x86_64.replies | 24 ++++-- tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_2.7.0.x86_64.replies | 28 +++++-- tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 2 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + .../qemucapabilitiesdata/caps_2.8.0.x86_64.replies | 37 +++++++-- tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 2 + .../qemucapabilitiesdata/caps_2.9.0.x86_64.replies | 49 +++++++++--- tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 3 + .../qemuxml2argv-intel-iommu-caching.args | 25 ++++++ .../qemuxml2argv-intel-iommu-caching.xml | 50 ++++++++++++ .../qemuxml2argv-intel-iommu-irqchip.args | 19 +++++ .../qemuxml2argv-intel-iommu-irqchip.xml | 31 ++++++++ tests/qemuxml2argvtest.c | 16 ++++ .../qemuxml2xmlout-intel-iommu-caching.xml | 1 + .../qemuxml2xmlout-intel-iommu-irqchip.xml | 1 + tests/qemuxml2xmltest.c | 2 + 37 files changed, 533 insertions(+), 49 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml -- 2.10.2
Add a new <irqchip> element with a mode attribute. Possible values are off, split or on. https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 10 +++++++ docs/schemas/domaincommon.rng | 16 ++++++++++ src/conf/domain_conf.c | 34 +++++++++++++++++++++- src/conf/domain_conf.h | 12 ++++++++ .../qemuxml2argv-intel-iommu-irqchip.xml | 29 ++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-irqchip.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e31a271..4b92a0f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1643,6 +1643,7 @@ </kvm> <pvspinlock state='on'/> <gic version='2'/> + <irqchip mode='split'/> </features> ...</pre> @@ -1804,6 +1805,15 @@ for hypervisor to decide. <span class="since">Since 2.1.0</span> </dd> + <dt><code>irqchip</code></dt> + <dd>Tune the in-kernel irqchip. Possible values for the + <code>mode</code> attribute are: + <code>on</code>, <code>split</code> and <code>off</code>. + <code>split</code> is useful for using interrupt remapping + with the <a href="#elementsIommu">IOMMU device</a>. + The default is left for hypervisor to decide. + <span class="since">Since 3.3.0</span> (QEMU only) + </dd> </dl> <h3><a name="elementsTime">Time keeping</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index eb4b0f7..7772a91 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4514,6 +4514,9 @@ </optional> </element> </optional> + <optional> + <ref name="irqchip"/> + </optional> </interleave> </element> </optional> @@ -4689,6 +4692,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 fe9b7c7..73fa268 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -140,7 +140,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "pmu", "vmport", "gic", - "smm") + "smm", + "irqchip") VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST, "default", @@ -857,6 +858,12 @@ VIR_ENUM_IMPL(virDomainLoader, "rom", "pflash") +VIR_ENUM_IMPL(virDomainIRQChip, + VIR_DOMAIN_IRQCHIP_LAST, + "off", + "split", + "on") + /* Internal mapping: subset of block job types that can be present in * <mirror> XML (remaining types are not two-phase). */ VIR_ENUM_DECL(virDomainBlockJob) @@ -17521,6 +17528,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; @@ -24601,6 +24626,13 @@ virDomainDefFormatInternal(virDomainDefPtr def, } break; + case VIR_DOMAIN_FEATURE_IRQCHIP: + if (def->features[i] == VIR_TRISTATE_SWITCH_ON) { + virBufferAsprintf(buf, "<irqchip mode='%s'/>\n", + virDomainIRQChipTypeToString(def->irqchip)); + } + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_FEATURE_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3b6b174..61af012 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1673,6 +1673,7 @@ typedef enum { VIR_DOMAIN_FEATURE_VMPORT, VIR_DOMAIN_FEATURE_GIC, VIR_DOMAIN_FEATURE_SMM, + VIR_DOMAIN_FEATURE_IRQCHIP, VIR_DOMAIN_FEATURE_LAST } virDomainFeature; @@ -1812,6 +1813,16 @@ struct _virDomainLoaderDef { void virDomainLoaderDefFree(virDomainLoaderDefPtr loader); +typedef enum { + VIR_DOMAIN_IRQCHIP_OFF = 0, + VIR_DOMAIN_IRQCHIP_SPLIT, + VIR_DOMAIN_IRQCHIP_ON, + + VIR_DOMAIN_IRQCHIP_LAST +} virDomainIRQChip; + +VIR_ENUM_DECL(virDomainIRQChip); + /* Operating system configuration data & machine / arch */ typedef struct _virDomainOSDef virDomainOSDef; typedef virDomainOSDef *virDomainOSDefPtr; @@ -2261,6 +2272,7 @@ struct _virDomainDef { unsigned int hyperv_spinlocks; virGICVersion gic_version; char *hyperv_vendor_id; + virDomainIRQChip irqchip; /* These options are of type virTristateSwitch: ON = keep, OFF = drop */ int caps_features[VIR_DOMAIN_CAPS_FEATURE_LAST]; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml new file mode 100644 index 0000000..cc895af --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <irqchip mode='split'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + <iommu model='intel'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml new file mode 120000 index 0000000..58a0199 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 2dccde7..40425f5 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1122,6 +1122,7 @@ mymain(void) DO_TEST("intel-iommu-machine", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); + DO_TEST("intel-iommu-irqchip", NONE); DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE); -- 2.10.2
On Wed, Apr 26, 2017 at 10:29:02AM +0200, Ján Tomko wrote:
Add a new <irqchip> element with a mode attribute.
Possible values are off, split or on.
Hi, Ján, Thanks you for cced me. One tiny comment on irqchip mode... Here could user specify irqchip=off from libvirt side? IIUC that might be dangerous since userspace APIC should only be for debugging purpose (when kernel-irqchip=off, we'll be using userspace APIC, not kernel one, and iirc Paolo mentioned known bugs in userspace APIC). So imho we'd better not allow user to use "off", but only "on" and "split". CCing Paolo here in case he has any comment on this. Thanks,
https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 10 +++++++ docs/schemas/domaincommon.rng | 16 ++++++++++ src/conf/domain_conf.c | 34 +++++++++++++++++++++- src/conf/domain_conf.h | 12 ++++++++ .../qemuxml2argv-intel-iommu-irqchip.xml | 29 ++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-irqchip.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e31a271..4b92a0f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1643,6 +1643,7 @@ </kvm> <pvspinlock state='on'/> <gic version='2'/> + <irqchip mode='split'/>
</features> ...</pre> @@ -1804,6 +1805,15 @@ for hypervisor to decide. <span class="since">Since 2.1.0</span> </dd> + <dt><code>irqchip</code></dt> + <dd>Tune the in-kernel irqchip. Possible values for the + <code>mode</code> attribute are: + <code>on</code>, <code>split</code> and <code>off</code>. + <code>split</code> is useful for using interrupt remapping + with the <a href="#elementsIommu">IOMMU device</a>. + The default is left for hypervisor to decide. + <span class="since">Since 3.3.0</span> (QEMU only) + </dd> </dl>
<h3><a name="elementsTime">Time keeping</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index eb4b0f7..7772a91 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4514,6 +4514,9 @@ </optional> </element> </optional> + <optional> + <ref name="irqchip"/> + </optional> </interleave> </element> </optional> @@ -4689,6 +4692,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 fe9b7c7..73fa268 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -140,7 +140,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "pmu", "vmport", "gic", - "smm") + "smm", + "irqchip")
VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST, "default", @@ -857,6 +858,12 @@ VIR_ENUM_IMPL(virDomainLoader, "rom", "pflash")
+VIR_ENUM_IMPL(virDomainIRQChip, + VIR_DOMAIN_IRQCHIP_LAST, + "off", + "split", + "on") + /* Internal mapping: subset of block job types that can be present in * <mirror> XML (remaining types are not two-phase). */ VIR_ENUM_DECL(virDomainBlockJob) @@ -17521,6 +17528,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; @@ -24601,6 +24626,13 @@ virDomainDefFormatInternal(virDomainDefPtr def, } break;
+ case VIR_DOMAIN_FEATURE_IRQCHIP: + if (def->features[i] == VIR_TRISTATE_SWITCH_ON) { + virBufferAsprintf(buf, "<irqchip mode='%s'/>\n", + virDomainIRQChipTypeToString(def->irqchip)); + } + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_FEATURE_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3b6b174..61af012 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1673,6 +1673,7 @@ typedef enum { VIR_DOMAIN_FEATURE_VMPORT, VIR_DOMAIN_FEATURE_GIC, VIR_DOMAIN_FEATURE_SMM, + VIR_DOMAIN_FEATURE_IRQCHIP,
VIR_DOMAIN_FEATURE_LAST } virDomainFeature; @@ -1812,6 +1813,16 @@ struct _virDomainLoaderDef {
void virDomainLoaderDefFree(virDomainLoaderDefPtr loader);
+typedef enum { + VIR_DOMAIN_IRQCHIP_OFF = 0, + VIR_DOMAIN_IRQCHIP_SPLIT, + VIR_DOMAIN_IRQCHIP_ON, + + VIR_DOMAIN_IRQCHIP_LAST +} virDomainIRQChip; + +VIR_ENUM_DECL(virDomainIRQChip); + /* Operating system configuration data & machine / arch */ typedef struct _virDomainOSDef virDomainOSDef; typedef virDomainOSDef *virDomainOSDefPtr; @@ -2261,6 +2272,7 @@ struct _virDomainDef { unsigned int hyperv_spinlocks; virGICVersion gic_version; char *hyperv_vendor_id; + virDomainIRQChip irqchip;
/* These options are of type virTristateSwitch: ON = keep, OFF = drop */ int caps_features[VIR_DOMAIN_CAPS_FEATURE_LAST]; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml new file mode 100644 index 0000000..cc895af --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <irqchip mode='split'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + <iommu model='intel'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml new file mode 120000 index 0000000..58a0199 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 2dccde7..40425f5 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1122,6 +1122,7 @@ mymain(void) DO_TEST("intel-iommu-machine", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); + DO_TEST("intel-iommu-irqchip", NONE);
DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE); -- 2.10.2
-- Peter Xu
On 26/04/2017 11:36, Peter Xu wrote:
Possible values are off, split or on. Hi, Ján,
Thanks you for cced me. One tiny comment on irqchip mode...
Here could user specify irqchip=off from libvirt side? IIUC that might be dangerous since userspace APIC should only be for debugging purpose (when kernel-irqchip=off, we'll be using userspace APIC, not kernel one, and iirc Paolo mentioned known bugs in userspace APIC). So imho we'd better not allow user to use "off", but only "on" and "split".
CCing Paolo here in case he has any comment on this.
Allowing "off" may be okay as long as it taints the domain. In the end, irqchip mode is a feature of the APIC/IOAPIC/PIC/PIT, and even more specifically: - for kernel-irqchip=off, the distinguishing feature is in-kernel vs. userspace APIC. - for kernel-irqchip=split, what we really care about is userspace IOAPIC. So perhaps another choice for the XML is: <features> <apic driver='qemu|kvm'/> <ioapic driver='qemu|kvm'/> </features> where (apic:qemu,ioapic:kvm) is invalid? Paolo
On Wed, Apr 26, 2017 at 04:19:07PM +0200, Paolo Bonzini wrote:
On 26/04/2017 11:36, Peter Xu wrote:
Possible values are off, split or on. Hi, Ján,
Thanks you for cced me. One tiny comment on irqchip mode...
Here could user specify irqchip=off from libvirt side? IIUC that might be dangerous since userspace APIC should only be for debugging purpose (when kernel-irqchip=off, we'll be using userspace APIC, not kernel one, and iirc Paolo mentioned known bugs in userspace APIC). So imho we'd better not allow user to use "off", but only "on" and "split".
CCing Paolo here in case he has any comment on this.
Allowing "off" may be okay as long as it taints the domain.
If it's only useful for debugging, I can drop "off" from the XML completely.
In the end, irqchip mode is a feature of the APIC/IOAPIC/PIC/PIT, and even more specifically:
- for kernel-irqchip=off, the distinguishing feature is in-kernel vs. userspace APIC.
- for kernel-irqchip=split, what we really care about is userspace IOAPIC.
So perhaps another choice for the XML is:
<features> <apic driver='qemu|kvm'/> <ioapic driver='qemu|kvm'/> </features>
where (apic:qemu,ioapic:kvm) is invalid?
And if apic:qemu,ioapic:qemu is only for debugging, the <apic> element can be dropped too. 'ioapic' is also more specific (so if kernel-irqchip starts to be useful on other archs, libvirt might need a new element?). I'm considering sticking the attribute to the already-existing apic element: <apic mode='kernel|split'/> But I don't really like any of the options. Jan
On 27/04/2017 16:07, Ján Tomko wrote:
And if apic:qemu,ioapic:qemu is only for debugging, the <apic> element can be dropped too.
'ioapic' is also more specific (so if kernel-irqchip starts to be useful on other archs, libvirt might need a new element?).
Yeah, using architecture-specific element names brings multiplication of nodes, but on the other hand it makes little sense to do it otherwise. And ARM already has <gic> KVM adopted the "irqchip" moniker, but I don't really like it because there are many irqchips in a modern computer (usually two layers, with one irqchip per CPU and one distributing IRQs to the CPUs): x86 has APIC and IOAPIC (even without considering legacy), PPC has XICS and XICP, ARM has the GIC and the distributor, and so on. For what it's worth, KVM on Raspberry Pi (when it happens) will need "-machine kernel_irqchip=off", so <gic version='...' driver='qemu'/>.
I'm considering sticking the attribute to the already-existing apic element: <apic mode='kernel|split'/>
Not really, because with "<apic mode='split'>" the APIC would be in the kernel. :) Paolo
On 04/26/2017 04:29 AM, Ján Tomko wrote:
Add a new <irqchip> element with a mode attribute.
Possible values are off, split or on.
https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 10 +++++++ docs/schemas/domaincommon.rng | 16 ++++++++++ src/conf/domain_conf.c | 34 +++++++++++++++++++++- src/conf/domain_conf.h | 12 ++++++++ .../qemuxml2argv-intel-iommu-irqchip.xml | 29 ++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-irqchip.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e31a271..4b92a0f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1643,6 +1643,7 @@ </kvm> <pvspinlock state='on'/> <gic version='2'/> + <irqchip mode='split'/>
</features> ...</pre> @@ -1804,6 +1805,15 @@ for hypervisor to decide. <span class="since">Since 2.1.0</span> </dd> + <dt><code>irqchip</code></dt> + <dd>Tune the in-kernel irqchip. Possible values for the + <code>mode</code> attribute are: + <code>on</code>, <code>split</code> and <code>off</code>. + <code>split</code> is useful for using interrupt remapping + with the <a href="#elementsIommu">IOMMU device</a>. + The default is left for hypervisor to decide. + <span class="since">Since 3.3.0</span> (QEMU only) + </dd> </dl>
You asked in your response to my comments from the v2 posting "What would you say?" Well, that's what I was expecting someone to tell me. If I'm reading the documentation and see this, I might wonder whether I need this and what it's used for. What does on, split, and off actually do. Reading literally just what you have here makes it seem more related to IOMMU. Hence why I asked. I'm certainly "hardware terminology wording challenged" - glad that Peter/Paolo provided some insights. After thinking about it a bit, I think no matter what we end up calling it as long as we can inform the reader/consumer of our documentation that this relates to the QEMU 'kernel_irqchip' setting which is an architecture and machine model specific setting to determine where to handle interrupts. (that's about as generic as comes to mind). That should give enough context to have the curious user to search on "kernel_irqchip"... I understand there's concern about being too specific, but since you're indicating this is a QEMU only feature - I think some QEMU specifics could be added. What's still not clear to me is how this setting affects IOMMU. I'm "assuming" that "up to this point" there isn't an impact. But I do see code that specifically checks if interrupt remapping is enabled, then irqchip must be off or split. Something that could be added with the docs when patch 5/6 are added. Hopefully this helps. I'm fine with the code. If you change the name of the feature to attempt to be more generic - that's fine. John
<h3><a name="elementsTime">Time keeping</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index eb4b0f7..7772a91 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4514,6 +4514,9 @@ </optional> </element> </optional> + <optional> + <ref name="irqchip"/> + </optional> </interleave> </element> </optional> @@ -4689,6 +4692,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 fe9b7c7..73fa268 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -140,7 +140,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "pmu", "vmport", "gic", - "smm") + "smm", + "irqchip")
VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST, "default", @@ -857,6 +858,12 @@ VIR_ENUM_IMPL(virDomainLoader, "rom", "pflash")
+VIR_ENUM_IMPL(virDomainIRQChip, + VIR_DOMAIN_IRQCHIP_LAST, + "off", + "split", + "on") + /* Internal mapping: subset of block job types that can be present in * <mirror> XML (remaining types are not two-phase). */ VIR_ENUM_DECL(virDomainBlockJob) @@ -17521,6 +17528,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; @@ -24601,6 +24626,13 @@ virDomainDefFormatInternal(virDomainDefPtr def, } break;
+ case VIR_DOMAIN_FEATURE_IRQCHIP: + if (def->features[i] == VIR_TRISTATE_SWITCH_ON) { + virBufferAsprintf(buf, "<irqchip mode='%s'/>\n", + virDomainIRQChipTypeToString(def->irqchip)); + } + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_FEATURE_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3b6b174..61af012 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1673,6 +1673,7 @@ typedef enum { VIR_DOMAIN_FEATURE_VMPORT, VIR_DOMAIN_FEATURE_GIC, VIR_DOMAIN_FEATURE_SMM, + VIR_DOMAIN_FEATURE_IRQCHIP,
VIR_DOMAIN_FEATURE_LAST } virDomainFeature; @@ -1812,6 +1813,16 @@ struct _virDomainLoaderDef {
void virDomainLoaderDefFree(virDomainLoaderDefPtr loader);
+typedef enum { + VIR_DOMAIN_IRQCHIP_OFF = 0, + VIR_DOMAIN_IRQCHIP_SPLIT, + VIR_DOMAIN_IRQCHIP_ON, + + VIR_DOMAIN_IRQCHIP_LAST +} virDomainIRQChip; + +VIR_ENUM_DECL(virDomainIRQChip); + /* Operating system configuration data & machine / arch */ typedef struct _virDomainOSDef virDomainOSDef; typedef virDomainOSDef *virDomainOSDefPtr; @@ -2261,6 +2272,7 @@ struct _virDomainDef { unsigned int hyperv_spinlocks; virGICVersion gic_version; char *hyperv_vendor_id; + virDomainIRQChip irqchip;
/* These options are of type virTristateSwitch: ON = keep, OFF = drop */ int caps_features[VIR_DOMAIN_CAPS_FEATURE_LAST]; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml new file mode 100644 index 0000000..cc895af --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <irqchip mode='split'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + <iommu model='intel'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml new file mode 120000 index 0000000..58a0199 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-irqchip.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 2dccde7..40425f5 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1122,6 +1122,7 @@ mymain(void) DO_TEST("intel-iommu-machine", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); + DO_TEST("intel-iommu-irqchip", NONE);
DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE);
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 83e979a..30571e8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -381,6 +381,7 @@ virDomainIOThreadIDAdd; virDomainIOThreadIDDefFree; virDomainIOThreadIDDel; virDomainIOThreadIDFind; +virDomainIRQChipTypeToString; virDomainKeyWrapCipherNameTypeFromString; virDomainKeyWrapCipherNameTypeToString; virDomainLeaseDefFree; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cc3e1f8..72870b9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -364,6 +364,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-cpu-definitions", /* 250 */ "block-write-threshold", "query-named-block-nodes", + "kernel-irqchip", ); @@ -3124,6 +3125,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "drive", "throttling.bps-total-max-length", QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH }, { "drive", "throttling.group", QEMU_CAPS_DRIVE_IOTUNE_GROUP }, { "spice", "rendernode", QEMU_CAPS_SPICE_RENDERNODE }, + { "machine", "kernel_irqchip", QEMU_CAPS_MACHINE_KERNEL_IRQCHIP }, }; static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index a7eec52..362347e 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -401,6 +401,7 @@ typedef enum { QEMU_CAPS_QUERY_CPU_DEFINITIONS, /* qmp query-cpu-definitions */ QEMU_CAPS_BLOCK_WRITE_THRESHOLD, /* BLOCK_WRITE_THRESHOLD event */ QEMU_CAPS_QUERY_NAMED_BLOCK_NODES, /* qmp query-named-block-nodes */ + QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, /* -machine kernel_irqchip */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1116d2c..c04bf83 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7379,6 +7379,17 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, } } + if (def->features[VIR_DOMAIN_FEATURE_IRQCHIP] == VIR_TRISTATE_SWITCH_ON) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("kernel-irqchip option is not supported by this " + "QEMU binary")); + goto cleanup; + } + virBufferAsprintf(&buf, ",kernel_irqchip=%s", + virDomainIRQChipTypeToString(def->irqchip)); + } + virCommandAddArgBuffer(cmd, &buf); } diff --git a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml index a68c13b..14f34b2 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml @@ -140,6 +140,7 @@ <flag name='display'/> <flag name='vhost-scsi'/> <flag name='query-cpu-definitions'/> + <flag name='kernel-irqchip'/> <version>1005003</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml index 365b3a6..8fc23d6 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml @@ -145,6 +145,7 @@ <flag name='display'/> <flag name='vhost-scsi'/> <flag name='query-cpu-definitions'/> + <flag name='kernel-irqchip'/> <version>1006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml index 689fbf8..47c8956 100644 --- a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml @@ -147,6 +147,7 @@ <flag name='display'/> <flag name='vhost-scsi'/> <flag name='query-cpu-definitions'/> + <flag name='kernel-irqchip'/> <version>1007000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml index e092dd2..afe7d53 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml @@ -163,6 +163,7 @@ <flag name='vhost-scsi'/> <flag name='query-cpu-definitions'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2001001</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml index ea03f2e..5bdc1a2 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -185,6 +185,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2004000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml index 164605f..36bc134 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -191,6 +191,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2005000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml index af3a8e7..46e77b7 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml @@ -168,6 +168,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml index 4402ffa..0c15b92 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml @@ -168,6 +168,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml index 3f05169..c2ee94d 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml @@ -162,6 +162,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index 1d823ea..0583d6a 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -200,6 +200,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml index 38d36b3..49baca8 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml @@ -130,6 +130,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index 24d7cb4..bee3ce0 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -202,6 +202,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml index 42c92c5..4fc20df 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -132,6 +132,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2007093</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml index 0bc1368..4e00816 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml @@ -203,6 +203,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2008000</version> <kvmVersion>0</kvmVersion> <package> (v2.8.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 6386c4e..1ca4df9 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -207,6 +207,7 @@ <flag name='query-cpu-definitions'/> <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> + <flag name='kernel-irqchip'/> <version>2008090</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0-rc0-142-g940a8ce)</package> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args new file mode 100644 index 0000000..899d240 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args @@ -0,0 +1,19 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-machine q35,accel=tcg,kernel_irqchip=split \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device intel-iommu diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c1b014b..80f10cc 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2492,6 +2492,10 @@ mymain(void) DO_TEST("intel-iommu-machine", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); + DO_TEST("intel-iommu-irqchip", + QEMU_CAPS_MACHINE_OPT, + QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, + QEMU_CAPS_DEVICE_INTEL_IOMMU); DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS); -- 2.10.2
Add a new attribute to control interrupt remapping. https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 22 ++++++++++++- docs/schemas/domaincommon.rng | 9 +++++ src/conf/domain_conf.c | 38 +++++++++++++++++++--- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-irqchip.xml | 4 ++- 5 files changed, 68 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 4b92a0f..e1a4625 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7362,7 +7362,9 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> - <iommu model='intel'/> + <iommu model='intel'> + <driver intremap='on'/> + </iommu> </devices> ... </pre> @@ -7373,6 +7375,24 @@ qemu-kvm -net nic,model=? /dev/null Currently only the <code>intel</code> model is supported. </p> </dd> + <dt><code>driver</code></dt> + <dd> + <p> + The <code>driver</code> subelement can be used to configure + additional options: + </p> + <dl> + <dt><code>intremap</code></dt> + <dd> + <p> + The <code>intremap</code> attribute with possible values + <code>on</code> and <code>off</code> can be used to + turn on interrupt remapping. <span class="since">Since 3.3.0</span> + (QEMU only) + </p> + </dd> + </dl> + </dd> </dl> <h3><a name="seclabel">Security label</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7772a91..cc10209 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3892,6 +3892,15 @@ <attribute name="model"> <value>intel</value> </attribute> + <optional> + <element name="driver"> + <optional> + <attribute name="intremap"> + <ref name="virOnOff"/> + </attribute> + </optional> + </element> + </optional> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 73fa268..650d206 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14136,12 +14136,16 @@ virDomainMemoryDefParseXML(xmlNodePtr memdevNode, static virDomainIOMMUDefPtr -virDomainIOMMUDefParseXML(xmlNodePtr node) +virDomainIOMMUDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt) { virDomainIOMMUDefPtr iommu = NULL, ret = NULL; + xmlNodePtr save = ctxt->node; char *tmp = NULL; int val; + ctxt->node = node; + if (VIR_ALLOC(iommu) < 0) goto cleanup; @@ -14158,10 +14162,20 @@ virDomainIOMMUDefParseXML(xmlNodePtr node) iommu->model = val; + VIR_FREE(tmp); + if ((tmp = virXPathString("string(./driver/@intremap)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unknown intremap value: %s"), tmp); + goto cleanup; + } + iommu->intremap = val; + } + ret = iommu; iommu = NULL; cleanup: + ctxt->node = save; VIR_FREE(iommu); VIR_FREE(tmp); return ret; @@ -14314,7 +14328,7 @@ virDomainDeviceDefParse(const char *xmlStr, goto error; break; case VIR_DOMAIN_DEVICE_IOMMU: - if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node))) + if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node, ctxt))) goto error; break; case VIR_DOMAIN_DEVICE_NONE: @@ -18444,7 +18458,7 @@ virDomainDefParseXML(xmlDocPtr xml, } if (n > 0) { - if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0]))) + if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0], ctxt))) goto error; } VIR_FREE(nodes); @@ -24100,8 +24114,24 @@ static void virDomainIOMMUDefFormat(virBufferPtr buf, const virDomainIOMMUDef *iommu) { - virBufferAsprintf(buf, "<iommu model='%s'/>\n", + virBuffer childBuf = VIR_BUFFER_INITIALIZER; + + virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2); + + if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&childBuf, "<driver intremap='%s'/>\n", + virTristateSwitchTypeToString(iommu->intremap)); + } + + virBufferAsprintf(buf, "<iommu model='%s'", virDomainIOMMUModelTypeToString(iommu->model)); + if (virBufferUse(&childBuf)) { + virBufferAddLit(buf, ">\n"); + virBufferAddBuffer(buf, &childBuf); + virBufferAddLit(buf, "</iommu>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 61af012..61f18cf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2213,6 +2213,7 @@ typedef enum { struct _virDomainIOMMUDef { virDomainIOMMUModel model; + virTristateSwitch intremap; }; /* * Guest VM main configuration diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml index cc895af..2100c08 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml @@ -24,6 +24,8 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <memballoon model='none'/> - <iommu model='intel'/> + <iommu model='intel'> + <driver intremap='on'/> + </iommu> </devices> </domain> -- 2.10.2
On 04/26/2017 04:29 AM, Ján Tomko wrote:
Add a new attribute to control interrupt remapping.
https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 22 ++++++++++++- docs/schemas/domaincommon.rng | 9 +++++ src/conf/domain_conf.c | 38 +++++++++++++++++++--- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-irqchip.xml | 4 ++- 5 files changed, 68 insertions(+), 6 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 4b92a0f..e1a4625 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7362,7 +7362,9 @@ qemu-kvm -net nic,model=? /dev/null <pre> ... <devices> - <iommu model='intel'/> + <iommu model='intel'> + <driver intremap='on'/> + </iommu> </devices> ... </pre> @@ -7373,6 +7375,24 @@ qemu-kvm -net nic,model=? /dev/null Currently only the <code>intel</code> model is supported. </p> </dd> + <dt><code>driver</code></dt> + <dd> + <p> + The <code>driver</code> subelement can be used to configure + additional options: + </p> + <dl> + <dt><code>intremap</code></dt> + <dd> + <p> + The <code>intremap</code> attribute with possible values + <code>on</code> and <code>off</code> can be used to + turn on interrupt remapping. <span class="since">Since 3.3.0</span> + (QEMU only)
D'oh - brain cramp obviously w/r/t thinking in my response to patch 1 that patch 5/6 enabled intremap. In any case, this is where I'd expect the "clarification" in the kernel_irqchip explanation to indicate that when using using intremap that kernel_irqchip must be "off" or "split". When I reviewed v2 it really wasn't clear whether the irqchip setting *had* to be enabled or not. I'm fine with letting the qemu startup fail if the user misconfigures using "intremap=on" and "irqchip=on", although it's also possible to make that check in post processing. It's about where it's desired to have the error come from. Don't allow the configuration or allow it and know that it'll fail. I disagree with your assertion that describing what this parameter does at a very high level is out of scope. As you point out it's related to the VT-d technology - something that could be listed/described in some way here. It doesn't need to be detailed, just enough to allow someone to know what the setting is related to. In this case, VT-D John
+ </p> + </dd> + </dl> + </dd> </dl>
<h3><a name="seclabel">Security label</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7772a91..cc10209 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3892,6 +3892,15 @@ <attribute name="model"> <value>intel</value> </attribute> + <optional> + <element name="driver"> + <optional> + <attribute name="intremap"> + <ref name="virOnOff"/> + </attribute> + </optional> + </element> + </optional> </element> </define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 73fa268..650d206 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14136,12 +14136,16 @@ virDomainMemoryDefParseXML(xmlNodePtr memdevNode,
static virDomainIOMMUDefPtr -virDomainIOMMUDefParseXML(xmlNodePtr node) +virDomainIOMMUDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt) { virDomainIOMMUDefPtr iommu = NULL, ret = NULL; + xmlNodePtr save = ctxt->node; char *tmp = NULL; int val;
+ ctxt->node = node; + if (VIR_ALLOC(iommu) < 0) goto cleanup;
@@ -14158,10 +14162,20 @@ virDomainIOMMUDefParseXML(xmlNodePtr node)
iommu->model = val;
+ VIR_FREE(tmp); + if ((tmp = virXPathString("string(./driver/@intremap)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unknown intremap value: %s"), tmp); + goto cleanup; + } + iommu->intremap = val; + } + ret = iommu; iommu = NULL;
cleanup: + ctxt->node = save; VIR_FREE(iommu); VIR_FREE(tmp); return ret; @@ -14314,7 +14328,7 @@ virDomainDeviceDefParse(const char *xmlStr, goto error; break; case VIR_DOMAIN_DEVICE_IOMMU: - if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node))) + if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node, ctxt))) goto error; break; case VIR_DOMAIN_DEVICE_NONE: @@ -18444,7 +18458,7 @@ virDomainDefParseXML(xmlDocPtr xml, }
if (n > 0) { - if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0]))) + if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0], ctxt))) goto error; } VIR_FREE(nodes); @@ -24100,8 +24114,24 @@ static void virDomainIOMMUDefFormat(virBufferPtr buf, const virDomainIOMMUDef *iommu) { - virBufferAsprintf(buf, "<iommu model='%s'/>\n", + virBuffer childBuf = VIR_BUFFER_INITIALIZER; + + virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2); + + if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&childBuf, "<driver intremap='%s'/>\n", + virTristateSwitchTypeToString(iommu->intremap)); + } + + virBufferAsprintf(buf, "<iommu model='%s'", virDomainIOMMUModelTypeToString(iommu->model)); + if (virBufferUse(&childBuf)) { + virBufferAddLit(buf, ">\n"); + virBufferAddBuffer(buf, &childBuf); + virBufferAddLit(buf, "</iommu>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 61af012..61f18cf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2213,6 +2213,7 @@ typedef enum {
struct _virDomainIOMMUDef { virDomainIOMMUModel model; + virTristateSwitch intremap; }; /* * Guest VM main configuration diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml index cc895af..2100c08 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.xml @@ -24,6 +24,8 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <memballoon model='none'/> - <iommu model='intel'/> + <iommu model='intel'> + <driver intremap='on'/> + </iommu> </devices> </domain>
Add the intremap= option to QEMU command line, corresponding to the <driver intremap> attribute of the iommu device. https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- src/qemu/qemu_capabilities.c | 8 ++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 18 ++++++++ .../qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 22 +++++++--- .../qemucapabilitiesdata/caps_2.5.0.x86_64.replies | 24 +++++++---- .../qemucapabilitiesdata/caps_2.6.0.x86_64.replies | 24 +++++++---- .../qemucapabilitiesdata/caps_2.7.0.x86_64.replies | 28 +++++++++---- tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.8.0.x86_64.replies | 37 ++++++++++++---- tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.9.0.x86_64.replies | 49 ++++++++++++++++++---- tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-intel-iommu-irqchip.args | 2 +- tests/qemuxml2argvtest.c | 1 + 14 files changed, 173 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 72870b9..0c7d7b1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -365,6 +365,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "block-write-threshold", "query-named-block-nodes", "kernel-irqchip", + "intel-iommu-intremap", ); @@ -1715,6 +1716,10 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsUSBNECXHCI[] = { { "p3", QEMU_CAPS_NEC_USB_XHCI_PORTS }, }; +static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIntelIOMMU[] = { + { "intremap", QEMU_CAPS_INTEL_IOMMU_INTREMAP }, +}; + /* see documentation for virQEMUCapsQMPSchemaGetByPath for the query format */ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "blockdev-add/arg-type/options/+gluster/debug-level", QEMU_CAPS_GLUSTER_DEBUG_LEVEL}, @@ -1822,6 +1827,9 @@ static struct virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { { "nec-usb-xhci", virQEMUCapsObjectPropsUSBNECXHCI, ARRAY_CARDINALITY(virQEMUCapsObjectPropsUSBNECXHCI), -1 }, + { "intel-iommu", virQEMUCapsObjectPropsIntelIOMMU, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsIntelIOMMU), + QEMU_CAPS_DEVICE_INTEL_IOMMU}, }; struct virQEMUCapsPropTypeObjects { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 362347e..4d19691 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -402,6 +402,7 @@ typedef enum { QEMU_CAPS_BLOCK_WRITE_THRESHOLD, /* BLOCK_WRITE_THRESHOLD event */ QEMU_CAPS_QUERY_NAMED_BLOCK_NODES, /* qmp query-named-block-nodes */ QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, /* -machine kernel_irqchip */ + QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c04bf83..07c4aeb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6681,6 +6681,20 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, if (!iommu) return 0; + switch (iommu->model) { + case VIR_DOMAIN_IOMMU_MODEL_INTEL: + if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_INTREMAP)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu: interrupt remapping is not supported " + "with this QEMU binary")); + return -1; + } + break; + case VIR_DOMAIN_IOMMU_MODEL_LAST: + break; + } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU)) return 0; /* Already handled via -machine */ @@ -6701,6 +6715,10 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, return -1; } virBufferAddLit(&opts, "intel-iommu"); + if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&opts, ",intremap=%s", + virTristateSwitchTypeToString(iommu->intremap)); + } case VIR_DOMAIN_IOMMU_MODEL_LAST: break; } diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies index 6822181..9f256c4 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies @@ -3123,6 +3123,16 @@ { "return": [ { + "name": "version", + "type": "uint32" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "name": "pc-i440fx-2.4", "is-default": true, "cpu-max": 255, @@ -3246,7 +3256,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -3336,21 +3346,21 @@ "name": "qemu64" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "passthrough" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -4358,7 +4368,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -4388,7 +4398,7 @@ "capability": "events" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies index 2eeed7d..876bc2f 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.replies @@ -3286,6 +3286,16 @@ { "return": [ { + "name": "version", + "type": "uint32" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "name": "pc-i440fx-2.4", "cpu-max": 255 }, @@ -3417,7 +3427,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -3507,21 +3517,21 @@ "name": "qemu64" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "passthrough" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -4566,7 +4576,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -4600,7 +4610,7 @@ "capability": "x-postcopy-ram" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -12145,7 +12155,7 @@ "meta-type": "array" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies index 0c285cd..d6d0b57 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.replies @@ -3368,6 +3368,16 @@ { "return": [ { + "name": "version", + "type": "uint32" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "name": "pc-0.12", "cpu-max": 255 }, @@ -3475,7 +3485,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -3565,21 +3575,21 @@ "name": "qemu64" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "passthrough" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -4673,7 +4683,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -4707,7 +4717,7 @@ "capability": "postcopy-ram" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -12712,7 +12722,7 @@ "meta-type": "array" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies index 4a87237..671a958 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.replies @@ -3559,6 +3559,20 @@ { "return": [ { + "name": "version", + "type": "uint32" + }, + { + "name": "intremap", + "type": "bool" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "hotpluggable-cpus": true, "name": "pc-0.12", "cpu-max": 255 @@ -3702,7 +3716,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -3795,21 +3809,21 @@ "name": "qemu64" } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "passthrough" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -4907,7 +4921,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -4941,7 +4955,7 @@ "capability": "postcopy-ram" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -13297,7 +13311,7 @@ "meta-type": "object" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index bee3ce0..ab0f8d2 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -203,6 +203,7 @@ <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> <flag name='kernel-irqchip'/> + <flag name='intel-iommu-intremap'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies index b3ad912..3087d76 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.replies @@ -3692,6 +3692,29 @@ { "return": [ { + "name": "eim", + "description": "on/off/auto", + "type": "OnOffAuto" + }, + { + "name": "x-buggy-eim", + "type": "bool" + }, + { + "name": "intremap", + "type": "bool" + }, + { + "name": "version", + "type": "uint32" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "hotpluggable-cpus": true, "name": "pc-0.12", "cpu-max": 255 @@ -3855,7 +3878,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -4061,21 +4084,21 @@ "static": false } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "passthrough" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -5198,7 +5221,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -5236,7 +5259,7 @@ "capability": "x-colo" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -14006,7 +14029,7 @@ "meta-type": "object" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml index 4e00816..85cae14 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml @@ -204,6 +204,7 @@ <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> <flag name='kernel-irqchip'/> + <flag name='intel-iommu-intremap'/> <version>2008000</version> <kvmVersion>0</kvmVersion> <package> (v2.8.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies index bae2475..3cfefb7 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies @@ -4003,6 +4003,37 @@ { "return": [ { + "name": "eim", + "description": "on/off/auto", + "type": "OnOffAuto" + }, + { + "name": "caching-mode", + "type": "bool" + }, + { + "name": "x-buggy-eim", + "type": "bool" + }, + { + "name": "intremap", + "type": "bool" + }, + { + "name": "version", + "type": "uint32" + }, + { + "name": "device-iotlb", + "type": "bool" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "hotpluggable-cpus": true, "name": "pc-0.12", "cpu-max": 255 @@ -4176,7 +4207,7 @@ "cpu-max": 255 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -4458,21 +4489,21 @@ "migration-safe": true } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ "tpm-tis" ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "passthrough" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -5721,7 +5752,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -5763,7 +5794,7 @@ "capability": "release-ram" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -14888,7 +14919,7 @@ "meta-type": "object" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { @@ -15067,7 +15098,7 @@ } } }, - "id": "libvirt-48" + "id": "libvirt-49" } { @@ -15308,7 +15339,7 @@ } } }, - "id": "libvirt-49" + "id": "libvirt-50" } { diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 1ca4df9..799c52a 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -208,6 +208,7 @@ <flag name='block-write-threshold'/> <flag name='query-named-block-nodes'/> <flag name='kernel-irqchip'/> + <flag name='intel-iommu-intremap'/> <version>2008090</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0-rc0-142-g940a8ce)</package> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args index 899d240..6c1e96a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-irqchip.args @@ -16,4 +16,4 @@ QEMU_AUDIO_DRV=none \ -monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ -no-acpi \ -boot c \ --device intel-iommu +-device intel-iommu,intremap=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 80f10cc..29cd8b5 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2495,6 +2495,7 @@ mymain(void) DO_TEST("intel-iommu-irqchip", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, + QEMU_CAPS_INTEL_IOMMU_INTREMAP, QEMU_CAPS_DEVICE_INTEL_IOMMU); DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS); -- 2.10.2
Add a new attribute to control the caching mode. https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 9 ++++ docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 24 +++++++++-- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-caching.xml | 50 ++++++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-caching.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e1a4625..266b16e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7391,6 +7391,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 cc10209..74cd863 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3899,6 +3899,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 650d206..6c696c8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14171,6 +14171,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; @@ -24118,9 +24127,18 @@ virDomainIOMMUDefFormat(virBufferPtr buf, virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2); - if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) { - virBufferAsprintf(&childBuf, "<driver intremap='%s'/>\n", - virTristateSwitchTypeToString(iommu->intremap)); + if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT || + iommu->caching != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAddLit(&childBuf, "<driver"); + if (iommu->intremap) { + virBufferAsprintf(&childBuf, " intremap='%s'", + virTristateSwitchTypeToString(iommu->intremap)); + } + if (iommu->caching != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&childBuf, " caching='%s'", + virTristateSwitchTypeToString(iommu->caching)); + } + virBufferAddLit(&childBuf, "/>\n"); } virBufferAsprintf(buf, "<iommu model='%s'", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 61f18cf..97c9356 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2214,6 +2214,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..8261d5a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml @@ -0,0 +1,50 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x7'/> + </controller> + <interface type='user'> + <mac address='52:54:00:ab:0c:5c'/> + <model type='rtl8139'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + <iommu model='intel'> + <driver intremap='on' caching='on'/> + </iommu> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml new file mode 120000 index 0000000..6935d93 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 40425f5..848da4f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1123,6 +1123,7 @@ mymain(void) QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); DO_TEST("intel-iommu-irqchip", NONE); + DO_TEST("intel-iommu-caching", NONE); DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE); -- 2.10.2
On Wed, Apr 26, 2017 at 10:29:06AM +0200, Ján Tomko wrote:
Add a new attribute to control the caching mode.
I don't know the detailed changes below, but... I feel like using the full term "caching-mode" might be slightly better, since "caching" is a too general name used everywhere, and that might mislead people. Yes I think even "caching-mode" is misleading, but it just seems better, and that's exactly the name that Intel defined in the spec. Anyway, please feel free to ignore this comment though! (In all cases, I blame Intel for the naming :-) Thanks,
https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 9 ++++ docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 24 +++++++++-- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-caching.xml | 50 ++++++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-caching.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e1a4625..266b16e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7391,6 +7391,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 cc10209..74cd863 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3899,6 +3899,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 650d206..6c696c8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14171,6 +14171,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;
@@ -24118,9 +24127,18 @@ virDomainIOMMUDefFormat(virBufferPtr buf,
virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
- if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) { - virBufferAsprintf(&childBuf, "<driver intremap='%s'/>\n", - virTristateSwitchTypeToString(iommu->intremap)); + if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT || + iommu->caching != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAddLit(&childBuf, "<driver"); + if (iommu->intremap) { + virBufferAsprintf(&childBuf, " intremap='%s'", + virTristateSwitchTypeToString(iommu->intremap)); + } + if (iommu->caching != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&childBuf, " caching='%s'", + virTristateSwitchTypeToString(iommu->caching)); + } + virBufferAddLit(&childBuf, "/>\n"); }
virBufferAsprintf(buf, "<iommu model='%s'", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 61f18cf..97c9356 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2214,6 +2214,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..8261d5a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml @@ -0,0 +1,50 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x7'/> + </controller> + <interface type='user'> + <mac address='52:54:00:ab:0c:5c'/> + <model type='rtl8139'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + <iommu model='intel'> + <driver intremap='on' caching='on'/> + </iommu> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml new file mode 120000 index 0000000..6935d93 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 40425f5..848da4f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1123,6 +1123,7 @@ mymain(void) QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); DO_TEST("intel-iommu-irqchip", NONE); + DO_TEST("intel-iommu-caching", NONE);
DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE); -- 2.10.2
-- Peter Xu
On 04/26/2017 04:29 AM, Ján Tomko wrote:
Add a new attribute to control the caching mode.
https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- docs/formatdomain.html.in | 9 ++++ docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 24 +++++++++-- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-caching.xml | 50 ++++++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-caching.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e1a4625..266b16e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7391,6 +7391,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)
IDC whether it's caching or caching-mode which Peter both suggests and says is misleading. Still indicating that this is what's used in order to enable VT-d might give a consumer/reader a chance to know what feature this is related to. I'm fine with the code - it's just the documentation that's bothering me. The capabilities code will need some obvious merging with current git head, but that's mundane. I'm OOO on Mon/Tue BTW, so a review from me of any followup would need to wait or you could twist someone else's ARM ;-) John
+ </p> + </dd> </dl> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index cc10209..74cd863 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3899,6 +3899,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 650d206..6c696c8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14171,6 +14171,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;
@@ -24118,9 +24127,18 @@ virDomainIOMMUDefFormat(virBufferPtr buf,
virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
- if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) { - virBufferAsprintf(&childBuf, "<driver intremap='%s'/>\n", - virTristateSwitchTypeToString(iommu->intremap)); + if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT || + iommu->caching != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAddLit(&childBuf, "<driver"); + if (iommu->intremap) { + virBufferAsprintf(&childBuf, " intremap='%s'", + virTristateSwitchTypeToString(iommu->intremap)); + } + if (iommu->caching != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&childBuf, " caching='%s'", + virTristateSwitchTypeToString(iommu->caching)); + } + virBufferAddLit(&childBuf, "/>\n"); }
virBufferAsprintf(buf, "<iommu model='%s'", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 61f18cf..97c9356 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2214,6 +2214,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..8261d5a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml @@ -0,0 +1,50 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x7'/> + </controller> + <interface type='user'> + <mac address='52:54:00:ab:0c:5c'/> + <model type='rtl8139'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + <iommu model='intel'> + <driver intremap='on' caching='on'/> + </iommu> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml new file mode 120000 index 0000000..6935d93 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-caching.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 40425f5..848da4f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1123,6 +1123,7 @@ mymain(void) QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_IOMMU); DO_TEST("intel-iommu-irqchip", NONE); + DO_TEST("intel-iommu-caching", NONE);
DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE);
Format the caching-mode option for the intel-iommu device, based on its <driver caching> attribute value. https://bugzilla.redhat.com/show_bug.cgi?id=1427005 --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 3 +++ src/qemu/qemu_command.c | 11 ++++++++++ tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-intel-iommu-caching.args | 25 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 11 ++++++++++ 6 files changed, 54 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0c7d7b1..0e9afd2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -366,6 +366,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-named-block-nodes", "kernel-irqchip", "intel-iommu-intremap", + + "intel-iommu-caching", /* 255 */ ); @@ -1718,6 +1720,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsUSBNECXHCI[] = { static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIntelIOMMU[] = { { "intremap", QEMU_CAPS_INTEL_IOMMU_INTREMAP }, + { "caching-mode", QEMU_CAPS_INTEL_IOMMU_CACHING }, }; /* see documentation for virQEMUCapsQMPSchemaGetByPath for the query format */ diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4d19691..a3ff7e6 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -404,6 +404,9 @@ typedef enum { QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, /* -machine kernel_irqchip */ QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */ + /* 255 */ + QEMU_CAPS_INTEL_IOMMU_CACHING, /* intel-iommu.caching-mode */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 07c4aeb..566bd87 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6690,6 +6690,13 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, "with this QEMU binary")); return -1; } + if (iommu->caching != VIR_TRISTATE_SWITCH_ABSENT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_CACHING)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu: caching mode is not supported " + "with this QEMU binary")); + return -1; + } break; case VIR_DOMAIN_IOMMU_MODEL_LAST: break; @@ -6719,6 +6726,10 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, virBufferAsprintf(&opts, ",intremap=%s", virTristateSwitchTypeToString(iommu->intremap)); } + if (iommu->caching != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&opts, ",caching-mode=%s", + virTristateSwitchTypeToString(iommu->caching)); + } case VIR_DOMAIN_IOMMU_MODEL_LAST: break; } diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 799c52a..0193492 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -209,6 +209,7 @@ <flag name='query-named-block-nodes'/> <flag name='kernel-irqchip'/> <flag name='intel-iommu-intremap'/> + <flag name='intel-iommu-caching'/> <version>2008090</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0-rc0-142-g940a8ce)</package> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args new file mode 100644 index 0000000..1bec6d0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-caching.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-machine q35,accel=tcg \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device intel-iommu,intremap=on,caching-mode=on \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ +-device ich9-usb-ehci1,id=usb,bus=pci.2,addr=0x2.0x7 \ +-device rtl8139,vlan=0,id=net0,mac=52:54:00:ab:0c:5c,bus=pci.2,addr=0x1 \ +-net user,vlan=0,name=hostnet0 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 29cd8b5..aab20c4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2497,6 +2497,17 @@ mymain(void) QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, QEMU_CAPS_INTEL_IOMMU_INTREMAP, QEMU_CAPS_DEVICE_INTEL_IOMMU); + DO_TEST("intel-iommu-caching", + QEMU_CAPS_MACHINE_OPT, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, + QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_DEVICE_INTEL_IOMMU, + QEMU_CAPS_INTEL_IOMMU_INTREMAP, + QEMU_CAPS_INTEL_IOMMU_CACHING); DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS); -- 2.10.2
participants (4)
-
John Ferlan -
Ján Tomko -
Paolo Bonzini -
Peter Xu