On Thu, Mar 16, 2023 at 05:49:20AM +0000, Tian, Kevin wrote: > External email: Use caution opening links or attachments > > > > From: Nicolin Chen > > Sent: Thursday, March 16, 2023 1:44 PM > > > > On Thu, Mar 16, 2023 at 05:38:41AM +0000, Tian, Kevin wrote: > > > External email: Use caution opening links or attachments > > > > > > > > > > From: Nicolin Chen <nicolinc@xxxxxxxxxx> > > > > Sent: Thursday, March 16, 2023 1:33 PM > > > > > > > > Hi Kevin, > > > > > > > > I've fixed the other two commits. Here is the one that I am > > > > not sure about: > > > > > > > > On Thu, Mar 16, 2023 at 02:53:50AM +0000, Tian, Kevin wrote: > > > > > > > > > > [2] This adds iommufd_access_detach() in the cdev series: > > > > > > "iommufd/device: Add iommufd_access_detach() API" > > > > > > > > > > > > > > > > > > https://github.com/nicolinc/iommufd/commit/4110522146ca1fc0d5321c04a > > > > > > 097e2c9d9e26af4 > > > > > > > > > > also add a check if old_ioas exists it must equal to the new_ioas in > > attach. > > > > > > > > This is the commit adding detach(). And there's a check in it: > > > > if (WARN_ON(!access->ioas)) > > > > > > > > Do you mean having an "if (access->ioas) return -EBUSY;" line > > > > in the commit adding attach()? > > > > > > if (access->ioas && access->ioas != new_ioas) > > > return -EBUSY; > > > > > > yes this is for attach. > > > > OK. For attach(), the access->ioas shouldn't be !NULL, I think. > > At the point of adding attach(), the uAPI doesn't support the > > replacement use case yet. And later we have a separate API for > > that. > > what about user calling attach twice in cdev? > > > > > So I think it'd be just: > > if (access->ioas) > > return -EBUSY; > > > > The reason why I didn't add it is actually because the caller > > vfio_iommufd_emulated_attach_ioas() has a check of "attached" > > already. Yet, it doesn't hurt to have one more in the API. > > > > but here the slight difference is that in physical path we allow > attach twice to the same hwpt. they should be consistent: > > if (idev->igroup->hwpt != NULL && idev->igroup->hwpt != hwpt) > return -EINVAL; I see. The point is to support duplicated calls: ATTACH (pt_id = ioas1) ATTACH (pt_id = ioas1) Then I will add this to keep the consistency: if (access->ioas != NULL && access->ioas != new_ioas) return -EINVAL; Thanks Nic
