On Tue, Apr 16, 2024 at 10:07:49AM +0800, Baolu Lu wrote:
> On 4/15/24 7:54 PM, Jason Gunthorpe wrote:
> > On Mon, Apr 15, 2024 at 01:32:03PM +0800, Baolu Lu wrote:
> > > On 4/12/24 4:15 PM, Yi Liu wrote:
> > > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > > > index 40dd439307e8..1e5e9249c93f 100644
> > > > --- a/include/linux/iommu.h
> > > > +++ b/include/linux/iommu.h
> > > > @@ -631,7 +631,7 @@ struct iommu_ops {
> > > > struct iommu_domain_ops {
> > > > int (*attach_dev)(struct iommu_domain *domain, struct device *dev);
> > > > int (*set_dev_pasid)(struct iommu_domain *domain, struct device *dev,
> > > > - ioasid_t pasid);
> > > > + ioasid_t pasid, struct iommu_domain *old);
> > > Is it possible to add another op to replace a domain for pasid? For
> > > example,
> > >
> > > int (*replace_dev_pasid)(domain, dev, pasid, old_domain)
> > We haven't needed that in the normal case, what would motivate it
> > here?
>
> My understanding of the difference between set_dev_pasid and
> replace_dev_pasid is that the former assumes that there is no domain
> attached to the pasid yet, so it sets the passed domain to it. For the
> latter, it simply replaces the existing domain with a new one.
>
> The set_dev_pasid doesn't need an old domain because it's assumed that
> the existing domain is NULL. The replace_dev_pasid could have an
> existing domain as its input.
I would just pass in the NULL domain for set than make another
op. iommu drivers should be exactly the same implementation for both
> Replace also implies an atomic switch between different domains. This
> makes it subtly different from a set operation.
Well, not necessarily hitless, but at least old/new/blocked - not
something corrupt.
Jason
