Re: [RFC v3 3/8] vfio: Reclaim PASIDs when application is down

[Alex Williamson <alex.williamson@redhat.com>]

On Wed, 29 Jan 2020 04:11:47 -0800
"Liu, Yi L" <yi.l.liu@intel.com> wrote:

> From: Liu Yi L <yi.l.liu@intel.com>
> 
> When userspace application is down, kernel should reclaim the PASIDs
> allocated for this application to avoid PASID leak. This patch adds
> a PASID list in vfio_mm structure to track the allocated PASIDs. The
> PASID reclaim will be triggered when last vfio container is released.
> 
> Previous discussions:
> https://patchwork.kernel.org/patch/11209429/
> 
> Cc: Kevin Tian <kevin.tian@intel.com>
> CC: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> ---
>  drivers/vfio/vfio.c  | 61 +++++++++++++++++++++++++++++++++++++++++++++++++---
>  include/linux/vfio.h |  6 ++++++
>  2 files changed, 64 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index c43c757..425d60a 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -2148,15 +2148,31 @@ static struct vfio_mm *vfio_create_mm(struct mm_struct *mm)
>  	vmm->pasid_quota = VFIO_DEFAULT_PASID_QUOTA;
>  	vmm->pasid_count = 0;
>  	mutex_init(&vmm->pasid_lock);
> +	INIT_LIST_HEAD(&vmm->pasid_list);
>  
>  	list_add(&vmm->vfio_next, &vfio.vfio_mm_list);
>  
>  	return vmm;
>  }
>  
> +static void vfio_mm_reclaim_pasid(struct vfio_mm *vmm)
> +{
> +	struct pasid_node *pnode, *tmp;
> +
> +	mutex_lock(&vmm->pasid_lock);
> +	list_for_each_entry_safe(pnode, tmp, &vmm->pasid_list, next) {
> +		pr_info("%s, reclaim pasid: %u\n", __func__, pnode->pasid);
> +		list_del(&pnode->next);
> +		ioasid_free(pnode->pasid);
> +		kfree(pnode);
> +	}
> +	mutex_unlock(&vmm->pasid_lock);
> +}
> +
>  static void vfio_mm_unlock_and_free(struct vfio_mm *vmm)
>  {
>  	mutex_unlock(&vfio.vfio_mm_lock);
> +	vfio_mm_reclaim_pasid(vmm);
>  	kfree(vmm);
>  }
>  
> @@ -2204,6 +2220,39 @@ struct vfio_mm *vfio_mm_get_from_task(struct task_struct *task)
>  }
>  EXPORT_SYMBOL_GPL(vfio_mm_get_from_task);
>  
> +/**
> + * Caller should hold vmm->pasid_lock
> + */
> +static int vfio_mm_insert_pasid_node(struct vfio_mm *vmm, u32 pasid)
> +{
> +	struct pasid_node *pnode;
> +
> +	pnode = kzalloc(sizeof(*pnode), GFP_KERNEL);
> +	if (!pnode)
> +		return -ENOMEM;
> +	pnode->pasid = pasid;
> +	list_add(&pnode->next, &vmm->pasid_list);
> +
> +	return 0;
> +}
> +
> +/**
> + * Caller should hold vmm->pasid_lock
> + */
> +static void vfio_mm_remove_pasid_node(struct vfio_mm *vmm, u32 pasid)
> +{
> +	struct pasid_node *pnode, *tmp;
> +
> +	list_for_each_entry_safe(pnode, tmp, &vmm->pasid_list, next) {
> +		if (pnode->pasid == pasid) {
> +			list_del(&pnode->next);
> +			kfree(pnode);
> +			break;
> +		}

The _safe() list walk variant is only needed when we continue to walk
the list after removing an entry.  Thanks,

Alex

> +	}
> +
> +}
> +
>  int vfio_mm_pasid_alloc(struct vfio_mm *vmm, int min, int max)
>  {
>  	ioasid_t pasid;
> @@ -2221,9 +2270,15 @@ int vfio_mm_pasid_alloc(struct vfio_mm *vmm, int min, int max)
>  		ret = -ENOSPC;
>  		goto out_unlock;
>  	}
> -	vmm->pasid_count++;
>  
> -	ret = pasid;
> +	if (vfio_mm_insert_pasid_node(vmm, pasid)) {
> +		ret = -ENOSPC;
> +		ioasid_free(pasid);
> +	} else {
> +		ret = pasid;
> +		vmm->pasid_count++;
> +	}
> +
>  out_unlock:
>  	mutex_unlock(&vmm->pasid_lock);
>  	return ret;
> @@ -2243,7 +2298,7 @@ int vfio_mm_pasid_free(struct vfio_mm *vmm, ioasid_t pasid)
>  		goto out_unlock;
>  	}
>  	ioasid_free(pasid);
> -
> +	vfio_mm_remove_pasid_node(vmm, pasid);
>  	vmm->pasid_count--;
>  out_unlock:
>  	mutex_unlock(&vmm->pasid_lock);
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index b6c9c8c..a2ea7e0 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -89,12 +89,18 @@ extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops);
>  extern void vfio_unregister_iommu_driver(
>  				const struct vfio_iommu_driver_ops *ops);
>  
> +struct pasid_node {
> +	u32			pasid;
> +	struct list_head	next;
> +};
> +
>  #define VFIO_DEFAULT_PASID_QUOTA	1000
>  struct vfio_mm {
>  	struct kref			kref;
>  	struct mutex			pasid_lock;
>  	int				pasid_quota;
>  	int				pasid_count;
> +	struct list_head		pasid_list;
>  	struct mm_struct		*mm;
>  	struct list_head		vfio_next;
>  };