Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 11 Jul 2023 11:18:54 -0300
From:   Jason Gunthorpe <jgg@ziepe.ca>
To:     Baolu Lu <baolu.lu@linux.intel.com>
Cc:     Tina Zhang <tina.zhang@intel.com>,
        Kevin Tian <kevin.tian@intel.com>,
        Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
        Michael Shavit <mshavit@google.com>, iommu@lists.linux.dev,
        linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 5/6] iommu: Support mm PASID 1:1 with sva domain
Message-ID: <ZK1kzlAoGg8qayrT@ziepe.ca>
References: <20230707013441.365583-1-tina.zhang@intel.com>
 <20230707013441.365583-6-tina.zhang@intel.com>
 <ZKw/xS7wOoRvNfnH@ziepe.ca>
 <6e88db76-6903-cb7b-b608-811a97986592@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <6e88db76-6903-cb7b-b608-811a97986592@linux.intel.com>
Precedence: bulk

On Tue, Jul 11, 2023 at 10:43:43AM +0800, Baolu Lu wrote:
> On 2023/7/11 1:28, Jason Gunthorpe wrote:
> > > @@ -88,31 +98,41 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm
> > >   		goto out_unlock;
> > >   	}
> > > -	if (domain) {
> > > -		domain->users++;
> > > -		goto out;
> > > +	if (unlikely(domain)) {
> > > +		/* Re-attach the device to the same domain? */
> > > +		if (domain == sva_domain) {
> > > +			goto out;
> > > +		} else {
> > > +			/* Didn't get detached from the previous domain? */
> > > +			ret = -EBUSY;
> > > +			goto out_unlock;
> > > +		}
> > >   	}
> > And if we do all of this we should just get rid of the horrible
> > iommu_get_domain_for_dev_pasid() entirely.
> 
> At the core level, we have no idea about whether an sva domain allocated
> for one device is compatible with another device. Hence, we should loop
> the sva domains in the list and if the attach interface feeds back
> -EINVAL's (not compatible), we should allocate a new domain for the
> attached device and put it in the list if the new attachment is
> successful.

Yes, generally.

It would be good to undertand if the drivers desire one sva domain per
instance or one sva domain per driver - but with this approach it
becomes a driver choice which to use. I would guess that one sva
domain per instance is slightly simpler in the drivers..

iommu_get_domain_for_dev_pasid() turns into a check if the group
already has a SVA domain bound from a list, ie we turn it upside down
and have it search the list under the xa_lock instead of trying to
return a domain pointer back out.

Jason