RE: [PATCH 07/10] vfio: Move vfio_external_check_extension() to vfio_file_ops

Tian, Kevin
2022-04-15T04:07:58Z

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Friday, April 15, 2022 2:46 AM
>
> Focus the new op into is_enforced_coherent() which only checks the

s/coherent/coherency/

> enforced DMA coherency property of the file.
>
> Make the new op self contained by properly refcounting the container
> before touching it.
>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
> drivers/vfio/vfio.c | 27 ++++++++++++++++++++++++---
> include/linux/vfio.h | 3 +--
> virt/kvm/vfio.c | 18 +-----------------
> 3 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index eb65b4c80ece64..c08093fb6d28d5 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -2003,14 +2003,35 @@ static struct iommu_group
> *vfio_file_iommu_group(struct file *filep)
> return group->iommu_group;
> }
>
> -long vfio_external_check_extension(struct vfio_group *group, unsigned long
> arg)
> +/**
> + * vfio_file_enforced_coherent - True if the DMA associated with the VFIO
> file
> + * is always CPU cache coherent
> + * @filep: VFIO file
> + *
> + * Enforced coherent means that the IOMMU ignores things like the PCIe

s/coherent/coherency/

> no-snoop
> + * bit in DMA transactions. A return of false indicates that the user has
> + * rights to access additional instructions such as wbinvd on x86.
> + */
> +static bool vfio_file_enforced_coherent(struct file *filep)
> {
> - return vfio_ioctl_check_extension(group->container, arg);
> + struct vfio_group *group = filep->private_data;
> + bool ret;
> +
> + /*
> + * Since the coherency state is determined only once a container is
> + * attached the user must do so before they can prove they have
> + * permission.
> + */
> + if (vfio_group_add_container_user(group))
> + return true;

I wonder whether it's better to return error here and let KVM to
decide whether it wants to allow wbinvd in such case (though
likely the conclusion is same) or simply rejects adding the group.

Thanks
Kevin