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

[Liu, Yi L]

Hi Alex,

> From: Alex Williamson [mailto:alex.williamson@xxxxxxxxxx]
> Sent: Thursday, January 30, 2020 7:57 AM
> To: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> Subject: Re: [RFC v3 3/8] vfio: Reclaim PASIDs when application is down
> 
> On Wed, 29 Jan 2020 04:11:47 -0800
> "Liu, Yi L" <yi.l.liu@xxxxxxxxx> wrote:
> 
> > From: Liu Yi L <yi.l.liu@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > CC: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>
> > Cc: Alex Williamson <alex.williamson@xxxxxxxxxx>
> > Cc: Eric Auger <eric.auger@xxxxxxxxxx>
> > Cc: Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx>
> > Signed-off-by: Liu Yi L <yi.l.liu@xxxxxxxxx>
> > ---
> >  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,

Nice catch. thanks, :-)

Regards,
Yi Liu