> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Friday, April 15, 2022 2:46 AM
>
> The only user wants to get a pointer to the struct iommu_group associated
> with the VFIO file being used. Instead of returning the group ID then
> searching sysfs for that string just directly return the iommu_group
> pointer already held by the vfio_group struct.
>
> It already has a safe lifetime due to the struct file kref.
>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
This is a nice improvement.
Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> ---
> drivers/vfio/vfio.c | 20 ++++++++-----
> include/linux/vfio.h | 2 +-
> virt/kvm/vfio.c | 68 ++++++--------------------------------------
> 3 files changed, 22 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 93508f6a8beda5..4d62de69705573 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1919,10 +1919,7 @@ static const struct file_operations
> vfio_device_fops = {
> * increments the container user counter to prevent
> * the VFIO group from disposal before KVM exits.
> *
> - * 3. The external user calls vfio_external_user_iommu_id()
> - * to know an IOMMU ID.
> - *
> - * 4. When the external KVM finishes, it calls
> + * 3. When the external KVM finishes, it calls
> * vfio_group_put_external_user() to release the VFIO group.
> * This call decrements the container user counter.
> */
> @@ -2001,11 +1998,19 @@ bool vfio_external_group_match_file(struct
> vfio_group *test_group,
> }
> EXPORT_SYMBOL_GPL(vfio_external_group_match_file);
>
> -int vfio_external_user_iommu_id(struct vfio_group *group)
> +/**
> + * vfio_file_iommu_group - Return the struct iommu_group for the vfio file
> + * @filep: VFIO file
> + *
> + * The returned iommu_group is valid as long as a ref is held on the filep.
> + * VFIO files always have an iommu_group, so this cannot fail.
> + */
> +static struct iommu_group *vfio_file_iommu_group(struct file *filep)
> {
> - return iommu_group_id(group->iommu_group);
> + struct vfio_group *group = filep->private_data;
> +
> + return group->iommu_group;
> }
> -EXPORT_SYMBOL_GPL(vfio_external_user_iommu_id);
>
> long vfio_external_check_extension(struct vfio_group *group, unsigned long
> arg)
> {
> @@ -2014,6 +2019,7 @@ long vfio_external_check_extension(struct
> vfio_group *group, unsigned long arg)
> EXPORT_SYMBOL_GPL(vfio_external_check_extension);
>
> static const struct vfio_file_ops vfio_file_group_ops = {
> + .get_iommu_group = vfio_file_iommu_group,
> };
>
> /**
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 409bbf817206cc..e5ca7d5a0f1584 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -139,6 +139,7 @@ int vfio_mig_get_next_state(struct vfio_device
> *device,
> * External user API
> */
> struct vfio_file_ops {
> + struct iommu_group *(*get_iommu_group)(struct file *filep);
> };
> extern struct vfio_group *vfio_group_get_external_user(struct file *filep);
> extern void vfio_group_put_external_user(struct vfio_group *group);
> @@ -146,7 +147,6 @@ extern struct vfio_group
> *vfio_group_get_external_user_from_dev(struct device
> *dev);
> extern bool vfio_external_group_match_file(struct vfio_group *group,
> struct file *filep);
> -extern int vfio_external_user_iommu_id(struct vfio_group *group);
> extern long vfio_external_check_extension(struct vfio_group *group,
> unsigned long arg);
> const struct vfio_file_ops *vfio_file_get_ops(struct file *filep);
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index 254d8c18378163..743e4870fa1825 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -118,47 +118,14 @@ static bool kvm_vfio_group_is_coherent(struct
> vfio_group *vfio_group)
> return ret > 0;
> }
>
> -static int kvm_vfio_external_user_iommu_id(struct vfio_group *vfio_group)
> -{
> - int (*fn)(struct vfio_group *);
> - int ret = -EINVAL;
> -
> - fn = symbol_get(vfio_external_user_iommu_id);
> - if (!fn)
> - return ret;
> -
> - ret = fn(vfio_group);
> -
> - symbol_put(vfio_external_user_iommu_id);
> -
> - return ret;
> -}
> -
> -static struct iommu_group *kvm_vfio_group_get_iommu_group(
> - struct vfio_group *group)
> -{
> - int group_id = kvm_vfio_external_user_iommu_id(group);
> -
> - if (group_id < 0)
> - return NULL;
> -
> - return iommu_group_get_by_id(group_id);
> -}
> -
> static void kvm_spapr_tce_release_vfio_group(struct kvm *kvm,
> - struct vfio_group *vfio_group)
> + struct kvm_vfio_group *kvg)
> {
> - struct iommu_group *grp;
> -
> if (!IS_ENABLED(CONFIG_SPAPR_TCE_IOMMU))
> return;
>
> - grp = kvm_vfio_group_get_iommu_group(vfio_group);
> - if (WARN_ON_ONCE(!grp))
> - return;
> -
> - kvm_spapr_tce_release_iommu_group(kvm, grp);
> - iommu_group_put(grp);
> + kvm_spapr_tce_release_iommu_group(kvm,
> + kvg->ops->get_iommu_group(kvg-
> >filp));
> }
>
> /*
> @@ -283,7 +250,7 @@ static int kvm_vfio_group_del(struct kvm_device
> *dev, unsigned int fd)
>
> list_del(&kvg->node);
> kvm_arch_end_assignment(dev->kvm);
> - kvm_spapr_tce_release_vfio_group(dev->kvm, kvg-
> >vfio_group);
> + kvm_spapr_tce_release_vfio_group(dev->kvm, kvg);
> kvm_vfio_group_set_kvm(kvg->vfio_group, NULL);
> kvm_vfio_group_put_external_user(kvg->vfio_group);
> fput(kvg->filp);
> @@ -306,10 +273,8 @@ static int kvm_vfio_group_set_spapr_tce(struct
> kvm_device *dev,
> {
> struct kvm_vfio_spapr_tce param;
> struct kvm_vfio *kv = dev->private;
> - struct vfio_group *vfio_group;
> struct kvm_vfio_group *kvg;
> struct fd f;
> - struct iommu_group *grp;
> int ret;
>
> if (!IS_ENABLED(CONFIG_SPAPR_TCE_IOMMU))
> @@ -322,18 +287,6 @@ static int kvm_vfio_group_set_spapr_tce(struct
> kvm_device *dev,
> if (!f.file)
> return -EBADF;
>
> - vfio_group = kvm_vfio_group_get_external_user(f.file);
> - if (IS_ERR(vfio_group)) {
> - ret = PTR_ERR(vfio_group);
> - goto err_fdput;
> - }
> -
> - grp = kvm_vfio_group_get_iommu_group(vfio_group);
> - if (WARN_ON_ONCE(!grp)) {
> - ret = -EIO;
> - goto err_put_external;
> - }
> -
> ret = -ENOENT;
>
> mutex_lock(&kv->lock);
> @@ -341,18 +294,13 @@ static int kvm_vfio_group_set_spapr_tce(struct
> kvm_device *dev,
> list_for_each_entry(kvg, &kv->group_list, node) {
> if (kvg->filp != f.file)
> continue;
> -
> - ret = kvm_spapr_tce_attach_iommu_group(dev->kvm,
> param.tablefd,
> - grp);
> + ret = kvm_spapr_tce_attach_iommu_group(
> + dev->kvm, param.tablefd,
> + kvg->ops->get_iommu_group(kvg->filp));
> break;
> }
>
> mutex_unlock(&kv->lock);
> -
> - iommu_group_put(grp);
> -err_put_external:
> - kvm_vfio_group_put_external_user(vfio_group);
> -err_fdput:
> fdput(f);
> return ret;
> }
> @@ -418,7 +366,7 @@ static void kvm_vfio_destroy(struct kvm_device *dev)
> struct kvm_vfio_group *kvg, *tmp;
>
> list_for_each_entry_safe(kvg, tmp, &kv->group_list, node) {
> - kvm_spapr_tce_release_vfio_group(dev->kvm, kvg-
> >vfio_group);
> + kvm_spapr_tce_release_vfio_group(dev->kvm, kvg);
> kvm_vfio_group_set_kvm(kvg->vfio_group, NULL);
> kvm_vfio_group_put_external_user(kvg->vfio_group);
> fput(kvg->filp);
> --
> 2.35.1
