On Fri, Mar 10, 2023 at 01:36:22PM -0400, Jason Gunthorpe wrote:
> On Wed, Mar 08, 2023 at 05:13:36AM -0800, Yi Liu wrote:
>
> > +int iommufd_access_set_ioas(struct iommufd_access *access, u32 ioas_id)
> > +{
> > + struct iommufd_ioas *new_ioas = NULL, *cur_ioas;
> > + struct iommufd_ctx *ictx = access->ictx;
> > + struct iommufd_object *obj;
> > + int rc = 0;
> > +
> > + if (ioas_id) {
> > + obj = iommufd_get_object(ictx, ioas_id, IOMMUFD_OBJ_IOAS);
> > + if (IS_ERR(obj))
> > + return PTR_ERR(obj);
> > + new_ioas = container_of(obj, struct iommufd_ioas, obj);
> > + }
> > +
> > + mutex_lock(&access->ioas_lock);
> > + cur_ioas = access->ioas;
> > + if (cur_ioas == new_ioas)
> > + goto out_unlock;
> > +
> > + if (new_ioas) {
> > + rc = iopt_add_access(&new_ioas->iopt, access);
> > + if (rc)
> > + goto out_unlock;
> > + iommufd_ref_to_users(obj);
> > + }
> > +
> > + if (cur_ioas) {
> > + iopt_remove_access(&cur_ioas->iopt, access);
> > + refcount_dec(&cur_ioas->obj.users);
> > + }
>
> This should match the physical side with an add/remove/replace
> API. Especially since remove is implicit in destroy this series only
> needs the add API
I assume that the API would be iommufd_access_attach,
iommufd_access_detach, and iommufd_access_replace(). And there
might be an iommufd_access_change_pt(access, pt, bool replace)?
> And the locking shouldn't come in another patch that brings the
> replace/remove since with just split add we don't need it.
Hmm. The iommufd_access_detach would be needed in the following
cdev series, while the iommufd_access_replace would be need in
my replace series. So, that would make the API be divided into
three series.
Perhaps we can have iommufd_access_attach/detach in this series
along with a vfio_iommufd_emulated_detach_ioas, and the locking
will come with another patch in replace series?
Thanks
Nic
