Messages in this thread | | | From | Zhangfei Gao <> | Date | Wed, 21 Feb 2024 16:01:56 +0800 | Subject | Re: [PATCH v10 5/6] iommu: Support mm PASID 1:n with sva domains |
| |
On Wed, 21 Feb 2024 at 15:41, Zhang, Tina <tina.zhang@intel.com> wrote: > > Hi, > > > -----Original Message----- > > From: Zhangfei Gao <zhangfei.gao@linaro.org> > > Sent: Wednesday, February 21, 2024 10:45 AM > > To: Baolu Lu <baolu.lu@linux.intel.com> > > Cc: Zhang, Tina <tina.zhang@intel.com>; iommu@lists.linux.dev; linux- > > kernel@vger.kernel.org; David Woodhouse <dwmw2@infradead.org>; Joerg > > Roedel <joro@8bytes.org>; Will Deacon <will@kernel.org>; Robin Murphy > > <robin.murphy@arm.com>; Jason Gunthorpe <jgg@ziepe.ca>; Tian, Kevin > > <kevin.tian@intel.com>; Nicolin Chen <nicolinc@nvidia.com>; Michael Shavit > > <mshavit@google.com>; Vasant Hegde <vasant.hegde@amd.com>; Jason > > Gunthorpe <jgg@nvidia.com>; Jean-Philippe Brucker <jean- > > philippe@linaro.org>; Hao Fang <fanghao11@huawei.com> > > Subject: Re: [PATCH v10 5/6] iommu: Support mm PASID 1:n with sva > > domains > > > > 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. > The domain->user is a refcount of the devices (or iommu group) attached to the domain. IOMMU core needs to keep this refcount to ensure that a sva domain will be released when no device uses it.
I think the limitation of one user only attach one domain one time does not make sense. Just like one file can only be opened one time by a user, then refcount is meanless.
Thanks > > Regards, > -Tina > > > > > Thanks
| |