Jason Gunthorpe <jgg@xxxxxxxxxx> 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
