On 23/09/2018 03:39, Lu Baolu wrote: >> +int iommu_sva_init_device(struct device *dev, unsigned long features, >> + unsigned int min_pasid, unsigned int max_pasid) >> +{ >> + int ret; >> + struct iommu_sva_param *param; >> + struct iommu_domain *domain = iommu_get_domain_for_dev(dev); > > This doesn't work for vt-d. The domains for host iova are self-managed > by vt-d driver itself. Hence, iommu_get_domain_for_dev() will always > return NULL unless an UNMANAGED domain is attached to the device. > > How about > > const struct iommu_ops *ops = dev->bus->iommu_ops;> > instead? Right, this should work. I also needed to change the IOMMU ops introduced in patch 3 to not take a domain. It's a shame that iommu-sva can't get the device's current domain, I was hoping we could manage the bonds per domain in common code. But it's not a big deal and on the upside, it simplifies these core patches. I was previously relying on "if we have a domain, then iommu_group_add_device has been called and therefore dev->iommu_param is set". I now do the same as iommu_register_device_fault_handler, check if iommu_param isn't NULL. I don't think there is a race with iommu_group_add/remove_device, since the device driver cannot call SVA functions before the core called its probe() callback and after the core called its remove() callback, which happen after iommu_group_add_device() and before iommu_group_remove_device() respectively. Though I don't have a full picture here, and might be wrong. I pushed the updated version to my sva/current branch Thanks, Jean