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

[Aneesh Kumar K . V]

"Tian, Kevin" <kevin.tian@xxxxxxxxx> writes:

>> From: Aneesh Kumar K.V (Arm) <aneesh.kumar@xxxxxxxxxx>
>> Sent: Tuesday, June 10, 2025 2:52 PM
>> 
>> The iommufd subsystem uses the VFIO character device descriptor to bind
>
> it's the 'user' or 'vmm' initiating that operation, not the subsystem itself.
>
>> a device file to an iommufd context via the VFIO_DEVICE_BIND_IOMMUFD
>> ioctl. This binding returns a bind_id, which is then used in subsequent
>
> there is no 'bind_id' in any uAPI. let's stick to 'devid'.
>
>> iommufd ioctls such as IOMMU_HWPT_ALLOC, IOMMU_VIOMMU_ALLOC,
>> and
>> IOMMU_VDEVICE_ALLOC.
>> 
>> Among these, IOMMU_VDEVICE_ALLOC is special—the lifetime of a virtual
>> device (vdevice) should be tied to the bound state of its physical
>> device.
>> 
>> In the current kernel, there is no enforced dependency between
>> iommufd_device and iommufd_vdevice. This patch introduces such a
>> dependency: when the device is unbound, the associated vdevice is now
>> automatically destroyed.
>
> This went backward.
>
> The initial v5 patch [1] from Nicolin was similar to what this
> patch does. Jason explained [2] why it's unsafe to destroy "userspace
> created" objects behind the scene. And a general rule in iommufd is
> to not take long term references on kernel owned objects.
>
> [1] https://lore.kernel.org/all/53025c827c44d68edb6469bfd940a8e8bc6147a5.1729897278.git.nicolinc@xxxxxxxxxx/
> [2] https://lore.kernel.org/all/20241029184801.GW6956@xxxxxxxxxx/
>
>> 
>> Although there is already an implicit dependency—vdevices can only be
>> destroyed after the iommufd_device is unbound due to the VFIO cdev file
>> descriptor holding a reference to the iommu file descriptor—this patch
>
> I didn't get this part. The user owns the life cycle of its created objects.
> We don't have such restriction that a vdevice object can be destroyed
> only after device is unbound...
>

What I described above explains how the kernel manages destruction of
these objects.

The vfio subsystem retains a reference to the iommufd file descriptor
through:

    vfio_df_ioctl_bind_iommufd() → iommufd_ctx_from_fd()

This reference prevents the associated vdevice from being destroyed when
the iommufd file descriptor is closed, as long as the idevice remains
bound. While vdevice objects can still be explicitly destroyed using
iommufd_destroy(), with this patch, if the idevice is still bound,
attempting to destroy the vdevice will return EBUSY.

In effect, the change ensures that once a vdevice is created, its
lifecycle is tied to that of the idevice.

>From other emails in this thread, I understand that beyond the the
changes in this patch, we want to prevent the reuse of the vdevice
object ID. We also need to ensure that any userspace-initiated operation
on this vdevice object ID results in an error after the idevice is
unbound?

-aneesh