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

Alex Williamson
2020-01-29T23:57:15Z

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;
> };