Re: [PATCH v2] iommu/arm-smmu-v3: Allow default substream bypass with a pasid support

From: Jason Gunthorpe <jgg@nvidia.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Nicolin Chen <nicolinc@nvidia.com>,
	will@kernel.org, joro@8bytes.org, mshavit@google.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev
Subject: Re: [PATCH v2] iommu/arm-smmu-v3: Allow default substream bypass with a pasid support
Date: Thu, 17 Aug 2023 13:28:39 -0300	[thread overview]
Message-ID: <ZN5Kt4mEGp0XnGVI@nvidia.com> (raw)
In-Reply-To: <9ba29982-ae96-bf53-f021-21cb1b22643a@arm.com>

On Thu, Aug 17, 2023 at 05:24:51PM +0100, Robin Murphy wrote:
> > > @@ -2435,6 +2440,16 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> > >   	} else if (smmu_domain->smmu != smmu)
> > >   		ret = -EINVAL;
> > > +	/*
> > > +	 * When attaching an IDENTITY domain to a master with pasid capability,
> > > +	 * the master can still enable SVA feature by allocating a multi-entry
> > > +	 * CD table and attaching the IDENTITY domain to its default substream
> > > +	 * that alone can be byassed using the S1DSS field of the STE.
> > > +	 */
> > > +	if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS && master->ssid_bits &&
> > > +	    smmu->features & ARM_SMMU_FEAT_TRANS_S1)
> > > +		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS_S1DSS;
> > 
> > Then you don't technically need to do this.
> > 
> > Though if we can't atomically change the STE from IDENTITY to IDENTIY
> > with a CD then you still have to do something here,
> 
> Strictly I think we are safe to do that - fill in all the S1* fields while
> Config[0] is still 0 and they're ignored, sync, then set Config[0]. Adding a
> CD table under a translation domain should be achievable as well, since
> S1CDMax, S1ContextPtr and S1Fmt can all be updated together atomically
> (although it's still the kind of switcheroo where I'd be scared of a massive
> boulder suddenly rolling out of the ceiling...)

That sounds pretty good then we don't have to force staying in CD mode

> > but really what we
> > want is to force a CD table for all cases if PASID is enabled, and
> > force DMA domains to be S2 domains as well.
> 
> Wut? No, DMA domains really want to be stage 1, for many reasons.
> Implementing them with stage 2 when stage 1 isn't supported was always a bit
> of a bodge, but thankfully I'm not aware of anyone ever building a
> stage-2-only SMMUv3 anyway.
> 
> The most glaringly obvious one, though, is that I think people like PASID
> support and SVA to actually work ;)

Blah, I keep doing this and swapping S1/S2 in this confusing naming
scheme (and it doesn't help that AMD is backwards, sigh) - I ment what
you said :)

Thanks,
Jason