Re: [Intel-gfx] [PATCH v1 1/5] iommufd: Create access in vfio_iommufd_emulated_bind()

[Nicolin Chen <nicolinc@nvidia.com>]

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