RE: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

[Tian, Kevin]

> From: Jean-Philippe Brucker <jean-philippe@xxxxxxxxxx>
> Sent: Wednesday, May 25, 2022 3:30 PM
> 
> On Wed, May 25, 2022 at 02:04:49AM +0000, Tian, Kevin wrote:
> > > From: Jean-Philippe Brucker <jean-philippe@xxxxxxxxxx>
> > > Sent: Tuesday, May 24, 2022 6:58 PM
> > >
> > > On Tue, May 24, 2022 at 10:22:28AM +0000, Tian, Kevin wrote:
> > > > > From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > > > > Sent: Thursday, May 19, 2022 3:21 PM
> > > > >
> > > > > The existing iommu SVA interfaces are implemented by calling the SVA
> > > > > specific iommu ops provided by the IOMMU drivers. There's no need
> for
> > > > > any SVA specific ops in iommu_ops vector anymore as we can achieve
> > > > > this through the generic attach/detach_dev_pasid domain ops.
> > > >
> > > > set/block_pasid_dev, to be consistent.
> > > >
> > > > > +
> > > > > +	mutex_lock(&iommu_sva_lock);
> > > > > +	/* Search for an existing domain. */
> > > > > +	domain = iommu_get_domain_for_dev_pasid(dev, mm-
> >pasid);
> > > > > +	if (domain) {
> > > > > +		sva_domain = to_sva_domain(domain);
> > > > > +		refcount_inc(&sva_domain->bond.users);
> > > > > +		goto out_success;
> > > > > +	}
> > > > > +
> > > >
> > > > why would one device/pasid be bound to a mm more than once?
> > >
> > > Device drivers can call bind() multiple times for the same device and mm,
> > > for example if one process wants to open multiple accelerator queues.
> > >
> >
> > Is it clearer to have a sva_bond_get/put() pair instead of calling
> > bind() multiple times here?
> 
> I don't think it's clearer, and it would force device drivers to keep
> track of {dev, mm} pairs, when the IOMMU subsystem already does that.
> At the moment a device driver calls
> 
> 	bond = iommu_sva_bind_device(dev, mm)
> 
> for each ADI that it wants to assign to userspace. If a process happens to
> want multiple ADIs on one device, then the {dev, mm} parameters are the
> same and bind() returns the same bond. Since the IOMMU driver needs to
> track these anyway, it might as well refcount them.
> 

My impression was that when an interface returns an object
then further reference to this object is usually directly acquired
on the object hence requires the caller to track it. But not a
strong opinion here if others all agree to favor simplicity for
the caller side.

Thanks
Kevin