On Tue, May 27, 2025 at 07:56:09PM +0530, Aneesh Kumar K.V wrote:
> 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.
Yes I am, what ABI are you talking about? CC is all new.
> 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.
It is not OK, we want this in the viommu not the device for a bunch of
other reasons. I don't want two copies of the KVM running around
inside iommfd..
> >> + 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.
You are driving it from the vfio side? Then you don't need the
refcount at all here because the vfio facing APIs already take one.
Jason
