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

[Jason Gunthorpe <jgg@ziepe.ca>]

On Wed, Jun 18, 2025 at 10:59:00AM +0530, Aneesh Kumar K.V wrote:
> Jason Gunthorpe <jgg@ziepe.ca> writes:
> 
> > On Tue, Jun 17, 2025 at 01:37:04PM +0530, Aneesh Kumar K.V wrote:
> >  
> >> How do we reclaim that object id for further reuse? 
> >
> > Maybe just don't? Userspace did something it shouldn't, it now leaked
> > 8 bytes of kernel memory until the FD is closed.
> >
> 
> Between the two sequences below, Sequence 1 is the correct one, since we
> want the object ID to be released after calling ioctl(DESTROY,
> vdevice_id), right?
> 
> Sequence 1 (Correct):
> 
> close(vfio_cdev)          → triggers vdevice destruction  
> ioctl(DESTROY, vdevice_id) → reclaims vdevice object ID  
> close(iommufd)  

This is wrong, the vdevice has outlived the idevice

> Sequence 2:
> 
> ioctl(DESTROY, vdevice_id) → returns EBUSY

It should not return EBUSY, it should destry the vdevice.

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);

> > You can keep the enum for flags, but 'force' isn't the right name. I
> > would think it is 'tombstone'
> 
> These values represent bit flags (e.g., 1, 2, 4, ...), meaning they are
> not mutually exclusive and can be combined using bitwise operations. As
> such, using an enum—which is typically intended for mutually exclusive
> values—is not appropriate in this case?

Meh. It is common to use enum for holding flags too. iommufd does it in many
places. There are good reasons to prefer to use enum vs #define.

Jason