[libvirt] [PATCH v2 1/2] capabilities: Provide info about host IOMMU support

[Erik Skultety]

On Sun, May 27, 2018 at 06:29:14PM +0200, Filip Alac wrote:
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=967231
>
> Signed-off-by: Filip Alac <filipalac at 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 |  3 +++
>  src/util/virpci.c            | 19 +++++++++++++++++++
>  src/util/virpci.h            |  2 ++
>  7 files changed, 49 insertions(+)
>
> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
> index 66c5de62e5..a604cc54d0 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>
> @@ -155,6 +158,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 dd2fc77f91..eb387916f2 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");

I don't think IOMMU depends on the power_management feature, IOW it should be
formatted outside this block.

>      } 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 f0a06a24df..4d41363a30 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;
       ^^^
bool will do just fine.

>  };
>
>  typedef int (*virDomainDefNamespaceParse)(xmlDocPtr, xmlNodePtr,
> @@ -327,4 +328,8 @@ void virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr);
>
>  int virCapabilitiesInitCaches(virCapsPtr caps);
>
> +int virCapabilitiesInitCaches(virCapsPtr caps);
> +
^^^^^^
Some copy paste leftover...

> +int virCapabilitiesHostInitIOMMU(virCapsPtr caps);
> +
>  #endif /* __VIR_CAPABILITIES_H */
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index a97b7fe223..258d02962c 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 40f49a8d9e..d95fa113b6 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -953,6 +953,9 @@ virQEMUCapsInit(virFileCachePtr cache)
>      virCapabilitiesAddHostMigrateTransport(caps, "tcp");
>      virCapabilitiesAddHostMigrateTransport(caps, "rdma");
>
> +    /* Add IOMMU info */
> +    virCapabilitiesHostInitIOMMU(caps);

Other drivers report capabilities too, so you should have a look at other
server-side drivers too, most notably lxc, libxl, virtuozzo, and test.

> +
>      /* QEMU can support pretty much every arch that exists,
>       * so just probe for them all - we gracefully fail
>       * if a qemu-system-$ARCH binary can't be found
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index 8d02366664..c88b13c97a 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);
>  }

virpci.c is supposed to contain functions to handle and manage PCI devices,
probing IOMMU support on the host is more of a system thing, so let's move this
to virutil.c and name it virHostHasIOMMU

> +
> +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;

We already have a helper that combines both of the checks ^above, have a look
at virFileIsDir and use that one instead.

> +
> +    /* Check if folder is empty */

Not entirely true per-se, it will only tell you there no sub-directories (which
is what you're after I know)

> +    if (sb.st_nlink <= 2)
> +        return false;

I'd prefer the following check instead:
if (virFileIsDir(path) || virDirRead(path) > 0)
    return true;

> +
> +    return true;
> +}
> diff --git a/src/util/virpci.h b/src/util/virpci.h
> index 794b7e59db..93ea8cdf6b 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

Erik