Re: [PATCH v2 14/18] iommu/arm-smmu-v3: Support domains with shared CDs

From: Michael Shavit <mshavit@google.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Will Deacon <will@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Joerg Roedel <joro@8bytes.org>,
	jean-philippe@linaro.org, nicolinc@nvidia.com,
	baolu.lu@linux.intel.com, linux-arm-kernel@lists.infradead.org,
	iommu@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 14/18] iommu/arm-smmu-v3: Support domains with shared CDs
Date: Wed, 14 Jun 2023 17:17:03 +0800	[thread overview]
Message-ID: <CAKHBV25K4BCewMdp3HcRtaX1iNhVpxL_6dMwp1_fmcQ5RWpKBQ@mail.gmail.com> (raw)
In-Reply-To: <ZIBxPd1/JCAle6yP@nvidia.com>

On Wed, Jun 7, 2023 at 1:09 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Tue, Jun 06, 2023 at 08:07:50PM +0800, Michael Shavit wrote:
> > SVA may attach a CD to masters that have different upstream SMMU
> > devices. The arm_smmu_domain structure can only be attached to a single
> > upstream SMMU device however.
>
> Isn't that pretty much because we don't support replicating
> invalidations to each of the different SMMU instances?

Looked into this some more, and supporting attach to multiple devices
is still very hard:
1. When an arm_smmu_domain is first attached to a master, it
initializes an io_pgtable_cfg object whose properties depend on the
master's upstream SMMU device. This io_pgtable_cfg is then used to
allocate an io_pgtable object, and the arm_smmu_ctx_desc's TTBR field
points to that io_pgtable's TTBR (and ditto for the vttbr on stage 2
domains). So then arm_smmu_domain needs to be split into two,
arm_smmu_domain and arm_smmu_device_domain with the latter containing
a per-SMMU device io_pgtable, arm_smmu_ctx_desc and arm_smmu_s2_cfg.
Each iommu_domain_ops operation now needs to loop over each
arm_smmu_device_domain.
2. Some of the iommu_domain fields also depend on the per-SMMU
io_pgtable_cfg; specifically pgsize_bitmap and geometry.aperture_end.
These need to be restricted as the domain is attached to more devices.
3. Attaching a domain to a new SMMU device must be prohibited after
any call to map_pages or if iommu_domain.pgsize_bitmap and
iommu-domain.geometry.aperture_end have been consumed by any system.
The first is something the arm-smmu-v3.c driver could feasibly enforce
on its own, the second requires changes to the iommu framework.

The arm-smmu-v3-sva.c implementation avoids all these problems because it
doesn't need to allocate an io_pgtable; the shared arm_smmu_ctx_desc's
TTBR is set to the MM's TTBR. One possible solution would be to make
the changes proposed in 1., but only allowing SVA domains to attach to
multiple devices:
1. Domain functions only accessed by arm-smmu-v3.c (such as
arm_smmu_iova_to_phys) can assume there's a single
arm_smmu_device_domain per arm_smmu_domain.
2. Domain functions also accessed by arm-smmu-v3-sva.c (such as
invalidations commands) must iterate over all arm_smmu_device_domains.

> On Wed, Jun 7, 2023 at 8:00 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> I think splitting it into a series to re-organize the way ste/cd stuff
> works is a nice contained topic.

Should I explicitly resend this patch series as a v3 cutting off
patches 14 and onwards?