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

Liu, Yi L
2020-01-31T12:42:36Z

Hi Alex,

> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Thursday, January 30, 2020 7:57 AM
> To: Liu, Yi L <yi.l.liu@intel.com>
> 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@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,

Nice catch. thanks, :-)

Regards,
Yi Liu