RE: [RFC PATCH] iommufd: Destroy vdevice on device unbind

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@xxxxxxxx>
> Sent: Wednesday, June 18, 2025 11:00 PM
> 
> On Wed, Jun 18, 2025 at 08:22:44PM +0530, Aneesh Kumar K.V wrote:
> > > The full sequence I would expect a sane userspace to do is:
> > >
> > > open(vfio_cdev)
> > > ioctl(vfio_cdev, VFIO_DEVICE_BIND_IOMMUFD, iommufd)
> > > ioctl(iommufd, IOMMUFD_CMD_VIOMMU_ALLOC)
> > > ioctl(iommufd, IOMMUFD_CMD_VDEVICE_ALLOC)
> > > ioctl(iommufd, IOMMUFD_CMD_VDEVICE_DEALLOC)
> > > ioctl(iommufd, IOMMUFD_CMD_VIOMMU_DEALLOC)
> > > close(vfio_cdev);
> > >
> >
> > And if the user does
> >
> > open(vfio_cdev)
> > ioctl(vfio_cdev, VFIO_DEVICE_BIND_IOMMUFD, iommufd)
> > ioctl(iommufd, IOMMUFD_CMD_VIOMMU_ALLOC)
> > ioctl(iommufd, IOMMUFD_CMD_VDEVICE_ALLOC)
> > close(vfio_cdev);   -> this should call vdevice_destroy because idevice is
> getting destroyed here (we will put XA_ZERO_ENTRY here).
> 
> Yes, we have to destroy the vdevice internally here
> 
> > ioctl(iommufd, IOMMUFD_CMD_VDEVICE_DEALLOC) -> No error, we
> convert the XA_ZERO_ENTRY to NULL here?
> 
> This should probably fail since the user has done something wrong and
> it would be the only way to realize it. The failure could clean up the
> tombstone, or it could just leak I don't have a strong feeling.
> 
> If you leak then using XA_ZERO_ENTRY is easy, if you want to clean up
> then you'd have to have a global static 'tombstone object' that sits
> in the xarray.
> 

I prefer to leaking as it's simpler. The only value of cleanup is to get
one free object ID, which matters little upon a failure condition.