[PATCH v1 0/5] Add PASID support to SMMUv3 unmanaged domains

[PATCH v1 0/5] Add PASID support to SMMUv3 unmanaged domains

[Michael Shavit]

Hi all,

This patch series refactors the arm-smmu-v3 driver and implements the
set_dev_pasid functionality for DMA and UNMANAGED iommu domains. As part
of this effort, we also refactor the arm-smmu-v3 driver such that each
iommu domain represent a single address space. In particular, stage 1
domains hold a single ContextDescriptor instead of the entire STE entry.

The refactor is arguably valuable independently from the set_dev_pasid
feature since an iommu_domain is conceptually closer to a single address
space than an entire STE. In addition this unlocks some nice clean-up of
the arm SVA implementation which today piggybacks SVA domains on the
"primary" domain.

This patch series makes some changes to SVA and PCIe, but I haven't
tested those features. The cd table allocations could also be further
optimized for masters that don't support multiple context.  However, the
SMMUv3 Nested translation patch series has me worried about this
change. At a glance, it seems like this refactor conflicts with its
proposed uAPI. Is this refactor no longer an appropriate clean-up or
path forward for set_dev_pasid support? Or could a uAPI that only
exposes a single CD instead of the entire STE be an appropriate fit for
the nesting use cases?

Thanks,
Michael Shavit

Link: https://lore.kernel.org/all/CAKHBV24u9b-=cJCF=Kt=3B4hynOyNr6gmi0F6zpO6b1mHc0Z7g@mail.gmail.com
Link: https://lore.kernel.org/all/cover.1683688960.git.nicolinc@nvidia.com/

Michael Shavit (5):
  iommu/arm-smmu-v3: Move cdtable to arm_smmu_master
  iommu/arm-smmu-v3: Add has_stage1 field
  iommu/arm-smmu-v3: Simplify arm_smmu_enable_ats
  iommu/arm-smmu-v3: Keep track of attached ssids
  iommu/arm-smmu-v3: Implement set_dev_pasid

 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  46 +-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 432 ++++++++++++------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  44 +-
 3 files changed, 344 insertions(+), 178 deletions(-)

-- 
2.40.1.521.gf1e218fcd8-goog

[PATCH v1 1/5] iommu/arm-smmu-v3: Move cdtable to arm_smmu_master

[Michael Shavit]

In the arm-smmu-v3 driver, stage 1 domains represent the entire STE.
Iommu domains are conceptually closer to a single address space (with
some exceptions), which are better represented by a single
ContextDescriptor for stage 1 domains. With this change, the CD table is
now owned by the arm_smmu_master instead of the attached domain. Each
stage 1 domain now represents a single context.

For now, SVA still piggy-backs on the primary arm_smmu_domain for other
state management (such as the devices list).

Signed-off-by: Michael Shavit <mshavit@google.com>
---
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  35 +++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 185 ++++++++----------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  29 +--
 3 files changed, 127 insertions(+), 122 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index a5a63b1c947eb..a721461b355c6 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -35,6 +35,9 @@ struct arm_smmu_bond {
 #define sva_to_bond(handle) \
 	container_of(handle, struct arm_smmu_bond, sva)
 
+#define cd_to_domain(cd) \
+	container_of(cd, struct arm_smmu_domain, cd)
+
 static DEFINE_MUTEX(sva_lock);
 
 /*
@@ -45,10 +48,12 @@ static struct arm_smmu_ctx_desc *
 arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
 {
 	int ret;
+	unsigned long flags;
 	u32 new_asid;
 	struct arm_smmu_ctx_desc *cd;
 	struct arm_smmu_device *smmu;
 	struct arm_smmu_domain *smmu_domain;
+	struct arm_smmu_master *master;
 
 	cd = xa_load(&arm_smmu_asid_xa, asid);
 	if (!cd)
@@ -62,7 +67,7 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
 		return cd;
 	}
 
-	smmu_domain = container_of(cd, struct arm_smmu_domain, s1_cfg.cd);
+	smmu_domain = cd_to_domain(cd);
 	smmu = smmu_domain->smmu;
 
 	ret = xa_alloc(&arm_smmu_asid_xa, &new_asid, cd,
@@ -80,7 +85,11 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
 	 * be some overlap between use of both ASIDs, until we invalidate the
 	 * TLB.
 	 */
-	arm_smmu_write_ctx_desc(smmu_domain, 0, cd);
+	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+	list_for_each_entry(master, &smmu_domain->devices, domain_head) {
+		arm_smmu_write_ctx_desc(master, 0, cd);
+	}
+	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 
 	/* Invalidate TLB entries previously associated with that context */
 	arm_smmu_tlb_inv_asid(smmu, asid);
@@ -211,6 +220,8 @@ static void arm_smmu_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
 {
 	struct arm_smmu_mmu_notifier *smmu_mn = mn_to_smmu(mn);
 	struct arm_smmu_domain *smmu_domain = smmu_mn->domain;
+	struct arm_smmu_master *master;
+	unsigned long flags;
 
 	mutex_lock(&sva_lock);
 	if (smmu_mn->cleared) {
@@ -222,7 +233,11 @@ static void arm_smmu_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
 	 * DMA may still be running. Keep the cd valid to avoid C_BAD_CD events,
 	 * but disable translation.
 	 */
-	arm_smmu_write_ctx_desc(smmu_domain, mm->pasid, &quiet_cd);
+	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+	list_for_each_entry(master, &smmu_domain->devices, domain_head) {
+		arm_smmu_write_ctx_desc(master, mm->pasid, &quiet_cd);
+	}
+	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 
 	arm_smmu_tlb_inv_asid(smmu_domain->smmu, smmu_mn->cd->asid);
 	arm_smmu_atc_inv_domain(smmu_domain, mm->pasid, 0, 0);
@@ -244,7 +259,8 @@ static const struct mmu_notifier_ops arm_smmu_mmu_notifier_ops = {
 
 /* Allocate or get existing MMU notifier for this {domain, mm} pair */
 static struct arm_smmu_mmu_notifier *
-arm_smmu_mmu_notifier_get(struct arm_smmu_domain *smmu_domain,
+arm_smmu_mmu_notifier_get(struct arm_smmu_master *master,
+			  struct arm_smmu_domain *smmu_domain,
 			  struct mm_struct *mm)
 {
 	int ret;
@@ -279,7 +295,7 @@ arm_smmu_mmu_notifier_get(struct arm_smmu_domain *smmu_domain,
 		goto err_free_cd;
 	}
 
-	ret = arm_smmu_write_ctx_desc(smmu_domain, mm->pasid, cd);
+	ret = arm_smmu_write_ctx_desc(master, mm->pasid, cd);
 	if (ret)
 		goto err_put_notifier;
 
@@ -294,7 +310,8 @@ arm_smmu_mmu_notifier_get(struct arm_smmu_domain *smmu_domain,
 	return ERR_PTR(ret);
 }
 
-static void arm_smmu_mmu_notifier_put(struct arm_smmu_mmu_notifier *smmu_mn)
+static void arm_smmu_mmu_notifier_put(struct arm_smmu_master *master,
+				      struct arm_smmu_mmu_notifier *smmu_mn)
 {
 	struct mm_struct *mm = smmu_mn->mn.mm;
 	struct arm_smmu_ctx_desc *cd = smmu_mn->cd;
@@ -304,7 +321,7 @@ static void arm_smmu_mmu_notifier_put(struct arm_smmu_mmu_notifier *smmu_mn)
 		return;
 
 	list_del(&smmu_mn->list);
-	arm_smmu_write_ctx_desc(smmu_domain, mm->pasid, NULL);
+	arm_smmu_write_ctx_desc(master, mm->pasid, NULL);
 
 	/*
 	 * If we went through clear(), we've already invalidated, and no
@@ -348,7 +365,7 @@ __arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm)
 	bond->sva.dev = dev;
 	refcount_set(&bond->refs, 1);
 
-	bond->smmu_mn = arm_smmu_mmu_notifier_get(smmu_domain, mm);
+	bond->smmu_mn = arm_smmu_mmu_notifier_get(master, smmu_domain, mm);
 	if (IS_ERR(bond->smmu_mn)) {
 		ret = PTR_ERR(bond->smmu_mn);
 		goto err_free_bond;
@@ -527,7 +544,7 @@ void arm_smmu_sva_remove_dev_pasid(struct iommu_domain *domain,
 
 	if (!WARN_ON(!bond) && refcount_dec_and_test(&bond->refs)) {
 		list_del(&bond->list);
-		arm_smmu_mmu_notifier_put(bond->smmu_mn);
+		arm_smmu_mmu_notifier_put(master, bond->smmu_mn);
 		kfree(bond);
 	}
 	mutex_unlock(&sva_lock);
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 3fd83fb757227..cee3efff3c9fa 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -965,14 +965,12 @@ void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid)
 	arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
 }
 
-static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
+static void arm_smmu_sync_cd(struct arm_smmu_master *master,
 			     int ssid, bool leaf)
 {
 	size_t i;
-	unsigned long flags;
-	struct arm_smmu_master *master;
 	struct arm_smmu_cmdq_batch cmds;
-	struct arm_smmu_device *smmu = smmu_domain->smmu;
+	struct arm_smmu_device *smmu = master->smmu;
 	struct arm_smmu_cmdq_ent cmd = {
 		.opcode	= CMDQ_OP_CFGI_CD,
 		.cfgi	= {
@@ -981,16 +979,17 @@ static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
 		},
 	};
 
-	cmds.num = 0;
+	/*
+	 * There's nothing to sync if the STE isn't valid yet.
+	 */
+	if (!master->domain)
+		return;
 
-	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
-	list_for_each_entry(master, &smmu_domain->devices, domain_head) {
-		for (i = 0; i < master->num_streams; i++) {
-			cmd.cfgi.sid = master->streams[i].id;
-			arm_smmu_cmdq_batch_add(smmu, &cmds, &cmd);
-		}
+	cmds.num = 0;
+	for (i = 0; i < master->num_streams; i++) {
+		cmd.cfgi.sid = master->streams[i].id;
+		arm_smmu_cmdq_batch_add(smmu, &cmds, &cmd);
 	}
-	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 
 	arm_smmu_cmdq_batch_submit(smmu, &cmds);
 }
@@ -1020,16 +1019,16 @@ static void arm_smmu_write_cd_l1_desc(__le64 *dst,
 	WRITE_ONCE(*dst, cpu_to_le64(val));
 }
 
-static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
+static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_master *master,
 				   u32 ssid)
 {
 	__le64 *l1ptr;
 	unsigned int idx;
 	struct arm_smmu_l1_ctx_desc *l1_desc;
-	struct arm_smmu_device *smmu = smmu_domain->smmu;
-	struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
+	struct arm_smmu_device *smmu = master->smmu;
+	struct arm_smmu_ctx_desc_cfg *cdcfg = &master->s1_cfg.cdcfg;
 
-	if (smmu_domain->s1_cfg.s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
+	if (master->s1_cfg.s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
 		return cdcfg->cdtab + ssid * CTXDESC_CD_DWORDS;
 
 	idx = ssid >> CTXDESC_SPLIT;
@@ -1041,13 +1040,13 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
 		l1ptr = cdcfg->cdtab + idx * CTXDESC_L1_DESC_DWORDS;
 		arm_smmu_write_cd_l1_desc(l1ptr, l1_desc);
 		/* An invalid L1CD can be cached */
-		arm_smmu_sync_cd(smmu_domain, ssid, false);
+		arm_smmu_sync_cd(master, ssid, false);
 	}
 	idx = ssid & (CTXDESC_L2_ENTRIES - 1);
 	return l1_desc->l2ptr + idx * CTXDESC_CD_DWORDS;
 }
 
-int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
+int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
 			    struct arm_smmu_ctx_desc *cd)
 {
 	/*
@@ -1059,16 +1058,16 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
 	 *     CD, then invalidate the old entry and mappings.
 	 * (4) Quiesce the context without clearing the valid bit. Disable
 	 *     translation, and ignore any translation fault.
-	 * (5) Remove a secondary CD.
+	 * (5) Remove a CD.
 	 */
 	u64 val;
 	bool cd_live;
 	__le64 *cdptr;
 
-	if (WARN_ON(ssid >= (1 << smmu_domain->s1_cfg.s1cdmax)))
+	if (WARN_ON(ssid >= (1 << master->s1_cfg.s1cdmax)))
 		return -E2BIG;
 
-	cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid);
+	cdptr = arm_smmu_get_cd_ptr(master, ssid);
 	if (!cdptr)
 		return -ENOMEM;
 
@@ -1096,7 +1095,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
 		 * order. Ensure that it observes valid values before reading
 		 * V=1.
 		 */
-		arm_smmu_sync_cd(smmu_domain, ssid, true);
+		arm_smmu_sync_cd(master, ssid, true);
 
 		val = cd->tcr |
 #ifdef __BIG_ENDIAN
@@ -1108,7 +1107,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
 			FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
 			CTXDESC_CD_0_V;
 
-		if (smmu_domain->stall_enabled)
+		if (master->stall_enabled)
 			val |= CTXDESC_CD_0_S;
 	}
 
@@ -1122,19 +1121,20 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
 	 *   without first making the structure invalid.
 	 */
 	WRITE_ONCE(cdptr[0], cpu_to_le64(val));
-	arm_smmu_sync_cd(smmu_domain, ssid, true);
+	arm_smmu_sync_cd(master, ssid, true);
 	return 0;
 }
 
-static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
+static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master)
 {
 	int ret;
 	size_t l1size;
 	size_t max_contexts;
-	struct arm_smmu_device *smmu = smmu_domain->smmu;
-	struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+	struct arm_smmu_device *smmu = master->smmu;
+	struct arm_smmu_s1_cfg *cfg = &master->s1_cfg;
 	struct arm_smmu_ctx_desc_cfg *cdcfg = &cfg->cdcfg;
 
+	cfg->s1cdmax = master->ssid_bits;
 	max_contexts = 1 << cfg->s1cdmax;
 
 	if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB) ||
@@ -1175,12 +1175,12 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
 	return ret;
 }
 
-static void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain)
+static void arm_smmu_free_cd_tables(struct arm_smmu_master *master)
 {
 	int i;
 	size_t size, l1size;
-	struct arm_smmu_device *smmu = smmu_domain->smmu;
-	struct arm_smmu_ctx_desc_cfg *cdcfg = &smmu_domain->s1_cfg.cdcfg;
+	struct arm_smmu_device *smmu = master->smmu;
+	struct arm_smmu_ctx_desc_cfg *cdcfg = &master->s1_cfg.cdcfg;
 
 	if (cdcfg->l1_desc) {
 		size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3);
@@ -1272,30 +1272,25 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
 	struct arm_smmu_device *smmu = NULL;
 	struct arm_smmu_s1_cfg *s1_cfg = NULL;
 	struct arm_smmu_s2_cfg *s2_cfg = NULL;
-	struct arm_smmu_domain *smmu_domain = NULL;
 	struct arm_smmu_cmdq_ent prefetch_cmd = {
 		.opcode		= CMDQ_OP_PREFETCH_CFG,
 		.prefetch	= {
 			.sid	= sid,
 		},
 	};
+	struct iommu_domain *domain = NULL;
 
 	if (master) {
-		smmu_domain = master->domain;
 		smmu = master->smmu;
-	}
-
-	if (smmu_domain) {
-		switch (smmu_domain->stage) {
-		case ARM_SMMU_DOMAIN_S1:
-			s1_cfg = &smmu_domain->s1_cfg;
-			break;
-		case ARM_SMMU_DOMAIN_S2:
-		case ARM_SMMU_DOMAIN_NESTED:
-			s2_cfg = &smmu_domain->s2_cfg;
-			break;
-		default:
-			break;
+		if (master->domain)
+			domain = &master->domain->domain;
+	}
+	if (domain) {
+		if (domain->type != IOMMU_DOMAIN_IDENTITY) {
+			if (master->s2_cfg)
+				s2_cfg = master->s2_cfg;
+			else
+				s1_cfg = &master->s1_cfg;
 		}
 	}
 
@@ -1319,8 +1314,8 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
 	val = STRTAB_STE_0_V;
 
 	/* Bypass/fault */
-	if (!smmu_domain || !(s1_cfg || s2_cfg)) {
-		if (!smmu_domain && disable_bypass)
+	if (!(s1_cfg || s2_cfg)) {
+		if (disable_bypass)
 			val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT);
 		else
 			val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS);
@@ -1863,7 +1858,7 @@ static void arm_smmu_tlb_inv_context(void *cookie)
 	 * careful, 007.
 	 */
 	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
-		arm_smmu_tlb_inv_asid(smmu, smmu_domain->s1_cfg.cd.asid);
+		arm_smmu_tlb_inv_asid(smmu, smmu_domain->cd.asid);
 	} else {
 		cmd.opcode	= CMDQ_OP_TLBI_S12_VMALL;
 		cmd.tlbi.vmid	= smmu_domain->s2_cfg.vmid;
@@ -1946,7 +1941,7 @@ static void arm_smmu_tlb_inv_range_domain(unsigned long iova, size_t size,
 	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
 		cmd.opcode	= smmu_domain->smmu->features & ARM_SMMU_FEAT_E2H ?
 				  CMDQ_OP_TLBI_EL2_VA : CMDQ_OP_TLBI_NH_VA;
-		cmd.tlbi.asid	= smmu_domain->s1_cfg.cd.asid;
+		cmd.tlbi.asid	= smmu_domain->cd.asid;
 	} else {
 		cmd.opcode	= CMDQ_OP_TLBI_S2_IPA;
 		cmd.tlbi.vmid	= smmu_domain->s2_cfg.vmid;
@@ -2071,13 +2066,9 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
 
 	/* Free the CD and ASID, if we allocated them */
 	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
-		struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
-
 		/* Prevent SVA from touching the CD while we're freeing it */
 		mutex_lock(&arm_smmu_asid_lock);
-		if (cfg->cdcfg.cdtab)
-			arm_smmu_free_cd_tables(smmu_domain);
-		arm_smmu_free_asid(&cfg->cd);
+		arm_smmu_free_asid(&smmu_domain->cd);
 		mutex_unlock(&arm_smmu_asid_lock);
 	} else {
 		struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
@@ -2088,67 +2079,45 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
 	kfree(smmu_domain);
 }
 
-static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
-				       struct arm_smmu_master *master,
+static int arm_smmu_domain_finalise_cd(struct arm_smmu_domain *smmu_domain,
 				       struct io_pgtable_cfg *pgtbl_cfg)
 {
 	int ret;
 	u32 asid;
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
-	struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+	struct arm_smmu_ctx_desc *cd = &smmu_domain->cd;
 	typeof(&pgtbl_cfg->arm_lpae_s1_cfg.tcr) tcr = &pgtbl_cfg->arm_lpae_s1_cfg.tcr;
 
-	refcount_set(&cfg->cd.refs, 1);
+	refcount_set(&cd->refs, 1);
 
 	/* Prevent SVA from modifying the ASID until it is written to the CD */
 	mutex_lock(&arm_smmu_asid_lock);
-	ret = xa_alloc(&arm_smmu_asid_xa, &asid, &cfg->cd,
+	ret = xa_alloc(&arm_smmu_asid_xa, &asid, cd,
 		       XA_LIMIT(1, (1 << smmu->asid_bits) - 1), GFP_KERNEL);
 	if (ret)
 		goto out_unlock;
 
-	cfg->s1cdmax = master->ssid_bits;
-
-	smmu_domain->stall_enabled = master->stall_enabled;
 
-	ret = arm_smmu_alloc_cd_tables(smmu_domain);
-	if (ret)
-		goto out_free_asid;
-
-	cfg->cd.asid	= (u16)asid;
-	cfg->cd.ttbr	= pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
-	cfg->cd.tcr	= FIELD_PREP(CTXDESC_CD_0_TCR_T0SZ, tcr->tsz) |
+	cd->asid	= (u16)asid;
+	cd->ttbr	= pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
+	cd->tcr		= FIELD_PREP(CTXDESC_CD_0_TCR_T0SZ, tcr->tsz) |
 			  FIELD_PREP(CTXDESC_CD_0_TCR_TG0, tcr->tg) |
 			  FIELD_PREP(CTXDESC_CD_0_TCR_IRGN0, tcr->irgn) |
 			  FIELD_PREP(CTXDESC_CD_0_TCR_ORGN0, tcr->orgn) |
 			  FIELD_PREP(CTXDESC_CD_0_TCR_SH0, tcr->sh) |
 			  FIELD_PREP(CTXDESC_CD_0_TCR_IPS, tcr->ips) |
 			  CTXDESC_CD_0_TCR_EPD1 | CTXDESC_CD_0_AA64;
-	cfg->cd.mair	= pgtbl_cfg->arm_lpae_s1_cfg.mair;
-
-	/*
-	 * Note that this will end up calling arm_smmu_sync_cd() before
-	 * the master has been added to the devices list for this domain.
-	 * This isn't an issue because the STE hasn't been installed yet.
-	 */
-	ret = arm_smmu_write_ctx_desc(smmu_domain, 0, &cfg->cd);
-	if (ret)
-		goto out_free_cd_tables;
+	cd->mair	= pgtbl_cfg->arm_lpae_s1_cfg.mair;
 
 	mutex_unlock(&arm_smmu_asid_lock);
 	return 0;
 
-out_free_cd_tables:
-	arm_smmu_free_cd_tables(smmu_domain);
-out_free_asid:
-	arm_smmu_free_asid(&cfg->cd);
 out_unlock:
 	mutex_unlock(&arm_smmu_asid_lock);
 	return ret;
 }
 
 static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,
-				       struct arm_smmu_master *master,
 				       struct io_pgtable_cfg *pgtbl_cfg)
 {
 	int vmid;
@@ -2173,8 +2142,7 @@ static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,
 	return 0;
 }
 
-static int arm_smmu_domain_finalise(struct iommu_domain *domain,
-				    struct arm_smmu_master *master)
+static int arm_smmu_domain_finalise(struct iommu_domain *domain)
 {
 	int ret;
 	unsigned long ias, oas;
@@ -2182,7 +2150,6 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
 	struct io_pgtable_cfg pgtbl_cfg;
 	struct io_pgtable_ops *pgtbl_ops;
 	int (*finalise_stage_fn)(struct arm_smmu_domain *,
-				 struct arm_smmu_master *,
 				 struct io_pgtable_cfg *);
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
@@ -2204,7 +2171,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
 		ias = min_t(unsigned long, ias, VA_BITS);
 		oas = smmu->ias;
 		fmt = ARM_64_LPAE_S1;
-		finalise_stage_fn = arm_smmu_domain_finalise_s1;
+		finalise_stage_fn = arm_smmu_domain_finalise_cd;
 		break;
 	case ARM_SMMU_DOMAIN_NESTED:
 	case ARM_SMMU_DOMAIN_S2:
@@ -2234,7 +2201,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
 	domain->geometry.aperture_end = (1UL << pgtbl_cfg.ias) - 1;
 	domain->geometry.force_aperture = true;
 
-	ret = finalise_stage_fn(smmu_domain, master, &pgtbl_cfg);
+	ret = finalise_stage_fn(smmu_domain, &pgtbl_cfg);
 	if (ret < 0) {
 		free_io_pgtable_ops(pgtbl_ops);
 		return ret;
@@ -2402,6 +2369,13 @@ static void arm_smmu_detach_dev(struct arm_smmu_master *master)
 
 	master->domain = NULL;
 	master->ats_enabled = false;
+	master->s2_cfg = NULL;
+	/*
+	 * Note that this will end up calling arm_smmu_sync_cd() even though
+	 * we're about to destroy the entire STE anyways. This is ok because
+	 * arm_smmu_sync_cd will exit early now that we've set domain to NULL;
+	 */
+	arm_smmu_write_ctx_desc(master, 0, NULL);
 	arm_smmu_install_ste_for_dev(master);
 }
 
@@ -2436,7 +2410,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 
 	if (!smmu_domain->smmu) {
 		smmu_domain->smmu = smmu;
-		ret = arm_smmu_domain_finalise(domain, master);
+		ret = arm_smmu_domain_finalise(domain);
 		if (ret) {
 			smmu_domain->smmu = NULL;
 			goto out_unlock;
@@ -2444,17 +2418,21 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	} else if (smmu_domain->smmu != smmu) {
 		ret = -EINVAL;
 		goto out_unlock;
-	} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
-		   master->ssid_bits != smmu_domain->s1_cfg.s1cdmax) {
-		ret = -EINVAL;
-		goto out_unlock;
-	} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
-		   smmu_domain->stall_enabled != master->stall_enabled) {
-		ret = -EINVAL;
-		goto out_unlock;
 	}
 
-	master->domain = smmu_domain;
+	/*
+	 * Note that this will end up calling arm_smmu_sync_cd() before
+	 * the master has been added to the devices list for this domain.
+	 * This isn't an issue because the STE hasn't been installed yet.
+	 */
+	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
+		ret = arm_smmu_write_ctx_desc(master, 0, &smmu_domain->cd);
+		if (ret)
+			goto out_unlock;
+	} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S2 ||
+		   smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED) {
+		master->s2_cfg = &smmu_domain->s2_cfg;
+	}
 
 	/*
 	 * The SMMU does not support enabling ATS with bypass. When the STE is
@@ -2466,6 +2444,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	if (smmu_domain->stage != ARM_SMMU_DOMAIN_BYPASS)
 		master->ats_enabled = arm_smmu_ats_supported(master);
 
+	master->domain = smmu_domain;
 	arm_smmu_install_ste_for_dev(master);
 
 	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
@@ -2703,6 +2682,13 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
 	    smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
 		master->stall_enabled = true;
 
+	ret = arm_smmu_alloc_cd_tables(master);
+	if (ret) {
+		arm_smmu_disable_pasid(master);
+		arm_smmu_remove_master(master);
+		goto err_free_master;
+	}
+
 	return &smmu->iommu;
 
 err_free_master:
@@ -2718,6 +2704,7 @@ static void arm_smmu_release_device(struct device *dev)
 	if (WARN_ON(arm_smmu_master_sva_enabled(master)))
 		iopf_queue_remove_device(master->smmu->evtq.iopf, dev);
 	arm_smmu_detach_dev(master);
+	arm_smmu_free_cd_tables(master);
 	arm_smmu_disable_pasid(master);
 	arm_smmu_remove_master(master);
 	kfree(master);
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index b574c58a34876..0b87c74bdf46e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -593,7 +593,6 @@ struct arm_smmu_ctx_desc_cfg {
 
 struct arm_smmu_s1_cfg {
 	struct arm_smmu_ctx_desc_cfg	cdcfg;
-	struct arm_smmu_ctx_desc	cd;
 	u8				s1fmt;
 	u8				s1cdmax;
 };
@@ -689,6 +688,8 @@ struct arm_smmu_master {
 	struct arm_smmu_domain		*domain;
 	struct list_head		domain_head;
 	struct arm_smmu_stream		*streams;
+	struct arm_smmu_s1_cfg		s1_cfg;
+	struct arm_smmu_s2_cfg		*s2_cfg;
 	unsigned int			num_streams;
 	bool				ats_enabled;
 	bool				stall_enabled;
@@ -707,25 +708,25 @@ enum arm_smmu_domain_stage {
 };
 
 struct arm_smmu_domain {
-	struct arm_smmu_device		*smmu;
-	struct mutex			init_mutex; /* Protects smmu pointer */
+	struct arm_smmu_device			*smmu;
+	 /* Protects smmu pointer */
+	struct mutex				init_mutex;
 
-	struct io_pgtable_ops		*pgtbl_ops;
-	bool				stall_enabled;
-	atomic_t			nr_ats_masters;
+	struct io_pgtable_ops			*pgtbl_ops;
+	atomic_t				nr_ats_masters;
 
-	enum arm_smmu_domain_stage	stage;
+	enum arm_smmu_domain_stage		stage;
 	union {
-		struct arm_smmu_s1_cfg	s1_cfg;
-		struct arm_smmu_s2_cfg	s2_cfg;
+		struct arm_smmu_ctx_desc	cd;
+		struct arm_smmu_s2_cfg		s2_cfg;
 	};
 
-	struct iommu_domain		domain;
+	struct iommu_domain			domain;
 
-	struct list_head		devices;
-	spinlock_t			devices_lock;
+	struct list_head			devices;
+	spinlock_t				devices_lock;
 
-	struct list_head		mmu_notifiers;
+	struct list_head			mmu_notifiers;
 };
 
 static inline struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
@@ -737,7 +738,7 @@ extern struct xarray arm_smmu_asid_xa;
 extern struct mutex arm_smmu_asid_lock;
 extern struct arm_smmu_ctx_desc quiet_cd;
 
-int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
+int arm_smmu_write_ctx_desc(struct arm_smmu_master *master, int ssid,
 			    struct arm_smmu_ctx_desc *cd);
 void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid);
 void arm_smmu_tlb_inv_range_asid(unsigned long iova, size_t size, int asid,
-- 
2.40.1.521.gf1e218fcd8-goog

Re: [PATCH v1 1/5] iommu/arm-smmu-v3: Move cdtable to arm_smmu_master

[Jason Gunthorpe]

On Thu, May 11, 2023 at 04:50:48AM +0800, Michael Shavit wrote:
> In the arm-smmu-v3 driver, stage 1 domains represent the entire STE.
> Iommu domains are conceptually closer to a single address space (with
> some exceptions), which are better represented by a single
> ContextDescriptor for stage 1 domains. With this change, the CD table is
> now owned by the arm_smmu_master instead of the attached domain. Each
> stage 1 domain now represents a single context.

As per my remark on the cover letter this seems a bit confusing
description, but the idea that the CD Table for kernel owned PASID
list is placed in the 'struct arm_smmu_master' does make sense to me

This patch seems very big, is there any way to break it into smaller
steps?
 
Jason

Re: [PATCH v1 1/5] iommu/arm-smmu-v3: Move cdtable to arm_smmu_master

[Michael Shavit]

On Thu, May 11, 2023 at 5:15 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
> As per my remark on the cover letter this seems a bit confusing
> description, but the idea that the CD Table for kernel owned PASID
> list is placed in the 'struct arm_smmu_master' does make sense to me

Ack; glad to hear the idea is well received. I'll rephrase commit
messages for v2.

> This patch seems very big, is there any way to break it into smaller
> steps?

I can add a preparatory patch that defines and allocates the
arm_smmu_master's cd table before this patch starts writing to that cd
table instead of the smmu_domain owned cd table. This doesn't cut down
the patch's size by a significant amount but it's a start. I'll try to
think about it some more but a lot of the other changes in this patch
are pretty tightly coupled.

[PATCH v1 2/5] iommu/arm-smmu-v3: Add has_stage1 field

[Michael Shavit]

Inferring the state of the STE based on attached domains becomes tricker
when multiple domains can be attached to a master on different PASIDs.
The new field allows the smmu driver to directly query the state of the
STE (S1 present, S2 present, neither) instead of inferring it from the
attached domain.

Signed-off-by: Michael Shavit <mshavit@google.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 22 +++++++--------------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index cee3efff3c9fa..7b4399b5ba68b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -980,9 +980,9 @@ static void arm_smmu_sync_cd(struct arm_smmu_master *master,
 	};
 
 	/*
-	 * There's nothing to sync if the STE isn't valid yet.
+	 * There's nothing to sync if stage 1 hasn't been installed yet.
 	 */
-	if (!master->domain)
+	if (!master->has_stage1)
 		return;
 
 	cmds.num = 0;
@@ -1278,20 +1278,11 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
 			.sid	= sid,
 		},
 	};
-	struct iommu_domain *domain = NULL;
-
 	if (master) {
 		smmu = master->smmu;
-		if (master->domain)
-			domain = &master->domain->domain;
-	}
-	if (domain) {
-		if (domain->type != IOMMU_DOMAIN_IDENTITY) {
-			if (master->s2_cfg)
-				s2_cfg = master->s2_cfg;
-			else
-				s1_cfg = &master->s1_cfg;
-		}
+		if (master->has_stage1)
+			s1_cfg = &master->s1_cfg;
+		s2_cfg = master->s2_cfg;
 	}
 
 	if (val & STRTAB_STE_0_V) {
@@ -2367,9 +2358,9 @@ static void arm_smmu_detach_dev(struct arm_smmu_master *master)
 	list_del(&master->domain_head);
 	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 
-	master->domain = NULL;
 	master->ats_enabled = false;
 	master->s2_cfg = NULL;
+	master->has_stage1 = false;
 	/*
 	 * Note that this will end up calling arm_smmu_sync_cd() even though
 	 * we're about to destroy the entire STE anyways. This is ok because
@@ -2426,6 +2417,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	 * This isn't an issue because the STE hasn't been installed yet.
 	 */
 	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
+		master->has_stage1 = true;
 		ret = arm_smmu_write_ctx_desc(master, 0, &smmu_domain->cd);
 		if (ret)
 			goto out_unlock;
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 0b87c74bdf46e..d715794572b13 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -689,6 +689,7 @@ struct arm_smmu_master {
 	struct list_head		domain_head;
 	struct arm_smmu_stream		*streams;
 	struct arm_smmu_s1_cfg		s1_cfg;
+	bool                            has_stage1;
 	struct arm_smmu_s2_cfg		*s2_cfg;
 	unsigned int			num_streams;
 	bool				ats_enabled;
-- 
2.40.1.521.gf1e218fcd8-goog

[PATCH v1 3/5] iommu/arm-smmu-v3: Simplify arm_smmu_enable_ats

[Michael Shavit]

arm_smmu_enable_ats's call to inv_domain would trigger an invalidation
for all masters that a domain is attached to everytime it's attached to
another ATS-enabled master. It doesn't seem like those invalidations are
necessary, and it's easier to reason about arm_smmu_enable_ats if it only
issues invalidation commands for the current master.

Signed-off-by: Michael Shavit <mshavit@google.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 7b4399b5ba68b..47dda287a4736 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2275,7 +2275,7 @@ static void arm_smmu_enable_ats(struct arm_smmu_master *master)
 	pdev = to_pci_dev(master->dev);
 
 	atomic_inc(&smmu_domain->nr_ats_masters);
-	arm_smmu_atc_inv_domain(smmu_domain, 0, 0, 0);
+	arm_smmu_atc_inv_master(master);
 	if (pci_enable_ats(pdev, stu))
 		dev_err(master->dev, "Failed to enable ATS (STU %zu)\n", stu);
 }
-- 
2.40.1.521.gf1e218fcd8-goog

[PATCH v1 4/5] iommu/arm-smmu-v3: Keep track of attached ssids

[Michael Shavit]

The arm-smmu-v3 driver keeps track of all masters that a domain is
attached so that it can re-write their STEs when the domain's ASID is
upated by SVA. This tracking is also used to invalidate ATCs on all
masters that a domain is attached.

This change introduces a new data structures to track all the CD entries
that a domain is attached to. This change is a pre-requisite to allow
domain attachment on non 0 SSIDs.

Signed-off-by: Michael Shavit <mshavit@google.com>
---
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   | 31 ++++---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 90 ++++++++++++-------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   | 17 ++--
 3 files changed, 89 insertions(+), 49 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index a721461b355c6..2eb066c0f3f99 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -53,7 +53,7 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
 	struct arm_smmu_ctx_desc *cd;
 	struct arm_smmu_device *smmu;
 	struct arm_smmu_domain *smmu_domain;
-	struct arm_smmu_master *master;
+	struct arm_smmu_attached_domain *attached_domain;
 
 	cd = xa_load(&arm_smmu_asid_xa, asid);
 	if (!cd)
@@ -85,11 +85,11 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
 	 * be some overlap between use of both ASIDs, until we invalidate the
 	 * TLB.
 	 */
-	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
-	list_for_each_entry(master, &smmu_domain->devices, domain_head) {
-		arm_smmu_write_ctx_desc(master, 0, cd);
+	spin_lock_irqsave(&smmu_domain->attached_domains_lock, flags);
+	list_for_each_entry(attached_domain, &smmu_domain->attached_domains, domain_head) {
+		arm_smmu_write_ctx_desc(attached_domain->master, attached_domain->ssid, cd);
 	}
-	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+	spin_unlock_irqrestore(&smmu_domain->attached_domains_lock, flags);
 
 	/* Invalidate TLB entries previously associated with that context */
 	arm_smmu_tlb_inv_asid(smmu, asid);
@@ -213,14 +213,14 @@ static void arm_smmu_mm_invalidate_range(struct mmu_notifier *mn,
 	if (!(smmu_domain->smmu->features & ARM_SMMU_FEAT_BTM))
 		arm_smmu_tlb_inv_range_asid(start, size, smmu_mn->cd->asid,
 					    PAGE_SIZE, false, smmu_domain);
-	arm_smmu_atc_inv_domain(smmu_domain, mm->pasid, start, size);
+	arm_smmu_atc_inv_domain_ssid(smmu_domain, mm->pasid, start, size);
 }
 
 static void arm_smmu_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
 {
 	struct arm_smmu_mmu_notifier *smmu_mn = mn_to_smmu(mn);
 	struct arm_smmu_domain *smmu_domain = smmu_mn->domain;
-	struct arm_smmu_master *master;
+	struct arm_smmu_attached_domain *attached_domain;
 	unsigned long flags;
 
 	mutex_lock(&sva_lock);
@@ -233,14 +233,19 @@ static void arm_smmu_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
 	 * DMA may still be running. Keep the cd valid to avoid C_BAD_CD events,
 	 * but disable translation.
 	 */
-	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
-	list_for_each_entry(master, &smmu_domain->devices, domain_head) {
-		arm_smmu_write_ctx_desc(master, mm->pasid, &quiet_cd);
+	spin_lock_irqsave(&smmu_domain->attached_domains_lock, flags);
+	list_for_each_entry(attached_domain, &smmu_domain->attached_domains, domain_head) {
+		/*
+		 * SVA domains piggyback on the attached_domain with SSID 0.
+		 */
+		if (attached_domain->ssid == 0)
+			arm_smmu_write_ctx_desc(attached_domain->master,
+						mm->pasid, &quiet_cd);
 	}
-	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+	spin_unlock_irqrestore(&smmu_domain->attached_domains_lock, flags);
 
 	arm_smmu_tlb_inv_asid(smmu_domain->smmu, smmu_mn->cd->asid);
-	arm_smmu_atc_inv_domain(smmu_domain, mm->pasid, 0, 0);
+	arm_smmu_atc_inv_domain_ssid(smmu_domain, mm->pasid, 0, 0);
 
 	smmu_mn->cleared = true;
 	mutex_unlock(&sva_lock);
@@ -329,7 +334,7 @@ static void arm_smmu_mmu_notifier_put(struct arm_smmu_master *master,
 	 */
 	if (!smmu_mn->cleared) {
 		arm_smmu_tlb_inv_asid(smmu_domain->smmu, cd->asid);
-		arm_smmu_atc_inv_domain(smmu_domain, mm->pasid, 0, 0);
+		arm_smmu_atc_inv_domain_ssid(smmu_domain, mm->pasid, 0, 0);
 	}
 
 	/* Frees smmu_mn */
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 47dda287a4736..81f49a86c1266 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1703,7 +1703,16 @@ static irqreturn_t arm_smmu_combined_irq_handler(int irq, void *dev)
 }
 
 static void
-arm_smmu_atc_inv_to_cmd(int ssid, unsigned long iova, size_t size,
+arm_smmu_atc_inv_cmd_set_ssid(int ssid, struct arm_smmu_cmdq_ent *cmd)
+{
+	*cmd = (struct arm_smmu_cmdq_ent) {
+		.substream_valid	= !!ssid,
+		.atc.ssid		= ssid,
+	};
+}
+
+static void
+arm_smmu_atc_inv_to_cmd(unsigned long iova, size_t size,
 			struct arm_smmu_cmdq_ent *cmd)
 {
 	size_t log2_span;
@@ -1728,8 +1737,8 @@ arm_smmu_atc_inv_to_cmd(int ssid, unsigned long iova, size_t size,
 	 */
 	*cmd = (struct arm_smmu_cmdq_ent) {
 		.opcode			= CMDQ_OP_ATC_INV,
-		.substream_valid	= !!ssid,
-		.atc.ssid		= ssid,
+		.substream_valid	= false,
+		.atc.ssid		= 0,
 	};
 
 	if (!size) {
@@ -1775,8 +1784,7 @@ static int arm_smmu_atc_inv_master(struct arm_smmu_master *master)
 	struct arm_smmu_cmdq_ent cmd;
 	struct arm_smmu_cmdq_batch cmds;
 
-	arm_smmu_atc_inv_to_cmd(0, 0, 0, &cmd);
-
+	arm_smmu_atc_inv_to_cmd(0, 0, &cmd);
 	cmds.num = 0;
 	for (i = 0; i < master->num_streams; i++) {
 		cmd.atc.sid = master->streams[i].id;
@@ -1786,13 +1794,21 @@ static int arm_smmu_atc_inv_master(struct arm_smmu_master *master)
 	return arm_smmu_cmdq_batch_submit(master->smmu, &cmds);
 }
 
-int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
-			    unsigned long iova, size_t size)
+/*
+ * If ssid is 0, the domain is invalidated on all SSIDs that it is attached to.
+ * Otherwise, the domain is specifically invalidated on the provided SSID only.
+ * This second functionality is provided specifically for SVA which wants to
+ * invalidate domains on SSIDs that aren't recorded in the master's
+ * attached_domains list.
+ */
+int arm_smmu_atc_inv_domain_ssid(struct arm_smmu_domain *smmu_domain, int ssid,
+				 unsigned long iova, size_t size)
 {
 	int i;
 	unsigned long flags;
 	struct arm_smmu_cmdq_ent cmd;
 	struct arm_smmu_master *master;
+	struct arm_smmu_attached_domain *attached_domain;
 	struct arm_smmu_cmdq_batch cmds;
 
 	if (!(smmu_domain->smmu->features & ARM_SMMU_FEAT_ATS))
@@ -1815,25 +1831,35 @@ int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
 	if (!atomic_read(&smmu_domain->nr_ats_masters))
 		return 0;
 
-	arm_smmu_atc_inv_to_cmd(ssid, iova, size, &cmd);
+	arm_smmu_atc_inv_to_cmd(iova, size, &cmd);
+	if (ssid != 0)
+		arm_smmu_atc_inv_cmd_set_ssid(ssid, &cmd);
 
 	cmds.num = 0;
 
-	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
-	list_for_each_entry(master, &smmu_domain->devices, domain_head) {
+	spin_lock_irqsave(&smmu_domain->attached_domains_lock, flags);
+	list_for_each_entry(attached_domain, &smmu_domain->attached_domains, domain_head) {
+		master = attached_domain->master;
 		if (!master->ats_enabled)
 			continue;
-
+		if (ssid == 0)
+			arm_smmu_atc_inv_cmd_set_ssid(attached_domain->ssid, &cmd);
 		for (i = 0; i < master->num_streams; i++) {
 			cmd.atc.sid = master->streams[i].id;
 			arm_smmu_cmdq_batch_add(smmu_domain->smmu, &cmds, &cmd);
 		}
 	}
-	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+	spin_unlock_irqrestore(&smmu_domain->attached_domains_lock, flags);
 
 	return arm_smmu_cmdq_batch_submit(smmu_domain->smmu, &cmds);
 }
 
+int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain,
+			    unsigned long iova, size_t size)
+{
+	return arm_smmu_atc_inv_domain_ssid(smmu_domain, 0, iova, size);
+}
+
 /* IO_PGTABLE API */
 static void arm_smmu_tlb_inv_context(void *cookie)
 {
@@ -1855,7 +1881,7 @@ static void arm_smmu_tlb_inv_context(void *cookie)
 		cmd.tlbi.vmid	= smmu_domain->s2_cfg.vmid;
 		arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
 	}
-	arm_smmu_atc_inv_domain(smmu_domain, 0, 0, 0);
+	arm_smmu_atc_inv_domain(smmu_domain, 0, 0);
 }
 
 static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,
@@ -1943,7 +1969,7 @@ static void arm_smmu_tlb_inv_range_domain(unsigned long iova, size_t size,
 	 * Unfortunately, this can't be leaf-only since we may have
 	 * zapped an entire table.
 	 */
-	arm_smmu_atc_inv_domain(smmu_domain, 0, iova, size);
+	arm_smmu_atc_inv_domain(smmu_domain, iova, size);
 }
 
 void arm_smmu_tlb_inv_range_asid(unsigned long iova, size_t size, int asid,
@@ -2023,8 +2049,8 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 		return NULL;
 
 	mutex_init(&smmu_domain->init_mutex);
-	INIT_LIST_HEAD(&smmu_domain->devices);
-	spin_lock_init(&smmu_domain->devices_lock);
+	INIT_LIST_HEAD(&smmu_domain->attached_domains);
+	spin_lock_init(&smmu_domain->attached_domains_lock);
 	INIT_LIST_HEAD(&smmu_domain->mmu_notifiers);
 
 	return &smmu_domain->domain;
@@ -2259,12 +2285,12 @@ static bool arm_smmu_ats_supported(struct arm_smmu_master *master)
 	return dev_is_pci(dev) && pci_ats_supported(to_pci_dev(dev));
 }
 
-static void arm_smmu_enable_ats(struct arm_smmu_master *master)
+static void arm_smmu_enable_ats(struct arm_smmu_master *master,
+				struct arm_smmu_domain *smmu_domain)
 {
 	size_t stu;
 	struct pci_dev *pdev;
 	struct arm_smmu_device *smmu = master->smmu;
-	struct arm_smmu_domain *smmu_domain = master->domain;
 
 	/* Don't enable ATS at the endpoint if it's not enabled in the STE */
 	if (!master->ats_enabled)
@@ -2280,10 +2306,9 @@ static void arm_smmu_enable_ats(struct arm_smmu_master *master)
 		dev_err(master->dev, "Failed to enable ATS (STU %zu)\n", stu);
 }
 
-static void arm_smmu_disable_ats(struct arm_smmu_master *master)
+static void arm_smmu_disable_ats(struct arm_smmu_master *master,
+				 struct arm_smmu_domain *smmu_domain)
 {
-	struct arm_smmu_domain *smmu_domain = master->domain;
-
 	if (!master->ats_enabled)
 		return;
 
@@ -2347,20 +2372,21 @@ static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
 static void arm_smmu_detach_dev(struct arm_smmu_master *master)
 {
 	unsigned long flags;
-	struct arm_smmu_domain *smmu_domain = master->domain;
+	struct arm_smmu_domain *smmu_domain = master->attached_domain.domain;
 
 	if (!smmu_domain)
 		return;
 
-	arm_smmu_disable_ats(master);
+	arm_smmu_disable_ats(master, smmu_domain);
 
-	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
-	list_del(&master->domain_head);
-	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+	spin_lock_irqsave(&smmu_domain->attached_domains_lock, flags);
+	list_del(&master->attached_domain.domain_head);
+	spin_unlock_irqrestore(&smmu_domain->attached_domains_lock, flags);
 
 	master->ats_enabled = false;
 	master->s2_cfg = NULL;
 	master->has_stage1 = false;
+	master->attached_domain.domain = NULL;
 	/*
 	 * Note that this will end up calling arm_smmu_sync_cd() even though
 	 * we're about to destroy the entire STE anyways. This is ok because
@@ -2436,14 +2462,16 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	if (smmu_domain->stage != ARM_SMMU_DOMAIN_BYPASS)
 		master->ats_enabled = arm_smmu_ats_supported(master);
 
-	master->domain = smmu_domain;
+	master->attached_domain.master = master;
+	master->attached_domain.domain = smmu_domain;
+	master->attached_domain.ssid = 0;
 	arm_smmu_install_ste_for_dev(master);
 
-	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
-	list_add(&master->domain_head, &smmu_domain->devices);
-	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+	spin_lock_irqsave(&smmu_domain->attached_domains_lock, flags);
+	list_add(&master->attached_domain.domain_head, &smmu_domain->attached_domains);
+	spin_unlock_irqrestore(&smmu_domain->attached_domains_lock, flags);
 
-	arm_smmu_enable_ats(master);
+	arm_smmu_enable_ats(master, smmu_domain);
 
 out_unlock:
 	mutex_unlock(&smmu_domain->init_mutex);
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index d715794572b13..35700534a0b4a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -681,11 +681,18 @@ struct arm_smmu_stream {
 	struct rb_node			node;
 };
 
+struct arm_smmu_attached_domain {
+	struct list_head domain_head;
+	struct arm_smmu_master *master;
+	int ssid;
+	struct arm_smmu_domain *domain;
+};
+
 /* SMMU private data for each master */
 struct arm_smmu_master {
 	struct arm_smmu_device		*smmu;
 	struct device			*dev;
-	struct arm_smmu_domain		*domain;
+	struct arm_smmu_attached_domain	attached_domain;
 	struct list_head		domain_head;
 	struct arm_smmu_stream		*streams;
 	struct arm_smmu_s1_cfg		s1_cfg;
@@ -724,8 +731,8 @@ struct arm_smmu_domain {
 
 	struct iommu_domain			domain;
 
-	struct list_head			devices;
-	spinlock_t				devices_lock;
+	struct list_head			attached_domains;
+	spinlock_t				attached_domains_lock;
 
 	struct list_head			mmu_notifiers;
 };
@@ -746,8 +753,8 @@ void arm_smmu_tlb_inv_range_asid(unsigned long iova, size_t size, int asid,
 				 size_t granule, bool leaf,
 				 struct arm_smmu_domain *smmu_domain);
 bool arm_smmu_free_asid(struct arm_smmu_ctx_desc *cd);
-int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid,
-			    unsigned long iova, size_t size);
+int arm_smmu_atc_inv_domain_ssid(struct arm_smmu_domain *smmu_domain, int ssid,
+				 unsigned long iova, size_t size);
 
 #ifdef CONFIG_ARM_SMMU_V3_SVA
 bool arm_smmu_sva_supported(struct arm_smmu_device *smmu);
-- 
2.40.1.521.gf1e218fcd8-goog

Re: [PATCH v1 4/5] iommu/arm-smmu-v3: Keep track of attached ssids

[Jason Gunthorpe]

On Thu, May 11, 2023 at 04:50:51AM +0800, Michael Shavit wrote:

> @@ -213,14 +213,14 @@ static void arm_smmu_mm_invalidate_range(struct mmu_notifier *mn,
>  	if (!(smmu_domain->smmu->features & ARM_SMMU_FEAT_BTM))
>  		arm_smmu_tlb_inv_range_asid(start, size, smmu_mn->cd->asid,
>  					    PAGE_SIZE, false, smmu_domain);
> -	arm_smmu_atc_inv_domain(smmu_domain, mm->pasid, start, size);
> +	arm_smmu_atc_inv_domain_ssid(smmu_domain, mm->pasid, start,
>  	size);

You should be getting rid of mm->pasid in this series as well.

When each domain keeps track of what STE/CD entries that point to it then
*ALL* invalidation should iterate over the list of pointing entires
and generate the correct invalidation for that pointer.

Eg we learn the PASID from the fact that a CD at PASID xyz is pointing
at this domain and generate an invalidation for that PASID.

mm->pasid is logically incorrect in all of this code with our
multi-attach model, it was here because this code wasn't tracking at
what as pointing at the iommu_domain.

Jason

Re: [PATCH v1 4/5] iommu/arm-smmu-v3: Keep track of attached ssids

[Michael Shavit]

> You should be getting rid of mm->pasid in this series as well.
>
> When each domain keeps track of what STE/CD entries that point to it then
> *ALL* invalidation should iterate over the list of pointing entires
> and generate the correct invalidation for that pointer.
>

Completely agree. The arm_smmu_atc_inv_domain_ssid function introduced
by this patch is a stopgap to decompose this patch from the SVA
refactor that's required to stop using ssid in these calls.
I also agree that such a refactoring probably belongs in the same
patch series. @Jean-Philippe Brucker and others: is there any way I
can about testing or at least exercising the SVA flow without physical
hardware that supports SVA?

Re: [PATCH v1 4/5] iommu/arm-smmu-v3: Keep track of attached ssids

[Jean-Philippe Brucker]

On Thu, May 11, 2023 at 11:26:48PM +0800, Michael Shavit wrote:
> > You should be getting rid of mm->pasid in this series as well.
> >
> > When each domain keeps track of what STE/CD entries that point to it then
> > *ALL* invalidation should iterate over the list of pointing entires
> > and generate the correct invalidation for that pointer.
> >
> 
> Completely agree. The arm_smmu_atc_inv_domain_ssid function introduced
> by this patch is a stopgap to decompose this patch from the SVA
> refactor that's required to stop using ssid in these calls.
> I also agree that such a refactoring probably belongs in the same
> patch series. @Jean-Philippe Brucker and others: is there any way I
> can about testing or at least exercising the SVA flow without physical
> hardware that supports SVA?

Yes, there is a model with a simple test device that supports PASID and
I/O page faults. It's not completely straightforward to setup and the
driver needs to be rewritten from scratch, but it's the best we have at
the moment.  I'd like to do something equally useful for QEMU, so we can
have proper regression tests, but that requires a lot of preliminary work
to add PASID+PRI to PCI, virtio and IOMMUs.

You'll need a kernel with the driver and a rootfs with the smmute
tool [1]; the RevC model [2] and a boot-wrapper [3]. 

  $ ${BOOTWRAPPER}/configure --host=aarch64-linux-gnu
     --with-dtb=${KERNEL}/arch/arm64/boot/dts/arm/fvp-base-revc.dts
     --with-kernel-dir=${KERNEL}
     --with-initrd=${BUILDROOT}/images/rootfs.cpio
     --with-cmdline="console=ttyAMA0"
     --enable-psci --enable-gicv3
  $ make	# produces linux-system.axf

Run the model:
  $ FVP_Base_RevC-2xAEMvA 
     -C bp.secure_memory=0
     -C 'pctl.startup=0.*.*.*'
     -C bp.refcounter.non_arch_start_at_default=1
     -C cache_state_modelled=0
     -C bp.vis.disable_visualisation=1
     -C bp.virtio_net.enabled=1
     -C bp.virtio_net.hostbridge.userNetPorts=8022=22
     -C bp.virtio_net.hostbridge.userNetworking=1
     -C pci.pci_smmuv3.mmu.SMMU_IDR0=135100351
     -C pci.pci_smmuv3.mmu.SMMU_IDR3=4116
     -C pci.pci_smmuv3.mmu.SMMU_IDR5=8389749
     -C cluster0.NUM_CORES=4
     -C cluster1.NUM_CORES=4
     -a 'cluster*.cpu*=linux-system.axf'

Then run a job using the tool. The process allocates two buffers and
passes their VA to the device (via the kernel driver). The device memcpies
one buffer to the other:

   $ smmute -u mmap
   ... Success

With smmu and iommu trace events enabled, a trace should contain
smmu_mm_invalidate and dev_fault/dev_page_response events.

It's not entirely representative of SVA flow, where an assignable device
interface is mapped into the process and the process launches jobs
directly without going through the kernel (that would now use
drivers/misc/uacce), but it does exercise IOMMU SVA: sva_bind(), device
accessing the process address space with PASID and some IOPFs, which I
think is what you're looking for. However this model doesn't have a PCI
test device so you won't be able to test ATC invalidations with PASID.

Other useful tests would be enabling lockdep (some intricate locking
between IOPF, the driver and mm), killing bound processes (-k), triggering
invalid accesses to verify TLB invalidations (-f tlb, I think). There is a
lot more to test, like thp and oom, but I don't have those in this branch.

Thanks,
Jean

[1] https://jpbrucker.net/git/linux/log/?h=sva/smmute-revc
[2] https://developer.arm.com/downloads/-/arm-ecosystem-models
[3] https://git.kernel.org/pub/scm/linux/kernel/git/mark/boot-wrapper-aarch64.git/

Re: [PATCH v1 4/5] iommu/arm-smmu-v3: Keep track of attached ssids

[Michael Shavit]

Oh nice, this is exactly what I was looking for (minus the missing ATC
inv, but that's somewhat easier to reason from code)! Thanks for the
detailed guide Jean!
I finally got around to trying it out and was able to see the page
fault followed by invalidations on this patch-series as it is. This
will be super useful to start refactoring SVA for a v2 of this patch.

On Fri, May 12, 2023 at 3:59 AM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> On Thu, May 11, 2023 at 11:26:48PM +0800, Michael Shavit wrote:
> > > You should be getting rid of mm->pasid in this series as well.
> > >
> > > When each domain keeps track of what STE/CD entries that point to it then
> > > *ALL* invalidation should iterate over the list of pointing entires
> > > and generate the correct invalidation for that pointer.
> > >
> >
> > Completely agree. The arm_smmu_atc_inv_domain_ssid function introduced
> > by this patch is a stopgap to decompose this patch from the SVA
> > refactor that's required to stop using ssid in these calls.
> > I also agree that such a refactoring probably belongs in the same
> > patch series. @Jean-Philippe Brucker and others: is there any way I
> > can about testing or at least exercising the SVA flow without physical
> > hardware that supports SVA?
>
> Yes, there is a model with a simple test device that supports PASID and
> I/O page faults. It's not completely straightforward to setup and the
> driver needs to be rewritten from scratch, but it's the best we have at
> the moment.  I'd like to do something equally useful for QEMU, so we can
> have proper regression tests, but that requires a lot of preliminary work
> to add PASID+PRI to PCI, virtio and IOMMUs.
>
> You'll need a kernel with the driver and a rootfs with the smmute
> tool [1]; the RevC model [2] and a boot-wrapper [3].
>
>   $ ${BOOTWRAPPER}/configure --host=aarch64-linux-gnu
>      --with-dtb=${KERNEL}/arch/arm64/boot/dts/arm/fvp-base-revc.dts
>      --with-kernel-dir=${KERNEL}
>      --with-initrd=${BUILDROOT}/images/rootfs.cpio
>      --with-cmdline="console=ttyAMA0"
>      --enable-psci --enable-gicv3
>   $ make        # produces linux-system.axf
>
> Run the model:
>   $ FVP_Base_RevC-2xAEMvA
>      -C bp.secure_memory=0
>      -C 'pctl.startup=0.*.*.*'
>      -C bp.refcounter.non_arch_start_at_default=1
>      -C cache_state_modelled=0
>      -C bp.vis.disable_visualisation=1
>      -C bp.virtio_net.enabled=1
>      -C bp.virtio_net.hostbridge.userNetPorts=8022=22
>      -C bp.virtio_net.hostbridge.userNetworking=1
>      -C pci.pci_smmuv3.mmu.SMMU_IDR0=135100351
>      -C pci.pci_smmuv3.mmu.SMMU_IDR3=4116
>      -C pci.pci_smmuv3.mmu.SMMU_IDR5=8389749
>      -C cluster0.NUM_CORES=4
>      -C cluster1.NUM_CORES=4
>      -a 'cluster*.cpu*=linux-system.axf'
>
> Then run a job using the tool. The process allocates two buffers and
> passes their VA to the device (via the kernel driver). The device memcpies
> one buffer to the other:
>
>    $ smmute -u mmap
>    ... Success
>
> With smmu and iommu trace events enabled, a trace should contain
> smmu_mm_invalidate and dev_fault/dev_page_response events.
>
> It's not entirely representative of SVA flow, where an assignable device
> interface is mapped into the process and the process launches jobs
> directly without going through the kernel (that would now use
> drivers/misc/uacce), but it does exercise IOMMU SVA: sva_bind(), device
> accessing the process address space with PASID and some IOPFs, which I
> think is what you're looking for. However this model doesn't have a PCI
> test device so you won't be able to test ATC invalidations with PASID.
>
> Other useful tests would be enabling lockdep (some intricate locking
> between IOPF, the driver and mm), killing bound processes (-k), triggering
> invalid accesses to verify TLB invalidations (-f tlb, I think). There is a
> lot more to test, like thp and oom, but I don't have those in this branch.
>
> Thanks,
> Jean
>
> [1] https://jpbrucker.net/git/linux/log/?h=sva/smmute-revc
> [2] https://developer.arm.com/downloads/-/arm-ecosystem-models
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/mark/boot-wrapper-aarch64.git/

Re: [PATCH v1 4/5] iommu/arm-smmu-v3: Keep track of attached ssids

[kernel test robot]

Hi Michael,

kernel test robot noticed the following build warnings:

[auto build test WARNING on v6.4-rc1]
[also build test WARNING on linus/master next-20230510]
[cannot apply to joro-iommu/next arm-perf/for-next/perf]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Michael-Shavit/iommu-arm-smmu-v3-Move-cdtable-to-arm_smmu_master/20230511-045514
base:   ac9a78681b921877518763ba0e89202254349d1b
patch link:    https://lore.kernel.org/r/20230510205054.2667898-5-mshavit%40google.com
patch subject: [PATCH v1 4/5] iommu/arm-smmu-v3: Keep track of attached ssids
config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20230511/202305110736.wdeuqPAi-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/2172fed7450d1bb8518b86b2b7113a1e42b4d456
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Michael-Shavit/iommu-arm-smmu-v3-Move-cdtable-to-arm_smmu_master/20230511-045514
        git checkout 2172fed7450d1bb8518b86b2b7113a1e42b4d456
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/iommu/arm/arm-smmu-v3/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305110736.wdeuqPAi-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:1857:5: warning: no previous prototype for 'arm_smmu_atc_inv_domain' [-Wmissing-prototypes]
    1857 | int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain,
         |     ^~~~~~~~~~~~~~~~~~~~~~~


vim +/arm_smmu_atc_inv_domain +1857 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

  1856	
> 1857	int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain,
  1858				    unsigned long iova, size_t size)
  1859	{
  1860		return arm_smmu_atc_inv_domain_ssid(smmu_domain, 0, iova, size);
  1861	}
  1862	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[PATCH v1 5/5] iommu/arm-smmu-v3: Implement set_dev_pasid

[Michael Shavit]

This change enables the use of the iommu_attach_dev_pasid API for
UNMANAGED domains. The primary use-case is to allow in-kernel users of
the iommu API to manage domains with PASID. This change also allows for
future support of pasid in the DMA api.

Signed-off-by: Michael Shavit <mshavit@google.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 165 +++++++++++++++++---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |   1 +
 2 files changed, 147 insertions(+), 19 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 81f49a86c1266..468a7a30ffe7b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2353,6 +2353,11 @@ static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
 	return 0;
 }
 
+static bool arm_smmu_master_has_pasid_domains(struct arm_smmu_master *master)
+{
+	return master->nr_attached_pasid_domains > 0;
+}
+
 static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
 {
 	struct pci_dev *pdev;
@@ -2396,6 +2401,33 @@ static void arm_smmu_detach_dev(struct arm_smmu_master *master)
 	arm_smmu_install_ste_for_dev(master);
 }
 
+/*
+ * Once attached for the first time, a domain can no longer be attached to any
+ * master with a distinct upstream SMMU.
+ */
+static int arm_smmu_prepare_domain_for_smmu(struct device *dev,
+					    struct arm_smmu_device *smmu,
+					    struct arm_smmu_domain *smmu_domain)
+{
+	int ret = 0;
+
+	mutex_lock(&smmu_domain->init_mutex);
+	if (!smmu_domain->smmu) {
+		smmu_domain->smmu = smmu;
+		ret = arm_smmu_domain_finalise(&smmu_domain->domain);
+		if (ret) {
+			smmu_domain->smmu = NULL;
+			goto out_unlock;
+		}
+	} else if (smmu_domain->smmu != smmu) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+out_unlock:
+	mutex_unlock(&smmu_domain->init_mutex);
+	return ret;
+}
+
 static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 {
 	int ret = 0;
@@ -2411,6 +2443,10 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	master = dev_iommu_priv_get(dev);
 	smmu = master->smmu;
 
+	ret = arm_smmu_prepare_domain_for_smmu(dev, smmu, smmu_domain);
+	if (ret)
+		return ret;
+
 	/*
 	 * Checking that SVA is disabled ensures that this device isn't bound to
 	 * any mm, and can be safely detached from its old domain. Bonds cannot
@@ -2421,22 +2457,18 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 		return -EBUSY;
 	}
 
-	arm_smmu_detach_dev(master);
-
-	mutex_lock(&smmu_domain->init_mutex);
-
-	if (!smmu_domain->smmu) {
-		smmu_domain->smmu = smmu;
-		ret = arm_smmu_domain_finalise(domain);
-		if (ret) {
-			smmu_domain->smmu = NULL;
-			goto out_unlock;
-		}
-	} else if (smmu_domain->smmu != smmu) {
-		ret = -EINVAL;
-		goto out_unlock;
+	/*
+	 * Attaching a bypass or stage 2 domain would break any domains attached
+	 * with pasid. Attaching an S1 domain should be feasible but requires
+	 * more complicated logic to handle.
+	 */
+	if (arm_smmu_master_has_pasid_domains(master)) {
+		dev_err(dev, "cannot attach - domain attached with pasid\n");
+		return -EBUSY;
 	}
 
+	arm_smmu_detach_dev(master);
+
 	/*
 	 * Note that this will end up calling arm_smmu_sync_cd() before
 	 * the master has been added to the devices list for this domain.
@@ -2446,7 +2478,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 		master->has_stage1 = true;
 		ret = arm_smmu_write_ctx_desc(master, 0, &smmu_domain->cd);
 		if (ret)
-			goto out_unlock;
+			return ret;
 	} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S2 ||
 		   smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED) {
 		master->s2_cfg = &smmu_domain->s2_cfg;
@@ -2473,11 +2505,74 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 
 	arm_smmu_enable_ats(master, smmu_domain);
 
-out_unlock:
-	mutex_unlock(&smmu_domain->init_mutex);
 	return ret;
 }
 
+static int arm_smmu_set_dev_pasid(struct iommu_domain *domain,
+				  struct device *dev, ioasid_t pasid)
+{
+	int ret = 0;
+	unsigned long flags;
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+	struct arm_smmu_device *smmu;
+	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+	struct arm_smmu_attached_domain *attached_domain;
+	struct arm_smmu_master *master;
+
+	if (!fwspec)
+		return -ENOENT;
+
+	master = dev_iommu_priv_get(dev);
+	smmu = master->smmu;
+
+	ret = arm_smmu_prepare_domain_for_smmu(dev, smmu, smmu_domain);
+	if (ret)
+		return ret;
+
+	if (pasid == 0) {
+		dev_err(dev, "pasid 0 is reserved for the device's primary domain\n");
+		return -ENODEV;
+	}
+
+	if (smmu_domain->stage != ARM_SMMU_DOMAIN_S1) {
+		dev_err(dev, "set_dev_pasid only supports stage 1 domains\n");
+		return -EINVAL;
+	}
+
+	if (!master->has_stage1 || master->s2_cfg)
+		return -EBUSY;
+
+	attached_domain = kzalloc(sizeof(*attached_domain), GFP_KERNEL);
+	if (!attached_domain)
+		return -ENOMEM;
+
+	attached_domain->master = master;
+	attached_domain->domain = smmu_domain;
+	attached_domain->ssid = pasid;
+
+	master->nr_attached_pasid_domains += 1;
+	/*
+	 * arm_smmu_share_asid may update the cd's asid value and write the
+	 * ctx_desc for every attached_domains in the list. There's a potential
+	 * race here regardless of whether we first the write the ctx_desc or
+	 * first insert into the domain's list. Grabbing the asic_lock prevents
+	 * SVA from changing the cd's ASID while the cd is being attached.
+	 */
+	mutex_lock(&arm_smmu_asid_lock);
+	ret = arm_smmu_write_ctx_desc(master, pasid, &smmu_domain->cd);
+	if (ret) {
+		mutex_unlock(&arm_smmu_asid_lock);
+		kfree(attached_domain);
+	}
+
+	spin_lock_irqsave(&smmu_domain->attached_domains_lock, flags);
+	list_add(&attached_domain->domain_head, &smmu_domain->attached_domains);
+	spin_unlock_irqrestore(&smmu_domain->attached_domains_lock, flags);
+	mutex_unlock(&arm_smmu_asid_lock);
+
+	return 0;
+}
+
 static int arm_smmu_map_pages(struct iommu_domain *domain, unsigned long iova,
 			      phys_addr_t paddr, size_t pgsize, size_t pgcount,
 			      int prot, gfp_t gfp, size_t *mapped)
@@ -2723,6 +2818,15 @@ static void arm_smmu_release_device(struct device *dev)
 
 	if (WARN_ON(arm_smmu_master_sva_enabled(master)))
 		iopf_queue_remove_device(master->smmu->evtq.iopf, dev);
+	if (WARN_ON(master->nr_attached_pasid_domains != 0)) {
+		/*
+		 * TODO: Do we need to handle this case?
+		 * This requires a mechanism to obtain all the pasid domains
+		 * that this master is attached to so that we can clean up the
+		 * domain's attached_domain list.
+		 */
+	}
+
 	arm_smmu_detach_dev(master);
 	arm_smmu_free_cd_tables(master);
 	arm_smmu_disable_pasid(master);
@@ -2858,12 +2962,34 @@ static int arm_smmu_def_domain_type(struct device *dev)
 static void arm_smmu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
 {
 	struct iommu_domain *domain;
+	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+	struct arm_smmu_domain *smmu_domain;
+	struct arm_smmu_attached_domain *attached_domain;
+	unsigned long flags;
 
-	domain = iommu_get_domain_for_dev_pasid(dev, pasid, IOMMU_DOMAIN_SVA);
+	if (!master || pasid == 0)
+		return;
+
+	domain = iommu_get_domain_for_dev_pasid(dev, pasid, 0);
 	if (WARN_ON(IS_ERR(domain)) || !domain)
 		return;
+	if (domain->type == IOMMU_DOMAIN_SVA)
+		return arm_smmu_sva_remove_dev_pasid(domain, dev, pasid);
 
-	arm_smmu_sva_remove_dev_pasid(domain, dev, pasid);
+	smmu_domain = to_smmu_domain(domain);
+	mutex_lock(&arm_smmu_asid_lock);
+	spin_lock_irqsave(&smmu_domain->attached_domains_lock, flags);
+	list_for_each_entry(attached_domain, &smmu_domain->attached_domains, domain_head) {
+		if (attached_domain->master != master ||
+		    attached_domain->ssid != pasid)
+			continue;
+		list_del(&attached_domain->domain_head);
+		break;
+	}
+	spin_unlock_irqrestore(&smmu_domain->attached_domains_lock, flags);
+	arm_smmu_write_ctx_desc(master, pasid, NULL);
+	master->nr_attached_pasid_domains -= 1;
+	mutex_unlock(&arm_smmu_asid_lock);
 }
 
 static struct iommu_ops arm_smmu_ops = {
@@ -2883,6 +3009,7 @@ static struct iommu_ops arm_smmu_ops = {
 	.owner			= THIS_MODULE,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev		= arm_smmu_attach_dev,
+		.set_dev_pasid		= arm_smmu_set_dev_pasid,
 		.map_pages		= arm_smmu_map_pages,
 		.unmap_pages		= arm_smmu_unmap_pages,
 		.flush_iotlb_all	= arm_smmu_flush_iotlb_all,
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 35700534a0b4a..9ef63ea381f4b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -705,6 +705,7 @@ struct arm_smmu_master {
 	bool				iopf_enabled;
 	struct list_head		bonds;
 	unsigned int			ssid_bits;
+	unsigned int			nr_attached_pasid_domains;
 };
 
 /* SMMU private data for an IOMMU domain */
-- 
2.40.1.521.gf1e218fcd8-goog

Re: [PATCH v1 0/5] Add PASID support to SMMUv3 unmanaged domains

[Jason Gunthorpe]

On Thu, May 11, 2023 at 04:50:47AM +0800, Michael Shavit wrote:
> Hi all,
> 
> This patch series refactors the arm-smmu-v3 driver and implements the
> set_dev_pasid functionality for DMA and UNMANAGED iommu domains. As part
> of this effort, we also refactor the arm-smmu-v3 driver such that each
> iommu domain represent a single address space. In particular, stage 1
> domains hold a single ContextDescriptor instead of the entire STE entry.

I'm not sure what you mean "holds" a ContextDescriptor?

Ideally the iommu_domain should only hold a pointer to some table
top. Depending on the domain type this would be a S1 IOPTE table, S2
IOPTE table or a CD table. Plus the non-table domains like IDENTITY
and blocked.

Logically when an iommu_domain is attached to a device or a PASID a
STE or CD is generated from the iommu_domain's configuration but the
iommu_domain doesn't "hold" it

When a kernel-owned CD table is used it should be held someplace else,
certianly not in the iommu_domain. Logically as a per-device
structure, but maybe with optimizations for sharing.

> The refactor is arguably valuable independently from the set_dev_pasid
> feature since an iommu_domain is conceptually closer to a single address
> space than an entire STE. In addition this unlocks some nice clean-up of
> the arm SVA implementation which today piggybacks SVA domains on the
> "primary" domain.

I always thought of this as sort of a pre-calculation of the STE, that
gets cached in the iommu_domain? Not sure the pre-calculation is that
valuable though..
 
> path forward for set_dev_pasid support? Or could a uAPI that only
> exposes a single CD instead of the entire STE be an appropriate fit for
> the nesting use cases?

The uAPI is to create an iommu_domain that holds a CD Table Top
located in user memory, it cannot deviate from this. These kinds of
iommu_domain's can only be pointed at by STEs.

Again it doesnt really "hold" the STE, but we can compute a STE that
points to the SD Table that it does hold.

Other than this, it is good to take this project on, getting PASID
support aligned with the new API is something that needs to be done
here!

Thanks,
Jason

Re: [PATCH v1 0/5] Add PASID support to SMMUv3 unmanaged domains

[Michael Shavit]

> Logically when an iommu_domain is attached to a device or a PASID a
> STE or CD is generated from the iommu_domain's configuration but the
> iommu_domain doesn't "hold" it
>

Ah yes, I was using iommu domain and arm_smmu_domain interchangeably
here since there's a 1:1 mapping between the two. In the current
smmuv3 implementation, arm_smmu_domain holds the s1cfg structure which
represents the s1 portion of an STE.

> I always thought of this as sort of a pre-calculation of the STE, that
> gets cached in the iommu_domain? Not sure the pre-calculation is that
> valuable though..

I think we can consider the case where an iommu domain is attached to
multiple masters as a form of pre-calculation and resource sharing.
When attaching an iommu domain that has been attached before, its
arm_smmu_domain contains an already finalized STE configuration that
can be immediately written to the smmu. I don't think there's anything
specific to SVA about this behavior however, SVA will do the same
amount of work whether the cd table is owned by some special iommu
domain or by the arm_smmu_master (since we require that special iommu
domain be attached to the master and disallow detaching it).

> Ideally the iommu_domain should only hold a pointer to some table
top. Depending on the domain type this would be a S1 IOPTE table, S2
IOPTE table or a CD table. Plus the non-table domains like IDENTITY
and blocked.

Gotcha. So this patch series should be less aggressive, but is
probably still workable with the nested domain patch series:
1. For (stage 1) unmanaged/dma and sva domains, arm_smmu_domains
should hold a single CD. For the nested domain series, arm_smmu_domain
can alternatively hold an entire s1cfg.
2. arm_smmu_master should own an s1cfg (which holds a cdtable) that is
used by unmanaged/dma and sva domains attached to this master.
3. arm_smmu_master also holds a pointer to the live s1cfg, which may
either points to its owned s1cfg, or the nested domain's s1cfg, or
null (bypass or stage2)







> > path forward for set_dev_pasid support? Or could a uAPI that only
> > exposes a single CD instead of the entire STE be an appropriate fit for
> > the nesting use cases?
>
> The uAPI is to create an iommu_domain that holds a CD Table Top
> located in user memory, it cannot deviate from this. These kinds of
> iommu_domain's can only be pointed at by STEs.
>
> Again it doesnt really "hold" the STE, but we can compute a STE that
> points to the SD Table that it does hold.
>
> Other than this, it is good to take this project on, getting PASID
> support aligned with the new API is something that needs to be done
> here!
>
> Thanks,
> Jason

Re: [PATCH v1 0/5] Add PASID support to SMMUv3 unmanaged domains

[Jason Gunthorpe]

On Thu, May 11, 2023 at 11:52:41AM +0800, Michael Shavit wrote:
> > Logically when an iommu_domain is attached to a device or a PASID a
> > STE or CD is generated from the iommu_domain's configuration but the
> > iommu_domain doesn't "hold" it
> >
> 
> Ah yes, I was using iommu domain and arm_smmu_domain interchangeably
> here since there's a 1:1 mapping between the two. In the current
> smmuv3 implementation, arm_smmu_domain holds the s1cfg structure which
> represents the s1 portion of an STE.

I mean to include the arm_smmu_domain too..

Generally this sort of seemed OK in the code, I just wouldn't use the
word 'hold' - the iommu_domain may cache a computed STE or CD value
but that the actual steering or context tables are held else where

ie you insert an iommu_domain into a steering or context table, it
does not 'hold' a table entry.

> specific to SVA about this behavior however, SVA will do the same
> amount of work whether the cd table is owned by some special iommu
> domain or by the arm_smmu_master (since we require that special iommu
> domain be attached to the master and disallow detaching it).

The CD table for SVA definately should not be part of an iommu_domain,
moving it to the master seems reasonable.
 
> Gotcha. So this patch series should be less aggressive, but is
> probably still workable with the nested domain patch series:
> 1. For (stage 1) unmanaged/dma and sva domains, arm_smmu_domains
> should hold a single CD. For the nested domain series, arm_smmu_domain
> can alternatively hold an entire s1cfg.

These are just pre computed values the can help when inserting the
iommu_domain into a steering or CD table

> 2. arm_smmu_master should own an s1cfg (which holds a cdtable) that is
> used by unmanaged/dma and sva domains attached to this master.

The arm_smmu_master's cd table can be inserted into a steering table

> 3. arm_smmu_master also holds a pointer to the live s1cfg, which may
> either points to its owned s1cfg, or the nested domain's s1cfg, or
> null (bypass or stage2)

The steering table either points to the CD table owned by the
arm_smmu_master, a S1 domain held by an iommu_domain, or a S1 & CD
table owned by userspace represented by a special nested iommu_domain
and its internal parent.

If a kernel owned S2 it attached then the S1 points at the CD table
owned by the arm_smmu_master and the CD table points to the S2, same
as if there was PASID (IIRC, from memory I don't have the spec here
right now)

Think about it in terms of what object owns the table and what other
object(s) are inserted into the the table. Nothing "holds" a table
entry.

Jason

Re: [PATCH v1 0/5] Add PASID support to SMMUv3 unmanaged domains

[Robin Murphy]

On 2023-05-11 05:33, Jason Gunthorpe wrote:
> On Thu, May 11, 2023 at 11:52:41AM +0800, Michael Shavit wrote:
>>> Logically when an iommu_domain is attached to a device or a PASID a
>>> STE or CD is generated from the iommu_domain's configuration but the
>>> iommu_domain doesn't "hold" it
>>>
>>
>> Ah yes, I was using iommu domain and arm_smmu_domain interchangeably
>> here since there's a 1:1 mapping between the two. In the current
>> smmuv3 implementation, arm_smmu_domain holds the s1cfg structure which
>> represents the s1 portion of an STE.
> 
> I mean to include the arm_smmu_domain too..
> 
> Generally this sort of seemed OK in the code, I just wouldn't use the
> word 'hold' - the iommu_domain may cache a computed STE or CD value
> but that the actual steering or context tables are held else where
> 
> ie you insert an iommu_domain into a steering or context table, it
> does not 'hold' a table entry.
> 
>> specific to SVA about this behavior however, SVA will do the same
>> amount of work whether the cd table is owned by some special iommu
>> domain or by the arm_smmu_master (since we require that special iommu
>> domain be attached to the master and disallow detaching it).
> 
> The CD table for SVA definately should not be part of an iommu_domain,
> moving it to the master seems reasonable.
>   
>> Gotcha. So this patch series should be less aggressive, but is
>> probably still workable with the nested domain patch series:
>> 1. For (stage 1) unmanaged/dma and sva domains, arm_smmu_domains
>> should hold a single CD. For the nested domain series, arm_smmu_domain
>> can alternatively hold an entire s1cfg.
> 
> These are just pre computed values the can help when inserting the
> iommu_domain into a steering or CD table

Right, a stage 1 domain still logically owns the *contents* of a 
corresponding CD, however in this design it can no longer own a physical 
CD structure, because now every device attached to the same domain must 
maintain its own distinct copy.

>> 2. arm_smmu_master should own an s1cfg (which holds a cdtable) that is
>> used by unmanaged/dma and sva domains attached to this master.
> 
> The arm_smmu_master's cd table can be inserted into a steering table

Not sure what you mean there... STE.S1ContextPtr is essentially just a 
pointer to an array of CD structures (which only contains 1 element when 
PASIDs aren't enabled), so every master must own its own CD table 
directly. There is no viable indirection if you want the abstraction to 
bear any relation to reality.

>> 3. arm_smmu_master also holds a pointer to the live s1cfg, which may
>> either points to its owned s1cfg, or the nested domain's s1cfg, or
>> null (bypass or stage2)
> 
> The steering table either points to the CD table owned by the
> arm_smmu_master, a S1 domain held by an iommu_domain, or a S1 & CD
> table owned by userspace represented by a special nested iommu_domain
> and its internal parent.
> 
> If a kernel owned S2 it attached then the S1 points at the CD table
> owned by the arm_smmu_master and the CD table points to the S2, same
> as if there was PASID (IIRC, from memory I don't have the spec here
> right now)

Nope, CDs *only* represent stage 1 domains, stage 2 configuration is a 
property of the STE, i.e. is directly connected to the arm_smmu_master.

Thanks,
Robin.

Re: [PATCH v1 0/5] Add PASID support to SMMUv3 unmanaged domains

[Jason Gunthorpe]

On Thu, May 11, 2023 at 01:33:58PM +0100, Robin Murphy wrote:
> > > 2. arm_smmu_master should own an s1cfg (which holds a cdtable) that is
> > > used by unmanaged/dma and sva domains attached to this master.
> > 
> > The arm_smmu_master's cd table can be inserted into a steering table
> 
> Not sure what you mean there... STE.S1ContextPtr is essentially just a
> pointer to an array of CD structures (which only contains 1 element when
> PASIDs aren't enabled), so every master must own its own CD table directly.
> There is no viable indirection if you want the abstraction to bear any
> relation to reality.

Yes, this is what I mean. Whenever we need a kernel owned CD table it
comes from the smmu master and is inserted into the steering table
owned by the arm_smmu_device.

"Insert" is just the usual verb we tend to use when talking about
these kinds of structures. Ie a PTE is inserted into a page table and
points at a page - a page table doesn't hold a PTE owned by the page.

So we have, basically, three kinds of tables, Steering/CD/IOPTE, they
are owned by their respective objects
arm_smmu_device/arm_smmu_master/arm_smmu_domain

And we insert pointers from Steering -> CD -> IOPTE as appropriate.

The only case a CD table is not in the arm_smmu_master is for nesting,
but we can still say that the nesting domain is inserted into the
steering table.

Jason