Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=967231
Signed-off-by: Filip Alac <filipalac@gmail.com>
---
docs/schemas/capability.rng | 11 +++++++++++
src/conf/capabilities.c | 8 ++++++++
src/conf/capabilities.h | 5 +++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_capabilities.c | 4 ++++
src/util/virpci.c | 19 +++++++++++++++++++
src/util/virpci.h | 2 ++
7 files changed, 50 insertions(+)
diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index e1ab5c224..7e7d8fbc7 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -39,6 +39,9 @@
<optional>
<ref name='power_management'/>
</optional>
+ <optional>
+ <ref name='iommu_support'/>
+ </optional>
<optional>
<ref name='migration'/>
</optional>
@@ -148,6 +151,14 @@
</element>
</define>
+ <define name='iommu_support'>
+ <optional>
+ <attribute name='support'>
+ <ref name='virYesNo'/>
+ </attribute>
+ </optional>
+ </define>
+
<define name='migration'>
<element name='migration_features'>
<optional>
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index dd2fc77f9..eb387916f 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1020,6 +1020,8 @@ virCapabilitiesFormatXML(virCapsPtr caps)
}
virBufferAdjustIndent(&buf, -2);
virBufferAddLit(&buf, "</power_management>\n");
+ virBufferAsprintf(&buf, "<iommu support='%s'/>\n",
+ caps->host.iommu ? "yes" : "no");
} else {
/* The host does not support any PM feature. */
virBufferAddLit(&buf, "<power_management/>\n");
@@ -1743,3 +1745,9 @@ virCapabilitiesInitCaches(virCapsPtr caps)
virBitmapFree(cpus);
return ret;
}
+
+int
+virCapabilitiesHostInitIOMMU(virCapsPtr caps)
+{
+ return caps->host.iommu = virPCIHasIOMMU();
+}
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index f0a06a24d..4d41363a3 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -183,6 +183,7 @@ struct _virCapsHost {
int nPagesSize; /* size of pagesSize array */
unsigned int *pagesSize; /* page sizes support on the system */
unsigned char host_uuid[VIR_UUID_BUFLEN];
+ int iommu;
};
typedef int (*virDomainDefNamespaceParse)(xmlDocPtr, xmlNodePtr,
@@ -327,4 +328,8 @@ void virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr);
int virCapabilitiesInitCaches(virCapsPtr caps);
+int virCapabilitiesInitCaches(virCapsPtr caps);
+
+int virCapabilitiesHostInitIOMMU(virCapsPtr caps);
+
#endif /* __VIR_CAPABILITIES_H */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a97b7fe22..258d02962 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -58,6 +58,7 @@ virCapabilitiesFreeMachines;
virCapabilitiesFreeNUMAInfo;
virCapabilitiesGetCpusForNodemask;
virCapabilitiesGetNodeInfo;
+virCapabilitiesHostInitIOMMU;
virCapabilitiesHostSecModelAddBaseLabel;
virCapabilitiesInitCaches;
virCapabilitiesInitNUMA;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 8a63db5f4..552d5452d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -948,6 +948,10 @@ virQEMUCapsInit(virFileCachePtr cache)
if (virCapabilitiesInitPages(caps) < 0)
VIR_WARN("Failed to get pages info");
+ /* Add IOMMU info */
+ if (virCapabilitiesHostInitIOMMU(caps) < 0)
+ VIR_WARN("Failed to get iommmu info");
+
/* Add domain migration transport URIs */
virCapabilitiesAddHostMigrateTransport(caps, "tcp");
virCapabilitiesAddHostMigrateTransport(caps, "rdma");
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 8d0236666..c88b13c97 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -3288,3 +3288,22 @@ virPCIEDeviceInfoFree(virPCIEDeviceInfoPtr dev)
VIR_FREE(dev->link_sta);
VIR_FREE(dev);
}
+
+bool
+virPCIHasIOMMU(void)
+{
+ struct stat sb;
+
+ /* We can only check on newer kernels with iommu groups & vfio */
+ if (stat("/sys/kernel/iommu_groups", &sb) < 0)
+ return false;
+
+ if (!S_ISDIR(sb.st_mode))
+ return false;
+
+ /* Check if folder is empty */
+ if (sb.st_nlink <= 2)
+ return false;
+
+ return true;
+}
diff --git a/src/util/virpci.h b/src/util/virpci.h
index 794b7e59d..93ea8cdf6 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -253,4 +253,6 @@ void virPCIEDeviceInfoFree(virPCIEDeviceInfoPtr dev);
ssize_t virPCIGetMdevTypes(const char *sysfspath,
virMediatedDeviceType ***types);
+bool virPCIHasIOMMU(void);
+
#endif /* __VIR_PCI_H__ */
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/25/2018 12:39 PM, Filip Alac wrote:
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=967231
>
> Signed-off-by: Filip Alac <filipalac@gmail.com>
> ---
> docs/schemas/capability.rng | 11 +++++++++++
> src/conf/capabilities.c | 8 ++++++++
> src/conf/capabilities.h | 5 +++++
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_capabilities.c | 4 ++++
> src/util/virpci.c | 19 +++++++++++++++++++
> src/util/virpci.h | 2 ++
> 7 files changed, 50 insertions(+)
>
> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
> index e1ab5c224..7e7d8fbc7 100644
> --- a/docs/schemas/capability.rng
> +++ b/docs/schemas/capability.rng
> @@ -39,6 +39,9 @@
> <optional>
> <ref name='power_management'/>
> </optional>
> + <optional>
> + <ref name='iommu_support'/>
> + </optional>
> <optional>
> <ref name='migration'/>
> </optional>
> @@ -148,6 +151,14 @@
> </element>
> </define>
>
> + <define name='iommu_support'>
> + <optional>
> + <attribute name='support'>
> + <ref name='virYesNo'/>
> + </attribute>
> + </optional>
> + </define>
This does not make much sense. This defines an optional attribute
'support' but does not say to which element.
> +
> <define name='migration'>
> <element name='migration_features'>
> <optional>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index dd2fc77f9..eb387916f 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -1020,6 +1020,8 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> }
> virBufferAdjustIndent(&buf, -2);
> virBufferAddLit(&buf, "</power_management>\n");
> + virBufferAsprintf(&buf, "<iommu support='%s'/>\n",
> + caps->host.iommu ? "yes" : "no");
This doesn't make much sense. This block starts with:
if (caps->host.powerMgmt) {
So you only print the <iommu/> iff power mgmt is available. These are
orthogonal features and have nothing in common.
> } else {
> /* The host does not support any PM feature. */
> virBufferAddLit(&buf, "<power_management/>\n");
> @@ -1743,3 +1745,9 @@ virCapabilitiesInitCaches(virCapsPtr caps)
> virBitmapFree(cpus);
> return ret;
> }
> +
> +int
> +virCapabilitiesHostInitIOMMU(virCapsPtr caps)
> +{
> + return caps->host.iommu = virPCIHasIOMMU();
> +}
Whoa, this is very unsafe. I don't quite see why
virCapabilitiesHostInitIOMMU() should fail when host doesn't have IOMMU.
In fact, it's quite the opposite because false is 0 and true is 1.
> diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
> index f0a06a24d..4d41363a3 100644
> --- a/src/conf/capabilities.h
> +++ b/src/conf/capabilities.h
> @@ -183,6 +183,7 @@ struct _virCapsHost {
> int nPagesSize; /* size of pagesSize array */
> unsigned int *pagesSize; /* page sizes support on the system */
> unsigned char host_uuid[VIR_UUID_BUFLEN];
> + int iommu;
You declare this int even though you use it only to store a boolean.
> };
>
> typedef int (*virDomainDefNamespaceParse)(xmlDocPtr, xmlNodePtr,
> @@ -327,4 +328,8 @@ void virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr);
>
> int virCapabilitiesInitCaches(virCapsPtr caps);
>
> +int virCapabilitiesInitCaches(virCapsPtr caps);
No need to copy this line ;-)
> +
> +int virCapabilitiesHostInitIOMMU(virCapsPtr caps);
> +
> #endif /* __VIR_CAPABILITIES_H */
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index a97b7fe22..258d02962 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -58,6 +58,7 @@ virCapabilitiesFreeMachines;
> virCapabilitiesFreeNUMAInfo;
> virCapabilitiesGetCpusForNodemask;
> virCapabilitiesGetNodeInfo;
> +virCapabilitiesHostInitIOMMU;
> virCapabilitiesHostSecModelAddBaseLabel;
> virCapabilitiesInitCaches;
> virCapabilitiesInitNUMA;
You should expose virPCIHasIOMMU too. After all, you're exposing it in
header file.
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 8a63db5f4..552d5452d 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -948,6 +948,10 @@ virQEMUCapsInit(virFileCachePtr cache)
> if (virCapabilitiesInitPages(caps) < 0)
> VIR_WARN("Failed to get pages info");
>
> + /* Add IOMMU info */
> + if (virCapabilitiesHostInitIOMMU(caps) < 0)
> + VIR_WARN("Failed to get iommmu info");
> +
In the XML you placed <iommu/> between <power_management/> and
<migration_features/>. Might be worth honouring that order here too.
> /* Add domain migration transport URIs */
> virCapabilitiesAddHostMigrateTransport(caps, "tcp");
> virCapabilitiesAddHostMigrateTransport(caps, "rdma");
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index 8d0236666..c88b13c97 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
> @@ -3288,3 +3288,22 @@ virPCIEDeviceInfoFree(virPCIEDeviceInfoPtr dev)
> VIR_FREE(dev->link_sta);
> VIR_FREE(dev);
> }
> +
See how the rest of functions in the file is separated by two empty
lines? Might be worth preserving that.
> +bool
> +virPCIHasIOMMU(void)
> +{
> + struct stat sb;
> +
> + /* We can only check on newer kernels with iommu groups & vfio */
> + if (stat("/sys/kernel/iommu_groups", &sb) < 0)
> + return false;
> +
> + if (!S_ISDIR(sb.st_mode))
> + return false;
> +
> + /* Check if folder is empty */
> + if (sb.st_nlink <= 2)
> + return false;
> +
> + return true;
> +}
This is similar to qemuHostdevHostSupportsPassthroughVFIO() which got me
thinking, do we even need this patch? I mean, we already kind of expose
if host is VFIO capable:
/domainCapabilities/hostdev/enum[name='pciBackend'/value/vfio.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.