Re: [PATCH 3/7] vfio: New external user group/file match

[Paolo Bonzini]

On 09/06/2017 23:59, Alex Williamson wrote:
> At the point where the kvm-vfio pseudo device wants to release its
> vfio group reference, we can't always acquire a new reference to make
> that happen.  The group can be in a state where we wouldn't allow a
> new reference to be added.  This new helper function allows a caller
> to match a file to a group to facilitate this.  Given a file and
> group, report if they match.  Thus the caller needs to already have a
> group reference to match to the file.  This allows the deletion of a
> group without acquiring a new reference.
> 
> Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>  drivers/vfio/vfio.c  |    9 +++++++++
>  include/linux/vfio.h |    2 ++
>  virt/kvm/vfio.c      |   27 +++++++++++++++++++--------
>  3 files changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index f2e24d5699f2..e6117de60f87 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1776,6 +1776,15 @@ void vfio_group_put_external_user(struct vfio_group *group)
>  }
>  EXPORT_SYMBOL_GPL(vfio_group_put_external_user);
>  
> +bool vfio_external_group_match_file(struct vfio_group *test_group,
> +				    struct file *filep)
> +{
> +	struct vfio_group *group = filep->private_data;
> +
> +	return (filep->f_op == &vfio_group_fops) && (group == test_group);
> +}
> +EXPORT_SYMBOL_GPL(vfio_external_group_match_file);
> +
>  int vfio_external_user_iommu_id(struct vfio_group *group)
>  {
>  	return iommu_group_id(group->iommu_group);
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index edf9b2cad277..9b34d0af5d27 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -97,6 +97,8 @@ extern void vfio_unregister_iommu_driver(
>   */
>  extern struct vfio_group *vfio_group_get_external_user(struct file *filep);
>  extern void vfio_group_put_external_user(struct vfio_group *group);
> +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);
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index db9036ef8c73..7c14729b1f2c 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -47,6 +47,22 @@ static struct vfio_group *kvm_vfio_group_get_external_user(struct file *filep)
>  	return vfio_group;
>  }
>  
> +static bool kvm_vfio_external_group_match_file(struct vfio_group *group,
> +					       struct file *filep)
> +{
> +	bool ret, (*fn)(struct vfio_group *, struct file *);
> +
> +	fn = symbol_get(vfio_external_group_match_file);
> +	if (!fn)
> +		return false;
> +
> +	ret = fn(group, filep);
> +
> +	symbol_put(vfio_external_group_match_file);
> +
> +	return ret;
> +}
> +
>  static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group)
>  {
>  	void (*fn)(struct vfio_group *);
> @@ -186,18 +202,13 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>  		if (!f.file)
>  			return -EBADF;
>  
> -		vfio_group = kvm_vfio_group_get_external_user(f.file);
> -		fdput(f);
> -
> -		if (IS_ERR(vfio_group))
> -			return PTR_ERR(vfio_group);
> -
>  		ret = -ENOENT;
>  
>  		mutex_lock(&kv->lock);
>  
>  		list_for_each_entry(kvg, &kv->group_list, node) {
> -			if (kvg->vfio_group != vfio_group)
> +			if (!kvm_vfio_external_group_match_file(kvg->vfio_group,
> +								f.file))
>  				continue;
>  
>  			list_del(&kvg->node);
> @@ -211,7 +222,7 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>  
>  		mutex_unlock(&kv->lock);
>  
> -		kvm_vfio_group_put_external_user(vfio_group);
> +		fdput(f);
>  
>  		kvm_vfio_update_coherency(dev);
>  
> 

Reviewed-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>