> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Wednesday, June 14, 2023 8:23 PM
> On Wed, Jun 14, 2023 at 06:20:10AM +0000, Tian, Kevin wrote:
> > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > > Sent: Wednesday, June 14, 2023 2:14 PM
> > >
> > >
> > > > With that I think Jason's suggestion is to lift that test into main.c:
> > > >
> > > > int vfio_register_group_dev(struct vfio_device *device)
> > > > {
> > > > /*
> > > > * VFIO always sets IOMMU_CACHE because we offer no way for
> > > userspace to
> > > > * restore cache coherency. It has to be checked here because it is
> > > only
> > > > * valid for cases where we are using iommu groups.
> > > > */
> > > > if (type == VFIO_IOMMU && !vfio_device_is_noiommu(device) &&
> > > > !device_iommu_capable(dev, IOMMU_CAP_CACHE_COHERENCY))
> > > > return ERR_PTR(-EINVAL);
> > >
> > > vfio_device_is_noiommu() needs to be called after vfio_device_set_group().
> > > Otherwise, it's always false. So still needs to call it in the
> > > __vfio_register_dev().
> >
> > yes
>
> Right, but it needs to be in vfio_main.c, not in the group.c - so
> another patch should be added to move it.
I've got a patch as below to move it.
>From 306e71325d255eef34a1c44312bf9cdc8c302faa Mon Sep 17 00:00:00 2001
From: Yi Liu <yi.l.liu@xxxxxxxxx>
Date: Wed, 14 Jun 2023 00:37:52 -0700
Subject: [PATCH] vfio: Move the IOMMU_CAP_CACHE_COHERENCY check in
__vfio_register_dev()
The IOMMU_CAP_CACHE_COHERENCY check only applies to the physical devices
that are IOMMU-backed. This change prepares for compiling the vfio_group
infrastructure optionally as cdev does not support the physical devices
that are not IOMMU-backed. This check help to fail the device registration
for such devices if only vfio_group infrastructure is compiled out.
Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
---
drivers/vfio/group.c | 10 ----------
drivers/vfio/vfio_main.c | 11 +++++++++++
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
index 41a09a2df690..c2e0128323a7 100644
--- a/drivers/vfio/group.c
+++ b/drivers/vfio/group.c
@@ -687,16 +687,6 @@ static struct vfio_group *vfio_group_find_or_alloc(struct device *dev)
if (!iommu_group)
return ERR_PTR(-EINVAL);
- /*
- * VFIO always sets IOMMU_CACHE because we offer no way for userspace to
- * restore cache coherency. It has to be checked here because it is only
- * valid for cases where we are using iommu groups.
- */
- if (!device_iommu_capable(dev, IOMMU_CAP_CACHE_COHERENCY)) {
- iommu_group_put(iommu_group);
- return ERR_PTR(-EINVAL);
- }
-
mutex_lock(&vfio.group_lock);
group = vfio_group_find_from_iommu(iommu_group);
if (group) {
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 51c80eb32af6..ffb4585b7f0e 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -292,6 +292,17 @@ static int __vfio_register_dev(struct vfio_device *device,
if (ret)
return ret;
+ /*
+ * VFIO always sets IOMMU_CACHE because we offer no way for userspace to
+ * restore cache coherency. It has to be checked here because it is only
+ * valid for cases where we are using iommu groups.
+ */
+ if (type == VFIO_IOMMU && !vfio_device_is_noiommu(device) &&
+ !device_iommu_capable(device->dev, IOMMU_CAP_CACHE_COHERENCY)) {
+ ret = -EINVAL;
+ goto err_out;
+ }
+
ret = vfio_device_add(device);
if (ret)
goto err_out;
--
2.34.1
> I prefer the idea that vfio_device_is_noiommu() works in all the
> kconfig scenarios rather than adding #ifdefs.
But the vfio_group would be empty when CONFIG_VFIO_GROUP is unset.
>From what I got now, when CONFIG_VFIO_GROUP is unset, the stub
function always returns false.
#if IS_ENABLED(CONFIG_VFIO_GROUP)
struct vfio_group {
...;
};
static inline bool vfio_device_is_noiommu(struct vfio_device *vdev)
{
return IS_ENABLED(CONFIG_VFIO_NOIOMMU) &&
vdev->group->type == VFIO_NO_IOMMU;
}
#else
struct vfio_group;
static inline bool vfio_device_is_noiommu(struct vfio_device *vdev)
{
return false;
}
#endif
Regards,
Yi Liu
