> From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> Sent: Wednesday, March 15, 2023 4:53 PM
>
> > From: Tian, Kevin <kevin.tian@xxxxxxxxx>
> > Sent: Wednesday, March 15, 2023 2:52 PM
> >
> > > From: Nicolin Chen
> > > Sent: Wednesday, March 15, 2023 2:22 PM
> > >
> > > On Wed, Mar 15, 2023 at 06:16:37AM +0000, Tian, Kevin wrote:
> > > > External email: Use caution opening links or attachments
> > > >
> > > >
> > > > > From: Nicolin Chen
> > > > > Sent: Wednesday, March 15, 2023 2:51 AM
> > > > >
> > > > > On Fri, Mar 10, 2023 at 02:08:15AM +0000, Tian, Kevin wrote:
> > > > > > External email: Use caution opening links or attachments
> > > > > >
> > > > > >
> > > > > > > From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> > > > > > > Sent: Wednesday, March 8, 2023 9:14 PM
> > > > > > >
> > > > > > > @@ -449,33 +450,18 @@ iommufd_access_create(struct
> > > iommufd_ctx
> > > > > *ictx,
> > > > > > > u32 ioas_id,
> > > > > > > access->data = data;
> > > > > > > access->ops = ops;
> > > > > > >
> > > > > > > - obj = iommufd_get_object(ictx, ioas_id, IOMMUFD_OBJ_IOAS);
> > > > > > > - if (IS_ERR(obj)) {
> > > > > > > - rc = PTR_ERR(obj);
> > > > > > > - goto out_abort;
> > > > > > > - }
> > > > > > > - access->ioas = container_of(obj, struct iommufd_ioas, obj);
> > > > > > > - iommufd_ref_to_users(obj);
> > > > > > > -
> > > > > > > if (ops->needs_pin_pages)
> > > > > > > access->iova_alignment = PAGE_SIZE;
> > > > > > > else
> > > > > > > access->iova_alignment = 1;
> > > > > > > - rc = iopt_add_access(&access->ioas->iopt, access);
> > > > > > > - if (rc)
> > > > > > > - goto out_put_ioas;
> > > > > > >
> > > > > > > /* The calling driver is a user until iommufd_access_destroy()
> */
> > > > > > > refcount_inc(&access->obj.users);
> > > > > > > + mutex_init(&access->ioas_lock);
> > > > > > > access->ictx = ictx;
> > > > > > > iommufd_ctx_get(ictx);
> > > > > >
> > > > > > this refcnt get should be moved to the start given next patch
> > > > > > removes the reference in the caller side.
>
> This change is ok but seems not necessary.
>
> Yes, vfio_iommufd_emulated_bind() will not have reference on the
> ictx after the next patch. However, it gets reference only because it
> wants to store it in vfio_device. Now, it does not store it. So no get.
> I think the caller of vfio_iommufd_emulated_bind() should ensure
> the ictx is valid. Also check the physical device bind. So maybe not
> necessary to get ictx before calling iommufd_access_create(). This is
> the same with the vfio_iommufd_physical_bind() which calls
> iommufd_device_bind() without ictx get, and iommufd_device_bind()
> also gets ictx in the end.
>
You are right. I overlooked the fact that ictx is already held by the
caller of bind.
