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

From: Nicolin Chen <nicolinc@nvidia.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: <will@kernel.org>, <jgg@nvidia.com>, <joro@8bytes.org>,
	<shameerali.kolothum.thodi@huawei.com>,
	<yangyicong@hisilicon.com>, <jean-philippe@linaro.org>,
	<linux-arm-kernel@lists.infradead.org>, <iommu@lists.linux.dev>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1] iommu/arm-smmu-v3: Allow default substream bypass with a pasid support
Date: Wed, 28 Jun 2023 08:59:47 -0700	[thread overview]
Message-ID: <ZJxY8wyNZiAI40lm@Asurada-Nvidia> (raw)
In-Reply-To: <2274ba47-7def-082c-8429-8f2cc18adf9f@arm.com>

Hi Robin,

On Wed, Jun 28, 2023 at 04:39:48PM +0100, Robin Murphy wrote:

> > > supporting properly, i.e. refactor a bit harder to separate the CD table
> > > parts which are common to both S1DSS bypass and S1 translation, from the
> > > CD/pagetable parts that are only relevant for translation. S1DSS bypass
> > > remains the same as Stream bypass in the sense that there is no
> > > structure corresponding to the identity domain itself, so not only does
> > > it not make sense to have a pagetable, there's also no valid place to
> > > put one anyway - touching the CD belonging to SSID 0 is strictly wrong.
> > 
> > I can try that. Yet, I think the S1DSS bypass case still belongs
> > to ARM_SMMU_DOMAIN_S1/arm_smmu_domain_finalise_s1, right?
> 
> That's what I'm disagreeing with - although S1DSS behaviour requires S1
> translation to be nominally enabled for the stream as a whole, the
> bypass domain is distinctly *not* an S1 translation domain, and there is
> no S1 context to finalise. I think it's either a generalisation of
> ARM_SMMU_DOMAIN_BYPASS based on s1cdmax, or it's its own new thing.

Hmm, the fundamental of my view is that the ARM_SMMU_DOMAIN_*
list is quite matching with the CONFIG field of an STE, which
turns the S1DSS bypass into the ARM_SMMU_DOMAIN_S1 category.
And after all S1DSS is named "S1" :)

Following your view, How about ARM_SMMU_DOMAIN_S1DSS_BYPASS?

> > I'd try keeping most of the parts intact while adding a pointer
> > to a structure holding pagetable stuff, to make it cleaner. Then
> > the S1DSS bypass case can be flagged by an empty pointer.
> 
> I'd expect that what you need for this is much the same as what Michael
> has already proposed for the PASID-generalisation series. The current
> inside-out notion of S1 domains owning CD tables is what's getting in
> the way of doing the right thing cleanly, in both cases.

Yea, Jason had the same remarks when I discussed this matter
with him. But I took the chance after I saw a shortcut :)

I have been aligning with Michael already, trying to figure
out how much ground that our use cases can share.

Thanks
Nic