Re: [PATCH v3 12/13] PCI/TSM: support TDI related operations for host TSM driver

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Jason Gunthorpe <jgg@nvidia.com> writes:

> On Tue, May 27, 2025 at 05:18:01PM +0530, Aneesh Kumar K.V wrote:
>> > yeah, I guess, there is a couple of places like this
>> >
>> > git grep pci_dev drivers/iommu/iommufd/
>> >
>> > drivers/iommu/iommufd/device.c:                 struct pci_dev *pdev = to_pci_dev(idev->dev);
>> > drivers/iommu/iommufd/eventq.c:         struct pci_dev *pdev = to_pci_dev(dev);
>> >
>> > Although I do not see any compelling reason to have pci_dev in the TSM API, struct device should just work and not spill any PCI details to IOMMUFD but whatever... Thanks,
>> 
>> Getting the kvm reference is tricky here.
>
> The KVM will come from the viommu object, passed in by userspace that
> is the plan at least.. If you are not presenting a viommu to the guest
> then I imagine we would still have some kind of NOP viommu object..
>

I assume you are not suggesting using IOMMU_VIOMMU_ALLOC? That would
break the ABI, which we need to maintain.

Instead, my approach uses VFIO_DEVICE_BIND_IOMMUFD to associate the KVM
context. The vfio device file descriptor had already been linked to the
KVM instance via KVM_DEV_VFIO_FILE_ADD.

Through VFIO_DEVICE_BIND_IOMMUFD, we inherit the necessary KVM details
and pass them along to iommufd_device, and subsequently to
iommufd_vdevice, using IOMMU_VDEVICE_ALLOC.

>
> We need an association between the viommu and vdevice to tell the TSM
> world what it is when we tell the TSM to create the vPCI function..
>
> There is a missing ioctl in this sequence, you have to register the
> vdev with the viommu to create a vPCI function, and that may trigger a
> TSM call too.
>
> The registration should link the vdev to the viommu and then you can
> get the viommu's kvm for a later bind.
>
>> +int iommufd_vdevice_tsm_bind_ioctl(struct iommufd_ucmd *ucmd)
>> +{
>> +	struct iommu_vdevice_id *cmd = ucmd->cmd;
>> +	struct iommufd_vdevice *vdev;
>> +	int rc = 0;
>> +
>> +	vdev = container_of(iommufd_get_object(ucmd->ictx, cmd->vdevice_id,
>> +					       IOMMUFD_OBJ_VDEVICE),
>> +			    struct iommufd_vdevice, obj);
>> +	if (IS_ERR(vdev))
>> +		return PTR_ERR(vdev);
>> +
>> +	rc = tsm_bind(vdev->dev, vdev->kvm, vdev->id);
>> +	if (rc) {
>> +		rc = -ENODEV;
>> +		goto out_put_vdev;
>> +	}
>> +
>> +	/* locking? */
>> +	vdev->tsm_bound = true;
>> +	refcount_inc(&vdev->obj.users);
>
> This refcount isn't going to work, it will make an error close()
> crash..
>
> You need to auto-unbind on destruction I think.

Can you elaborate on that? if vdevice is tsm_bound,
iommufd_vdevice_destroy() do call tsm_unbind in the changes I shared.

-aneesh