Re: [PATCH v2 05/10] iommufd: Add replace support in iommufd_access_set_ioas()

[Nicolin Chen]

On Thu, Feb 09, 2023 at 03:13:08AM +0000, Tian, Kevin wrote:
 
> > --- a/drivers/iommu/iommufd/device.c
> > +++ b/drivers/iommu/iommufd/device.c
> > @@ -509,11 +509,23 @@ int iommufd_access_set_ioas(struct
> > iommufd_access *access, u32 ioas_id)
> >               iommufd_ref_to_users(obj);
> >       }
> >
> > +     /*
> > +      * Set ioas to NULL to block any further iommufd_access_pin_pages().
> > +      * iommufd_access_unpin_pages() can continue using access-
> > >ioas_unpin.
> > +      */
> > +     access->ioas = NULL;
> > +
> >       if (cur_ioas) {
> > +             if (new_ioas) {
> > +                     mutex_unlock(&access->ioas_lock);
> > +                     access->ops->unmap(access->data, 0, ULONG_MAX);
> > +                     mutex_lock(&access->ioas_lock);
> > +             }
> 
> why does above only apply to a valid new_ioas? this is the cleanup on
> cur_ioas then required even when new_ioas=NULL.
  
Though it'd make sense to put it in the common path, our current
detach routine doesn't call this unmap. If we do so, it'd become
something new to the normal detach routine. Or does this mean the
detach routine has been missing an unmap call so far?

Thanks
Nic