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