> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Thursday, April 21, 2022 3:23 AM
>
> Instead of a general extension check change the function into a limited
> test if the iommu_domain has enforced coherency, which is the only thing
> kvm needs to query.
>
> Make the new op self contained by properly refcounting the container
> before touching it.
>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> ---
> drivers/vfio/vfio.c | 30 +++++++++++++++++++++++++++---
> include/linux/vfio.h | 3 +--
> virt/kvm/vfio.c | 16 ++++++++--------
> 3 files changed, 36 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index c9122c84583aa1..ae3e802991edf2 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -2005,11 +2005,35 @@ struct iommu_group
> *vfio_file_iommu_group(struct file *file)
> }
> EXPORT_SYMBOL_GPL(vfio_file_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
> + * @file: VFIO group file
> + *
> + * Enforced coherency means that the IOMMU ignores things like the PCIe
> 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.
> + */
> +bool vfio_file_enforced_coherent(struct file *file)
> {
> - return vfio_ioctl_check_extension(group->container, arg);
> + struct vfio_group *group = file->private_data;
> + bool ret;
> +
> + if (file->f_op != &vfio_group_fops)
> + return true;
> +
> + /*
> + * 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;
IMHO I still think returning an error here is better for 'user must
do so' than telling inaccurate info and leading to a situation
where lacking of wbinvd may incur various cache problem which
is hard to debug. Yes, it's user's own problem but having a place
to capture this problem early is still a nice thing to do.
> + ret = vfio_ioctl_check_extension(group->container,
> VFIO_DMA_CC_IOMMU);
> + vfio_group_try_dissolve_container(group);
> + return ret;
> }
> -EXPORT_SYMBOL_GPL(vfio_external_check_extension);
> +EXPORT_SYMBOL_GPL(vfio_file_enforced_coherent);
>
> /*
> * Sub-module support
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 132cf3e7cda8db..7f022ae126a392 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -143,8 +143,7 @@ extern void vfio_group_put_external_user(struct
> vfio_group *group);
> extern struct vfio_group *vfio_group_get_external_user_from_dev(struct
> device
> *dev);
> extern struct iommu_group *vfio_file_iommu_group(struct file *file);
> -extern long vfio_external_check_extension(struct vfio_group *group,
> - unsigned long arg);
> +extern bool vfio_file_enforced_coherent(struct file *file);
>
> #define VFIO_PIN_PAGES_MAX_ENTRIES (PAGE_SIZE/sizeof(unsigned
> long))
>
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index 50193ae270faca..2330b0c272e671 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -75,20 +75,20 @@ static void kvm_vfio_group_set_kvm(struct
> vfio_group *group, struct kvm *kvm)
> symbol_put(vfio_group_set_kvm);
> }
>
> -static bool kvm_vfio_group_is_coherent(struct vfio_group *vfio_group)
> +static bool kvm_vfio_file_enforced_coherent(struct file *file)
> {
> - long (*fn)(struct vfio_group *, unsigned long);
> - long ret;
> + bool (*fn)(struct file *file);
> + bool ret;
>
> - fn = symbol_get(vfio_external_check_extension);
> + fn = symbol_get(vfio_file_enforced_coherent);
> if (!fn)
> return false;
>
> - ret = fn(vfio_group, VFIO_DMA_CC_IOMMU);
> + ret = fn(file);
>
> - symbol_put(vfio_external_check_extension);
> + symbol_put(vfio_file_enforced_coherent);
>
> - return ret > 0;
> + return ret;
> }
>
> #ifdef CONFIG_SPAPR_TCE_IOMMU
> @@ -136,7 +136,7 @@ static void kvm_vfio_update_coherency(struct
> kvm_device *dev)
> mutex_lock(&kv->lock);
>
> list_for_each_entry(kvg, &kv->group_list, node) {
> - if (!kvm_vfio_group_is_coherent(kvg->vfio_group)) {
> + if (!kvm_vfio_file_enforced_coherent(kvg->file)) {
> noncoherent = true;
> break;
> }
> --
> 2.36.0
