Re: [PATCH v10 5/6] iommu: Support mm PASID 1:n with sva domains

From: Zhangfei Gao
Date: Tue Feb 20 2024 - 20:28:31 EST


Hi, Tina

On Wed, 21 Feb 2024 at 07:58, Zhang, Tina <tina.zhang@xxxxxxxxx> 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.

Thanks