Re: [PATCH v4 12/17] iommufd: Add kAPI toward external drivers for physical devices

Jason Gunthorpe
2022-11-08T17:57:17Z

On Tue, Nov 08, 2022 at 10:34:05PM +0800, Yi Liu wrote:
> > +/**
> > + * iommufd_device_bind - Bind a physical device to an iommu fd
> > + * @ictx: iommufd file descriptor
> > + * @dev: Pointer to a physical PCI device struct
> > + * @id: Output ID number to return to userspace for this device
> > + *
> > + * A successful bind establishes an ownership over the device and returns
> > + * struct iommufd_device pointer, otherwise returns error pointer.
> > + *
> > + * A driver using this API must set driver_managed_dma and must not touch
> > + * the device until this routine succeeds and establishes ownership.
> > + *
> > + * Binding a PCI device places the entire RID under iommufd control.
> > + *
> > + * The caller must undo this with iommufd_unbind_device()
>
> it should be iommufd_device_unbind() now.

Done

> > +static int iommufd_device_do_attach(struct iommufd_device *idev,
> > + struct iommufd_hw_pagetable *hwpt,
> > + unsigned int flags)
> > +{
> > + phys_addr_t sw_msi_start = 0;
> > + int rc;
> > +
> > + mutex_lock(&hwpt->devices_lock);
> > +
> > + /*
> > + * Try to upgrade the domain we have, it is an iommu driver bug to
> > + * report IOMMU_CAP_ENFORCE_CACHE_COHERENCY but fail
> > + * enforce_cache_coherency when there are no devices attached to the
> > + * domain.
> > + */
> > + if (idev->enforce_cache_coherency && !hwpt->enforce_cache_coherency) {
> > + if (hwpt->domain->ops->enforce_cache_coherency)
> > + hwpt->enforce_cache_coherency =
> > + hwpt->domain->ops->enforce_cache_coherency(
> > + hwpt->domain);
> > + if (!hwpt->enforce_cache_coherency) {
> > + WARN_ON(list_empty(&hwpt->devices));
> > + rc = -EINVAL;
> > + goto out_unlock;
> > + }
> > + }
> > +
> > + rc = iopt_table_enforce_group_resv_regions(&hwpt->ioas->iopt, idev->dev,
> > + idev->group, &sw_msi_start);
> > + if (rc)
> > + goto out_unlock;
> > +
> > + rc = iommufd_device_setup_msi(idev, hwpt, sw_msi_start, flags);
> > + if (rc)
> > + goto out_iova;
>
> aren't the above two operations only once for a group? I remember you did
> the two after iommu_attach_group().

No, with the new attach logic per-device is simpler.

iopt_table_enforce_group_resv_regions() tags all the reserved ranges
with:

rc = iopt_reserve_iova(iopt, resv->start,
resv->length - 1 + resv->start,
device);

So they are all undone as each device detaches

And iommufd_device_setup_msi() keeps track of what has happened to the
domain via:

if (hwpt->msi_cookie)
return 0;
rc = iommu_get_msi_cookie(hwpt->domain, sw_msi_start);
if (rc)
return rc;
hwpt->msi_cookie = true;

So it is OK to call it multiple times

Thanks,
Jason