Re: [PATCH v7 07/14] iommu/arm-smmu-v3: Thread SSID through the arm_smmu_attach_*() interface

From: Jason Gunthorpe <jgg@nvidia.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: iommu@lists.linux.dev, Joerg Roedel <joro@8bytes.org>,
	linux-arm-kernel@lists.infradead.org,
	Robin Murphy <robin.murphy@arm.com>,
	Will Deacon <will@kernel.org>, Eric Auger <eric.auger@redhat.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>,
	Moritz Fischer <mdf@kernel.org>,
	Michael Shavit <mshavit@google.com>,
	patches@lists.linux.dev,
	Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
Subject: Re: [PATCH v7 07/14] iommu/arm-smmu-v3: Thread SSID through the arm_smmu_attach_*() interface
Date: Mon, 13 May 2024 19:28:40 -0300	[thread overview]
Message-ID: <ZkKUGMhZ8fAmtnis@nvidia.com> (raw)
In-Reply-To: <ZkG0FYhhqSW7Zmt6@nvidia.com>

On Sun, May 12, 2024 at 11:32:53PM -0700, Nicolin Chen wrote:
> On Wed, May 08, 2024 at 03:57:15PM -0300, Jason Gunthorpe wrote:
> > Allow creating and managing arm_smmu_mater_domain's with a non-zero SSID
> > through the arm_smmu_attach_*() family of functions. This triggers ATC
> > invalidation for the correct SSID in PASID cases and tracks the
> > per-attachment SSID in the struct arm_smmu_master_domain.
> > 
> > Generalize arm_smmu_attach_remove() to be able to remove SSID's as well by
> > ensuring the ATC for the PASID is flushed properly.
> > 
> > Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> > Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> 
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> 
> > @@ -2689,11 +2692,14 @@ static void arm_smmu_attach_commit(struct arm_smmu_master *master,
> >  		 * SMMU is translating for the new domain and both the old&new
> >  		 * domain will issue invalidations.
> >  		 */
> > -		arm_smmu_atc_inv_master(master);
> > +		if (state->want_ats)
> > +			arm_smmu_atc_inv_master(master, state->ssid);
> 
> Just trying to confirm my understanding here:
> 
> In the arm_smmu_set_pasid pathway, ssid is a valid pasid. So,
> here we invalidate both new and old master_domains using the
> same ssid/pasid, if either of them occupies the same cd entry.

Yes.

> And...
> 
> > -	arm_smmu_remove_master_domain(master, state->old_domain);
> > +	arm_smmu_remove_master_domain(master, state->old_domain, state->ssid);
> 
> then we remove the old one here.
> 
> Though the flow makes sense to me, yet I wonder if there can be
> such a use case like this where an old master_domain is holding
> the same cd entry as the one wanted by the new master_domain?

It could happen, the API allows it, and the short lived double
invalidation is a trade off to allow loose locking on the invalidation
side. For this kind of thing we want to make the common case of
invalidation to run fast.

Jason