Messages in this thread | | | From | Zhangfei Gao <> | Date | Wed, 21 Feb 2024 14:26:49 +0800 | Subject | Re: [PATCH v10 5/6] iommu: Support mm PASID 1:n with sva domains |
| |
On Wed, 21 Feb 2024 at 11:52, Baolu Lu <baolu.lu@linux.intel.com> wrote: > > On 2024/2/21 10:45, Zhangfei Gao wrote: > > On Wed, 21 Feb 2024 at 10:06, Baolu Lu<baolu.lu@linux.intel.com> > > wrote: > >> On 2024/2/21 9:28, Zhangfei Gao wrote: > >>> On Wed, 21 Feb 2024 at 07:58, Zhang, Tina<tina.zhang@intel.com> > >>> wrote: > >>> > >>>>>> struct iommu_sva *iommu_sva_bind_device(struct device > >>>>>> *dev, struct mm_struct *mm) { + struct > >>>>>> iommu_mm_data *iommu_mm; struct iommu_domain *domain; > >>>>>> struct iommu_sva *handle; int ret; > >>>>>> > >>>>>> + mutex_lock(&iommu_sva_lock); + /* Allocate > >>>>>> mm->pasid if necessary. */ - ret = > >>>>>> iommu_sva_alloc_pasid(mm, dev); - if (ret) - return > >>>>>> ERR_PTR(ret); + iommu_mm = iommu_alloc_mm_data(mm, > >>>>>> dev); + if (IS_ERR(iommu_mm)) { + ret = > >>>>>> PTR_ERR(iommu_mm); + goto out_unlock; + } > >>>>>> > >>>>>> handle = kzalloc(sizeof(*handle), GFP_KERNEL); - if > >>>>>> (!handle) - return ERR_PTR(-ENOMEM); - - > >>>>>> mutex_lock(&iommu_sva_lock); - /* Search for an > >>>>>> existing domain. */ - domain = > >>>>>> iommu_get_domain_for_dev_pasid(dev, mm->pasid, - > >>>>>> IOMMU_DOMAIN_SVA); - if (IS_ERR(domain)) { - ret = > >>>>>> PTR_ERR(domain); + if (!handle) { + ret = -ENOMEM; > >>>>>> goto out_unlock; } > >>>>>> > >>>>>> - if (domain) { - domain->users++; - > >>>>>> goto out; > >>>>> Our multi bind test case broke since 6.8-rc1. The test case > >>>>> can use same domain & pasid, return different handle, 6.7 > >>>>> simply domain->users ++ and return. > >>>>> > >>>>>> + /* Search for an existing domain. */ + > >>>>>> list_for_each_entry(domain, &mm->iommu_mm->sva_domains, > >>>>>> next) > >>>>> { > >>>>>> + ret = iommu_attach_device_pasid(domain, > >>>>>> dev, + iommu_mm->pasid); > >>>>> Now iommu_attach_device_pasid return BUSY since the same > >>>>> pasid. And then iommu_sva_bind_device attach ret=-16 > >>>> Sounds like the test case tries to bind a device to a same mm > >>>> multiple times without unbinding the device and the > >>>> expectation is that it can always return a valid handle to pass > >>>> the test. Right? > >>> Yes > >>> > >>> The device can bind to the same mm multi-times and return > >>> different handle, Since the refcount, no need to unbind and bind > >>> sequently, The unbind can happen later with the handle. > >> Is there any real use case to bind an mm to the pasid of a device > >> multiple times? If there are cases, is it better to handle this in > >> the uacce driver? > > Yes, it is required for multi-thread, the device can provide > > multi-queue to speed up. > > > >> From iommu core's perspective, it doesn't make sense to attach the > >> same domain to the same device (or pasid) multiple times. > > But is it the refcount domain->user++ used for? Is there any reason > > not doing this. > > I was just thinking about whether to do this in the iommu core, or in > the upper layers, like uacce or iommufd. It seems that there is no need > to attach a domain to a device or pasid again if it has already been > attached.
It would be more complicated since the return handle can be used to distinguish different queues of the device.
I think domain->user should handle this case as before.
Anyway, I have sent a patch to get more feedback.
Thanks
| |