On Mon, 4 Jul 2022 22:10:50 -0300
Jason Gunthorpe <jgg@xxxxxxxxxx> 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(-)
Fixed-up Reported-by, added Tested-by, and pushed to vfio for-linus
branch for v5.19. Thanks,
Alex
>
> 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.
>
> 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
