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

[Jason Gunthorpe <jgg@nvidia.com>]

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