On 7/5/22 11:10, Jason Gunthorpe wrote:
The test isn't going to work if a group doesn't exist. Normally this isn't
a problem since VFIO isn't going to create a device if there is no group,
but the special CONFIG_VFIO_NOIOMMU behavior allows bypassing this
prevention. The new cap test effectively forces a group and breaks this
config option.
Move the cap test to vfio_group_find_or_alloc() which is the earliest time
we know we have a group available and thus are not running in noiommu mode.
Fixes: e8ae0e140c05 ("vfio: Require that devices support DMA cache coherence")
Reported-by "chenxiang (M)" <chenxiang66@xxxxxxxxxxxxx>
Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
---
drivers/vfio/vfio.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
This should fixe the issue with dpdk on noiommu, but I've left PPC out.
I think the right way to fix PPC is to provide the iommu_ops for the devices
groups it is creating. They don't have to be fully functional - eg they don't
have to to create domains, but if the ops exist they can correctly respond to
iommu_capable() and we don't need special code here to work around PPC being
weird.
This is what I've been doing since Friday and while the coherency thing
is easy to do, the iommu_group_claim_dma_owner() is not - it wants to
allocate domains which leads to more hooks and even when I do all that -
for example, iommu_group_claim_dma_owner() won't do what it supposed to
as nothing outside VFIO is going to try mapping DMA and fail to report
that the group is not "viable".
I think I'll post the iommu_ops PPC patch for coherency and continue
poking the other thing. Thanks,
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index e43b9496464bbf..cbb693359502d9 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -552,6 +552,16 @@ 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 (!iommu_capable(dev->bus, IOMMU_CAP_CACHE_COHERENCY)) {
+ iommu_group_put(iommu_group);
+ return ERR_PTR(-EINVAL);
+ }
+
group = vfio_group_get_from_iommu(iommu_group);
if (!group)
group = vfio_create_group(iommu_group, VFIO_IOMMU);
@@ -604,13 +614,6 @@ static int __vfio_register_dev(struct vfio_device *device,
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.
- */
- if (!iommu_capable(device->dev->bus, IOMMU_CAP_CACHE_COHERENCY))
- return -EINVAL;
-
return __vfio_register_dev(device,
vfio_group_find_or_alloc(device->dev));
}
base-commit: e2475f7b57209e3c67bf856e1ce07d60d410fb40
--
Alexey
