5:47 a.m.
https://bugzilla.redhat.com/show_bug.cgi?id=1235581
Note that while support for -device intel-iommu has been
merged in QEMU for a while, that option is currently broken.
A fix is on the way:
https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg03548.html
Ján Tomko (3):
Introduce <iommu> device
Add QEMU_CAPS_DEVICE_INTEL_IOMMU
qemu: format intel-iommu on the command line
docs/schemas/domaincommon.rng | 11 +++++++
src/conf/domain_conf.c | 37 ++++++++++++++++++++++
src/conf/domain_conf.h | 14 ++++++++
src/libvirt_private.syms | 2 ++
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 30 ++++++++++++++++++
tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 +
.../qemuxml2argvdata/qemuxml2argv-intel-iommu.args | 22 +++++++++++++
.../qemuxml2argvdata/qemuxml2argv-intel-iommu.xml | 37 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 2 ++
.../qemuxml2xmlout-intel-iommu.xml | 37 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 4 +++
15 files changed, 202 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.xml
--
2.7.3
5:47 a.m.
New subject: [libvirt] [PATCH 1/3] Introduce <iommu> device
A device with an attribude 'model'.
intel-iommu is accepted so far:
<devices>
...
<iommu model='intel-iommu'/>
</devices>
https://bugzilla.redhat.com/show_bug.cgi?id=1235580
---
docs/schemas/domaincommon.rng | 11 +++++++
src/conf/domain_conf.c | 37 ++++++++++++++++++++++
src/conf/domain_conf.h | 14 ++++++++
src/libvirt_private.syms | 2 ++
.../qemuxml2argvdata/qemuxml2argv-intel-iommu.xml | 37 ++++++++++++++++++++++
.../qemuxml2xmlout-intel-iommu.xml | 37 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 4 +++
7 files changed, 142 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 162c2e0..c8a8ecb 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3709,6 +3709,14 @@
</optional>
</define>
+ <define name="iommu">
+ <element name="iommu">
+ <attribute name="model">
+ <value>intel-iommu</value>
+ </attribute>
+ </element>
+ </define>
+
<define name="input">
<element name="input">
<choice>
@@ -4198,6 +4206,9 @@
<zeroOrMore>
<ref name="panic"/>
</zeroOrMore>
+ <optional>
+ <ref name="iommu"/>
+ </optional>
</interleave>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9443281..07889ff 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -802,6 +802,9 @@ VIR_ENUM_IMPL(virDomainTPMModel, VIR_DOMAIN_TPM_MODEL_LAST,
VIR_ENUM_IMPL(virDomainTPMBackend, VIR_DOMAIN_TPM_TYPE_LAST,
"passthrough")
+VIR_ENUM_IMPL(virDomainIOMMUModel, VIR_DOMAIN_IOMMU_MODEL_LAST,
+ "intel-iommu")
+
VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST,
"default",
"unmap",
@@ -2615,6 +2618,8 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainPanicDefFree(def->panics[i]);
VIR_FREE(def->panics);
+ VIR_FREE(def->iommu);
+
VIR_FREE(def->idmap.uidmap);
VIR_FREE(def->idmap.gidmap);
@@ -17239,6 +17244,33 @@ virDomainDefParseXML(xmlDocPtr xml,
}
VIR_FREE(nodes);
+ if ((n = virXPathNodeSet("./devices/iommu", ctxt, &nodes)) < 0)
+ goto error;
+
+ if (n > 1) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("only a single IOMMU device is supported"));
+ goto error;
+ }
+
+ if (n > 0) {
+ int val;
+ if (VIR_ALLOC(def->iommu) < 0)
+ goto error;
+
+ if (!(tmp = virXMLPropString(nodes[0], "model"))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing model for IOMMU device"));
+ goto error;
+ }
+ if ((val = virDomainIOMMUModelTypeFromString(tmp)) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, _("unknown IOMMU model: %s"),
tmp);
+ goto error;
+ }
+ def->iommu->model = val;
+ }
+ VIR_FREE(nodes);
+
/* analysis of the user namespace mapping */
if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0)
goto error;
@@ -23561,6 +23593,11 @@ virDomainDefFormatInternal(virDomainDefPtr def,
goto error;
}
+ if (def->iommu) {
+ virBufferAsprintf(buf, "<iommu model='%s'/>\n",
+ virDomainIOMMUModelTypeToString(def->iommu->model));
+ }
+
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</devices>\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6e81e52..918c185 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -150,6 +150,9 @@ typedef virDomainShmemDef *virDomainShmemDefPtr;
typedef struct _virDomainTPMDef virDomainTPMDef;
typedef virDomainTPMDef *virDomainTPMDefPtr;
+typedef struct _virDomainIOMMUDef virDomainIOMMUDef;
+typedef virDomainIOMMUDef *virDomainIOMMUDefPtr;
+
/* Flags for the 'type' field in virDomainDeviceDef */
typedef enum {
VIR_DOMAIN_DEVICE_NONE = 0,
@@ -2112,6 +2115,15 @@ struct _virDomainKeyWrapDef {
int dea; /* enum virTristateSwitch */
};
+typedef enum {
+ VIR_DOMAIN_IOMMU_MODEL_INTEL,
+
+ VIR_DOMAIN_IOMMU_MODEL_LAST
+} virDomainIOMMUModel;
+
+struct _virDomainIOMMUDef {
+ virDomainIOMMUModel model;
+};
/*
* Guest VM main configuration
*
@@ -2249,6 +2261,7 @@ struct _virDomainDef {
virCPUDefPtr cpu;
virSysinfoDefPtr sysinfo;
virDomainRedirFilterDefPtr redirfilter;
+ virDomainIOMMUDefPtr iommu;
void *namespaceData;
virDomainXMLNamespace ns;
@@ -3006,6 +3019,7 @@ VIR_ENUM_DECL(virDomainTPMModel)
VIR_ENUM_DECL(virDomainTPMBackend)
VIR_ENUM_DECL(virDomainMemoryModel)
VIR_ENUM_DECL(virDomainMemoryBackingModel)
+VIR_ENUM_DECL(virDomainIOMMUModel)
/* from libvirt.h */
VIR_ENUM_DECL(virDomainState)
VIR_ENUM_DECL(virDomainNostateReason)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 501c23e..73cecfd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -343,6 +343,8 @@ virDomainHubTypeToString;
virDomainHypervTypeFromString;
virDomainHypervTypeToString;
virDomainInputDefFree;
+virDomainIOMMUModelTypeFromString;
+virDomainIOMMUModelTypeToString;
virDomainIOThreadIDAdd;
virDomainIOThreadIDDefFree;
virDomainIOThreadIDDel;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.xml
b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.xml
new file mode 100644
index 0000000..8d95383
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.xml
@@ -0,0 +1,37 @@
+<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='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='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='virtio'>
+ <address type='pci' domain='0x0000' bus='0x02'
slot='0x01' function='0x0'/>
+ </memballoon>
+ <iommu model='intel-iommu'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.xml
new file mode 100644
index 0000000..8d95383
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.xml
@@ -0,0 +1,37 @@
+<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='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='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='virtio'>
+ <address type='pci' domain='0x0000' bus='0x02'
slot='0x01' function='0x0'/>
+ </memballoon>
+ <iommu model='intel-iommu'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 7db9cb7..855eb40 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -829,6 +829,10 @@ mymain(void)
DO_TEST("video-qxl-heads");
DO_TEST("video-qxl-noheads");
+ DO_TEST_FULL("intel-iommu", WHEN_ACTIVE, GIC_NONE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE);
+
qemuTestDriverFree(&driver);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
--
2.7.3
9:34 a.m.
New subject: [libvirt] [PATCH 1/3] Introduce <iommu> device
On 06/23/2016 06:47 AM, Ján Tomko wrote:
A device with an attribude 'model'.
Attribute
intel-iommu is accepted so far:
<devices>
...
<iommu model='intel-iommu'/>
</devices>
The text reads like it already exists, but we're adding the XML here.
https://bugzilla.redhat.com/show_bug.cgi?id=1235580
---
docs/schemas/domaincommon.rng | 11 +++++++
src/conf/domain_conf.c | 37 ++++++++++++++++++++++
src/conf/domain_conf.h | 14 ++++++++
src/libvirt_private.syms | 2 ++
.../qemuxml2argvdata/qemuxml2argv-intel-iommu.xml | 37 ++++++++++++++++++++++
.../qemuxml2xmlout-intel-iommu.xml | 37 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 4 +++
7 files changed, 142 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.xml
FYI: There is a qemu v5 of the series now:
https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07686.html
w/r/t this patch:
There's no description in formatdomain.html.in
I think you need to add ABI checks too.
Although the implementation is to add a "-device intel-iommu' to the
command line, it would seem to me based purely on the former/current
means to enable this via "-machine,iommu=on" and the comments from the
qemu commit:
"The device is part of the machine properties because we wanted
to ensure is created before any other PCI device."
is this then perhaps more of a hypervisor feature since IIUC in the long
run it's the Intel VT-d and AMD IOV "support" that allow it to work. It
seems though if it were to be used prior to qemu 2.7, then providing
"machine,iommu=on" would be required.
In any case, probably need to wait for QEMU fixes to be finally accepted
as well...
diff --git a/docs/schemas/domaincommon.rng
b/docs/schemas/domaincommon.rng
index 162c2e0..c8a8ecb 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3709,6 +3709,14 @@
</optional>
</define>
+ <define name="iommu">
+ <element name="iommu">
+ <attribute name="model">
+ <value>intel-iommu</value>
Isn't this redundant?
<iommu model='intel-iommu'>
Is the future command option going to be "-device amd-iommu" or "-device
ppc-iommu" or "-device arm-iommu"?
Seems we'd want to indicate "<iommu model='intel'/> and then
generate
the command using '%s'-iommu, using *TypeToString.
+ </attribute>
+ </element>
+ </define>
+
<define name="input">
<element name="input">
<choice>
@@ -4198,6 +4206,9 @@
<zeroOrMore>
<ref name="panic"/>
</zeroOrMore>
+ <optional>
+ <ref name="iommu"/>
+ </optional>
</interleave>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9443281..07889ff 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -802,6 +802,9 @@ VIR_ENUM_IMPL(virDomainTPMModel, VIR_DOMAIN_TPM_MODEL_LAST,
VIR_ENUM_IMPL(virDomainTPMBackend, VIR_DOMAIN_TPM_TYPE_LAST,
"passthrough")
+VIR_ENUM_IMPL(virDomainIOMMUModel, VIR_DOMAIN_IOMMU_MODEL_LAST,
+ "intel-iommu")
+
VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST,
"default",
"unmap",
@@ -2615,6 +2618,8 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainPanicDefFree(def->panics[i]);
VIR_FREE(def->panics);
+ VIR_FREE(def->iommu);
+
VIR_FREE(def->idmap.uidmap);
VIR_FREE(def->idmap.gidmap);
@@ -17239,6 +17244,33 @@ virDomainDefParseXML(xmlDocPtr xml,
}
VIR_FREE(nodes);
+ if ((n = virXPathNodeSet("./devices/iommu", ctxt, &nodes)) < 0)
+ goto error;
+
+ if (n > 1) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("only a single IOMMU device is supported"));
+ goto error;
+ }
+
+ if (n > 0) {
+ int val;
+ if (VIR_ALLOC(def->iommu) < 0)
+ goto error;
+
+ if (!(tmp = virXMLPropString(nodes[0], "model"))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing model for IOMMU device"));
+ goto error;
+ }
+ if ((val = virDomainIOMMUModelTypeFromString(tmp)) < 0) {
You won't need val, if you go with
if ((def->iommu->model = ...
IDC either way.
+ virReportError(VIR_ERR_XML_ERROR, _("unknown IOMMU
model: %s"), tmp);
+ goto error;
+ }
+ def->iommu->model = val;
+ }
+ VIR_FREE(nodes);
+
/* analysis of the user namespace mapping */
if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0)
goto error;
@@ -23561,6 +23593,11 @@ virDomainDefFormatInternal(virDomainDefPtr def,
goto error;
}
+ if (def->iommu) {
+ virBufferAsprintf(buf, "<iommu model='%s'/>\n",
+ virDomainIOMMUModelTypeToString(def->iommu->model));
+ }
+
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</devices>\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6e81e52..918c185 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -150,6 +150,9 @@ typedef virDomainShmemDef *virDomainShmemDefPtr;
typedef struct _virDomainTPMDef virDomainTPMDef;
typedef virDomainTPMDef *virDomainTPMDefPtr;
+typedef struct _virDomainIOMMUDef virDomainIOMMUDef;
+typedef virDomainIOMMUDef *virDomainIOMMUDefPtr;
+
/* Flags for the 'type' field in virDomainDeviceDef */
typedef enum {
VIR_DOMAIN_DEVICE_NONE = 0,
@@ -2112,6 +2115,15 @@ struct _virDomainKeyWrapDef {
int dea; /* enum virTristateSwitch */
};
+typedef enum {
+ VIR_DOMAIN_IOMMU_MODEL_INTEL,
+
+ VIR_DOMAIN_IOMMU_MODEL_LAST
+} virDomainIOMMUModel;
+
+struct _virDomainIOMMUDef {
+ virDomainIOMMUModel model;
+};
/*
* Guest VM main configuration
*
@@ -2249,6 +2261,7 @@ struct _virDomainDef {
virCPUDefPtr cpu;
virSysinfoDefPtr sysinfo;
virDomainRedirFilterDefPtr redirfilter;
+ virDomainIOMMUDefPtr iommu;
void *namespaceData;
virDomainXMLNamespace ns;
@@ -3006,6 +3019,7 @@ VIR_ENUM_DECL(virDomainTPMModel)
VIR_ENUM_DECL(virDomainTPMBackend)
VIR_ENUM_DECL(virDomainMemoryModel)
VIR_ENUM_DECL(virDomainMemoryBackingModel)
+VIR_ENUM_DECL(virDomainIOMMUModel)
/* from libvirt.h */
VIR_ENUM_DECL(virDomainState)
VIR_ENUM_DECL(virDomainNostateReason)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 501c23e..73cecfd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -343,6 +343,8 @@ virDomainHubTypeToString;
virDomainHypervTypeFromString;
virDomainHypervTypeToString;
virDomainInputDefFree;
+virDomainIOMMUModelTypeFromString;
+virDomainIOMMUModelTypeToString;
virDomainIOThreadIDAdd;
virDomainIOThreadIDDefFree;
virDomainIOThreadIDDel;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.xml
b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.xml
new file mode 100644
index 0000000..8d95383
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.xml
@@ -0,0 +1,37 @@
+<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='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='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='virtio'>
+ <address type='pci' domain='0x0000' bus='0x02'
slot='0x01' function='0x0'/>
+ </memballoon>
+ <iommu model='intel-iommu'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.xml
new file mode 100644
Since I recently went through this with the secret/LUKS changes and the
fact that the input/output files don't differ, if you create a soft link
in the output directory to the argvdata directory for it's file instead
of a whole new file, then the xml2xml test will use that.
John
index 0000000..8d95383
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu.xml
@@ -0,0 +1,37 @@
+<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='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='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='virtio'>
+ <address type='pci' domain='0x0000' bus='0x02'
slot='0x01' function='0x0'/>
+ </memballoon>
+ <iommu model='intel-iommu'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 7db9cb7..855eb40 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -829,6 +829,10 @@ mymain(void)
DO_TEST("video-qxl-heads");
DO_TEST("video-qxl-noheads");
+ DO_TEST_FULL("intel-iommu", WHEN_ACTIVE, GIC_NONE,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE);
+
qemuTestDriverFree(&driver);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
5:47 a.m.
New subject: [libvirt] [PATCH 2/3] Add QEMU_CAPS_DEVICE_INTEL_IOMMU
Check whether QEMU supports -device intel-iommu
https://bugzilla.redhat.com/show_bug.cgi?id=1235580
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 +
5 files changed, 6 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4ed5b71..2076297 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -336,6 +336,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"drive-detect-zeroes",
"tls-creds-x509", /* 230 */
+ "intel-iommu",
);
@@ -1564,6 +1565,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "pxb", QEMU_CAPS_DEVICE_PXB },
{ "pxb-pcie", QEMU_CAPS_DEVICE_PXB_PCIE },
{ "tls-creds-x509", QEMU_CAPS_OBJECT_TLS_CREDS_X509 },
+ { "intel-iommu", QEMU_CAPS_DEVICE_INTEL_IOMMU },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index affb639..da448e9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -370,6 +370,7 @@ typedef enum {
/* 230 */
QEMU_CAPS_OBJECT_TLS_CREDS_X509, /* -object tls-creds-x509 */
+ QEMU_CAPS_DEVICE_INTEL_IOMMU, /* -device intel-iommu */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
index 112ac95..98c260c 100644
--- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
@@ -182,6 +182,7 @@
<flag name='qxl-vga.max_outputs'/>
<flag name='spice-unix'/>
<flag name='drive-detect-zeroes'/>
+ <flag name='intel-iommu'/>
<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 8157985..590c8c1 100644
--- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
@@ -187,6 +187,7 @@
<flag name='spice-unix'/>
<flag name='drive-detect-zeroes'/>
<flag name='tls-creds-x509'/>
+ <flag name='intel-iommu'/>
<version>2005000</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 1d503dd..128ac11 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
@@ -193,6 +193,7 @@
<flag name='spice-unix'/>
<flag name='drive-detect-zeroes'/>
<flag name='tls-creds-x509'/>
+ <flag name='intel-iommu'/>
<version>2006000</version>
<kvmVersion>0</kvmVersion>
<package></package>
--
2.7.3
9:36 a.m.
New subject: [libvirt] [PATCH 2/3] Add QEMU_CAPS_DEVICE_INTEL_IOMMU
On 06/23/2016 06:47 AM, Ján Tomko wrote:
Check whether QEMU supports -device intel-iommu
https://bugzilla.redhat.com/show_bug.cgi?id=1235580
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 +
5 files changed, 6 insertions(+)
Your cover letter notes the qemu support -device option is broken and a
"fix" is on qemu-devel (assuming it would go into QEMU 2.7).
However, you've touched caps 2.4, 2.5, and 2.6. So, if -device is broken
before qemu 2.7, what happens if we "pass" our capabilities checks and
attempt to add using a broken mechanism?
It seems one way to "detect" that the -device iommu is not broken is to
not find the "iommu" property on the -machine command line (patch 5 of
the qemu series) - a rather ugly means of detection.
Or of course we could not care that it's broken? or we could document
that although available prior to qemu 2.7, it really only works properly
in 2.7 since as qemu-devel reviews pretty much note it wasn't something
expected to be in production use yet.
I don't think what's below is wrong, but I wonder if we need to add a
means to detect things that have been discussed on the qemu-devel
regarding the need to remove the machine attribute 'iommu'?
If this is all possible, then perhaps we would have a helper API added
that would detect the device flag and detect a missing iommu from
machine properties and return true/false based on that (so that the
"next" patch just calls it rather than checking the bit itself).
John
diff --git a/src/qemu/qemu_capabilities.c
b/src/qemu/qemu_capabilities.c
index 4ed5b71..2076297 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -336,6 +336,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"drive-detect-zeroes",
"tls-creds-x509", /* 230 */
+ "intel-iommu",
);
@@ -1564,6 +1565,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "pxb", QEMU_CAPS_DEVICE_PXB },
{ "pxb-pcie", QEMU_CAPS_DEVICE_PXB_PCIE },
{ "tls-creds-x509", QEMU_CAPS_OBJECT_TLS_CREDS_X509 },
+ { "intel-iommu", QEMU_CAPS_DEVICE_INTEL_IOMMU },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index affb639..da448e9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -370,6 +370,7 @@ typedef enum {
/* 230 */
QEMU_CAPS_OBJECT_TLS_CREDS_X509, /* -object tls-creds-x509 */
+ QEMU_CAPS_DEVICE_INTEL_IOMMU, /* -device intel-iommu */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
index 112ac95..98c260c 100644
--- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
@@ -182,6 +182,7 @@
<flag name='qxl-vga.max_outputs'/>
<flag name='spice-unix'/>
<flag name='drive-detect-zeroes'/>
+ <flag name='intel-iommu'/>
<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 8157985..590c8c1 100644
--- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
@@ -187,6 +187,7 @@
<flag name='spice-unix'/>
<flag name='drive-detect-zeroes'/>
<flag name='tls-creds-x509'/>
+ <flag name='intel-iommu'/>
<version>2005000</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 1d503dd..128ac11 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
@@ -193,6 +193,7 @@
<flag name='spice-unix'/>
<flag name='drive-detect-zeroes'/>
<flag name='tls-creds-x509'/>
+ <flag name='intel-iommu'/>
<version>2006000</version>
<kvmVersion>0</kvmVersion>
<package></package>
5:47 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: format intel-iommu on the command line
<devices>
<iommu model='intel-iommu'/>
</devices>
results in:
-device intel-iommu
https://bugzilla.redhat.com/show_bug.cgi?id=1235580
---
src/qemu/qemu_command.c | 30 ++++++++++++++++++++++
.../qemuxml2argvdata/qemuxml2argv-intel-iommu.args | 22 ++++++++++++++++
tests/qemuxml2argvtest.c | 2 ++
3 files changed, 54 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6944129..9f1d447 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6171,6 +6171,33 @@ qemuBuildBootCommandLine(virCommandPtr cmd,
static int
+qemuBuildIOMMUCommandLine(virCommandPtr cmd,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
+{
+ if (!def->iommu)
+ return 0;
+
+ switch (def->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));
+ return -1;
+ }
+ virCommandAddArgList(cmd, "-device",
+ virDomainIOMMUModelTypeToString(def->iommu->model),
+ NULL);
+ case VIR_DOMAIN_IOMMU_MODEL_LAST:
+ break;
+ }
+ return 0;
+}
+
+
+static int
qemuBuildGlobalControllerCommandLine(virCommandPtr cmd,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
@@ -9254,6 +9281,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildBootCommandLine(cmd, def, qemuCaps, &emitBootindex) < 0)
goto error;
+ if (qemuBuildIOMMUCommandLine(cmd, def, qemuCaps) < 0)
+ goto error;
+
if (qemuBuildGlobalControllerCommandLine(cmd, def, qemuCaps) < 0)
goto error;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args
b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args
new file mode 100644
index 0000000..69e4490
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args
@@ -0,0 +1,22 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M q35 \
+-m 214 \
+-smp 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 \
+-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 virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x1
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a4b8bf4..59fa420 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2023,6 +2023,8 @@ mymain(void)
QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_USB_HUB);
DO_TEST("acpi-table", NONE);
+ DO_TEST("intel-iommu", QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_INTEL_IOMMU);
qemuTestDriverFree(&driver);
--
2.7.3
9:47 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: format intel-iommu on the command line
On 06/23/2016 06:47 AM, Ján Tomko wrote:
<devices>
<iommu model='intel-iommu'/>
</devices>
results in:
-device intel-iommu
Might be nice to add a bit more "meat" to the commit message.
https://bugzilla.redhat.com/show_bug.cgi?id=1235580
---
src/qemu/qemu_command.c | 30 ++++++++++++++++++++++
.../qemuxml2argvdata/qemuxml2argv-intel-iommu.args | 22 ++++++++++++++++
tests/qemuxml2argvtest.c | 2 ++
3 files changed, 54 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args
One other thing that wasn't 100% from the qemu patches - is there a
specific need to ensure the "-device intel-iommu" is placed on the
command line prior to any PCI device being placed there as well?
It's that same snippet I pulled from review of patch 1:
"The device is part of the machine properties because we wanted
to ensure is created before any other PCI device."
But qemu patch 5/5 removes iommu from the machine command line, so I'm
left wondering if there isn't some assumption that the -device option
will need to be generated before any other PCI device.
I only mention this because if that's the case, then I think we need to
add some comments prior to calling qemuBuildIOMMUCommandLine to indicate
that placement for the "-device intel-iommu" must be done before other
PCI devices. I realize there's a general feeling against too many
comments; however, it might avoid having some bug creep in because
someone didn't know or follow a bz link or a qemu doc link from a commit
message...
Alternatively if the XML implementation becomes a hypervisor feature,
then qemuBuildBootCommandLine could add the option.
That also provides a way to have "either" 'iommu=on' or 'device
intel-iommu' be generated with "-machine" or after machine is created to
help hide or ensure future generations don't erroneously move things.
John
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6944129..9f1d447 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6171,6 +6171,33 @@ qemuBuildBootCommandLine(virCommandPtr cmd,
static int
+qemuBuildIOMMUCommandLine(virCommandPtr cmd,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
+{
+ if (!def->iommu)
+ return 0;
+
+ switch (def->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));
+ return -1;
+ }
+ virCommandAddArgList(cmd, "-device",
+ virDomainIOMMUModelTypeToString(def->iommu->model),
+ NULL);
Obviously this would need adjustment if you changed the model...
+ case VIR_DOMAIN_IOMMU_MODEL_LAST:
+ break;
+ }
+ return 0;
+}
+
+
+static int
qemuBuildGlobalControllerCommandLine(virCommandPtr cmd,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
@@ -9254,6 +9281,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildBootCommandLine(cmd, def, qemuCaps, &emitBootindex) < 0)
goto error;
+ if (qemuBuildIOMMUCommandLine(cmd, def, qemuCaps) < 0)
+ goto error;
+
if (qemuBuildGlobalControllerCommandLine(cmd, def, qemuCaps) < 0)
goto error;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args
b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args
new file mode 100644
index 0000000..69e4490
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu.args
@@ -0,0 +1,22 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M q35 \
+-m 214 \
+-smp 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 \
+-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 virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x1
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a4b8bf4..59fa420 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2023,6 +2023,8 @@ mymain(void)
QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_USB_HUB);
DO_TEST("acpi-table", NONE);
+ DO_TEST("intel-iommu", QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_INTEL_IOMMU);
qemuTestDriverFree(&driver);
3105
days inactive
3110
days old
6 comments
2 participants
participants (2)
-
John Ferlan -
Ján Tomko