Re: [PATCH v4 1/4] iommufd/device: Associate a kvm pointer to iommufd_device

[Aneesh Kumar K . V]

Jason Gunthorpe <jgg@xxxxxxxx> writes:

> On Mon, Apr 27, 2026 at 11:40:02AM +0530, Aneesh Kumar K.V (Arm) wrote:
>> From: Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx>
>> 
>> Add a struct kvm * to iommufd_device_bind() fn and associate it
>> with idev if bind is successful.
>> 
>> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx>
>> Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
>> [nicolinc: fix build error in iommufd_test_mock_domain()]
>> Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@xxxxxxxxxx>
>> ---
>>  drivers/iommu/iommufd/device.c          | 5 ++++-
>>  drivers/iommu/iommufd/iommufd_private.h | 2 ++
>>  drivers/iommu/iommufd/selftest.c        | 2 +-
>>  drivers/vfio/iommufd.c                  | 2 +-
>>  include/linux/iommufd.h                 | 4 +++-
>>  5 files changed, 11 insertions(+), 4 deletions(-)
>
>> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
>> index 344d620cdecc..453fbceb9219 100644
>> --- a/drivers/iommu/iommufd/device.c
>> +++ b/drivers/iommu/iommufd/device.c
>> @@ -203,6 +203,7 @@ void iommufd_device_destroy(struct iommufd_object *obj)
>>   * iommufd_device_bind - Bind a physical device to an iommu fd
>>   * @ictx: iommufd file descriptor
>>   * @dev: Pointer to a physical device struct
>> + * @kvm: Pointer to struct kvm if device belongs to a KVM VM
>>   * @id: Output ID number to return to userspace for this device
>>   *
>>   * A successful bind establishes an ownership over the device and returns
>> @@ -216,7 +217,8 @@ void iommufd_device_destroy(struct iommufd_object *obj)
>>   * The caller must undo this with iommufd_device_unbind()
>>   */
>>  struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
>> -					   struct device *dev, u32 *id)
>> +					   struct device *dev, struct kvm *kvm,
>> +					   u32 *id)
>
> I thought we were trying to get away from struct kvm?
>
> https://lore.kernel.org/all/adf29Rn7q9Db0hxc@xxxxxxxxxx/
>
> Ie this should be a 'struct file *kvm_fd'
>
> ?
>
> Though I am wondering how practical it is to do this at this moment :\
>

Should we also switch 

modified   drivers/vfio/vfio.h
@@ -22,8 +22,8 @@ struct vfio_device_file {
 
 	u8 access_granted;
 	u32 devid; /* only valid when iommufd is valid */
-	spinlock_t kvm_ref_lock; /* protect kvm field */
-	struct kvm *kvm;
+	spinlock_t kvm_ref_lock; /* protect kvm_file */
+	struct file *kvm_file;
 	struct iommufd_ctx *iommufd; /* protected by struct vfio_device_set::lock */
 };
 
@@ -88,7 +88,7 @@ struct vfio_group {
 #endif
 	enum vfio_group_type		type;
 	struct mutex			group_lock;
-	struct kvm			*kvm;
+	struct file			*kvm_file;
 	struct file			*opened_file;
 	struct blocking_notifier_head	notifier;
 	struct iommufd_ctx		*iommufd;

ie,
KVM_CREATE_DEVICE with KVM_DEV_TYPE_VFIO still use kvm->users_count,
KVM_DEV_VFIO_FILE_ADD -> will switch to get_file(kvm->_file);
and VFIO_DEVICE_BIND_IOMMUFD -> will switch to get_file(df->kvm_file)

> Maybe ask Paolo how his series is going?
>
>>  {
>>  	struct iommufd_device *idev;
>>  	struct iommufd_group *igroup;
>> @@ -266,6 +268,7 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
>>  	if (!iommufd_selftest_is_mock_dev(dev))
>>  		iommufd_ctx_get(ictx);
>>  	idev->dev = dev;
>> +	idev->kvm = kvm;
>
> If we do that then I'd take a file refcount here ^^^
>
> And then put it back when the idev is freed
>
> That makes the lifecycle robust.
>
> Jason

-aneesh