On Fri, Feb 24, 2023 at 03:43:37AM +0000, Liu, Yi L wrote: > > From: Tian, Kevin <kevin.tian@xxxxxxxxx> > > Sent: Friday, February 24, 2023 10:48 AM > > > > > From: Jason Gunthorpe > > > Sent: Friday, February 24, 2023 10:36 AM > > > > > > On Fri, Feb 24, 2023 at 02:21:33AM +0000, Tian, Kevin wrote: > > > > > > > Yi, while you are incorporating this change please also update the > > > > uapi header. Rename 'group_fds[]' to 'fds[]' and add comment to > > > > explain that it could be an array of group fds or a single iommufd. > > > > > > Upon reflection we can probably make it even simpler and just have a 0 > > > length fd array mean to use the iommufd the vfio_device is already > > > associated with > > > > > > And the check for correctness can be simplified to simply see if each > > > vfio_device in the dev_set is attached to the same iommufd ctx > > > pointer instead of searching the xarray. > > How about the hot reset info path? We can still keep reporting the > current information to userspace. Isn't it? Yeah, but I wonder if it is useful > another tricky question. If user passess iommufd down for reset > in the vfio iommufd compatible mode, should we support it as > well? I would say if the 0 fds mode is used and the current vfio_Device does not have an iommufd ctx then fail. That is the only requirement, however it got that ctx doesn't matter. > > Locking is fine since dev_set->lock is already held in the reset path. > > dev_set->lock is held prior to call bind_iommufd, so I agree locking > is ok. As long as the vdev's iommufd ctx and opencount cannot change under the devset lock, which I think is the case. It should be documented though in the vfio core code, as it is a bit subtle what the devset lock actually covers. Jason
