[PATCH v2 01/10] iommu/arm-smmu-v3: Store IOTLB cache tags in struct arm_smmu_attach_state

Nicolin Chen posted 10 patches 2 weeks, 3 days ago
[PATCH v2 01/10] iommu/arm-smmu-v3: Store IOTLB cache tags in struct arm_smmu_attach_state
Posted by Nicolin Chen 2 weeks, 3 days ago
So far, an IOTLB tag (ASID or VMID) has been stored in the arm_smmu_domain
structure. Its lifecycle is aligned with the smmu_domain.

However, an IOTLB tag (ASID/VMID) will not be used:
1) Before being installed to CD/STE during a device attachment
2) After being removed from CD/STE during a device detachment

Both (1) and (2) exactly align with the lifecycle of smmu_domain->invs.

The bigger problem is that storing the IOTLB tag in struct arm_smmu_domain
makes it difficult to share across SMMU instances, a common use case for a
nesting parent domain.

Store the IOTLB tags (old domain's and new domain's) in the state. This'll
be forwarded to CD and STE entries, to replace the references of cd->asid
and s2_cfg->vmid from the smmu_domain.

Add a new arm_smmu_domain_get_iotlb_tag() helper provisionally copying the
existing IOTLB tags from smmu_domain to fill arm_smmu_master_build_invs().
Later it will retrieve an IOTLB tag from the smmu_domain->invs or allocate
a new one for the lifecycle issue.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  6 ++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 78 ++++++++++++++-------
 2 files changed, 60 insertions(+), 24 deletions(-)

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 4f104c1baa67..73cb59c7d4b1 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -1111,11 +1111,13 @@ static inline bool arm_smmu_master_canwbs(struct arm_smmu_master *master)
  * @new_invs: for new domain, this is the new invs array to update domain->invs;
  *            for old domain, this is the master->build_invs to pass in as the
  *            to_unref argument to an arm_smmu_invs_unref() call
+ * @tag: IOTLB cache tag (INV_TYPE_S1_ASID or INV_TYPE_S2_VMID)
  */
 struct arm_smmu_inv_state {
 	struct arm_smmu_invs __rcu **invs_ptr;
 	struct arm_smmu_invs *old_invs;
 	struct arm_smmu_invs *new_invs;
+	struct arm_smmu_inv tag;
 };
 
 struct arm_smmu_attach_state {
@@ -1132,6 +1134,10 @@ struct arm_smmu_attach_state {
 	bool ats_enabled;
 };
 
+int arm_smmu_domain_get_iotlb_tag(struct arm_smmu_domain *smmu_domain,
+				  struct arm_smmu_device *smmu,
+				  struct arm_smmu_inv *tag);
+
 int arm_smmu_attach_prepare(struct arm_smmu_attach_state *state,
 			    struct iommu_domain *new_domain);
 void arm_smmu_attach_commit(struct arm_smmu_attach_state *state);
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 9912262a0e3b..325eabb51c81 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3117,6 +3117,33 @@ static void arm_smmu_disable_iopf(struct arm_smmu_master *master,
 		iopf_queue_remove_device(master->smmu->evtq.iopf, master->dev);
 }
 
+int arm_smmu_domain_get_iotlb_tag(struct arm_smmu_domain *smmu_domain,
+				  struct arm_smmu_device *smmu,
+				  struct arm_smmu_inv *tag)
+{
+	/* Decide the type of the iotlb cache tag */
+	switch (smmu_domain->stage) {
+	case ARM_SMMU_DOMAIN_SVA:
+	case ARM_SMMU_DOMAIN_S1:
+		tag->type = INV_TYPE_S1_ASID;
+		break;
+	case ARM_SMMU_DOMAIN_S2:
+		tag->type = INV_TYPE_S2_VMID;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	tag->smmu = smmu;
+
+	if (tag->type == INV_TYPE_S1_ASID)
+		tag->id = smmu_domain->cd.asid;
+	else
+		tag->id = smmu_domain->s2_cfg.vmid;
+
+	return 0;
+}
+
 static struct arm_smmu_inv *
 arm_smmu_master_build_inv(struct arm_smmu_master *master,
 			  enum arm_smmu_inv_type type, u32 id, ioasid_t ssid,
@@ -3176,7 +3203,8 @@ arm_smmu_master_build_inv(struct arm_smmu_master *master,
  */
 static struct arm_smmu_invs *
 arm_smmu_master_build_invs(struct arm_smmu_master *master, bool ats_enabled,
-			   ioasid_t ssid, struct arm_smmu_domain *smmu_domain)
+			   ioasid_t ssid, struct arm_smmu_domain *smmu_domain,
+			   struct arm_smmu_inv *tag)
 {
 	const bool nesting = smmu_domain->nest_parent;
 	size_t pgsize = 0, i;
@@ -3189,30 +3217,15 @@ arm_smmu_master_build_invs(struct arm_smmu_master *master, bool ats_enabled,
 	if (master->smmu->features & ARM_SMMU_FEAT_RANGE_INV)
 		pgsize = __ffs(smmu_domain->domain.pgsize_bitmap);
 
-	switch (smmu_domain->stage) {
-	case ARM_SMMU_DOMAIN_SVA:
-	case ARM_SMMU_DOMAIN_S1:
-		if (!arm_smmu_master_build_inv(master, INV_TYPE_S1_ASID,
-					       smmu_domain->cd.asid,
-					       IOMMU_NO_PASID, pgsize))
-			return NULL;
-		break;
-	case ARM_SMMU_DOMAIN_S2:
-		if (!arm_smmu_master_build_inv(master, INV_TYPE_S2_VMID,
-					       smmu_domain->s2_cfg.vmid,
-					       IOMMU_NO_PASID, pgsize))
-			return NULL;
-		break;
-	default:
-		WARN_ON(true);
+	if (!arm_smmu_master_build_inv(master, tag->type, tag->id,
+				       IOMMU_NO_PASID, pgsize))
 		return NULL;
-	}
 
 	/* All the nested S1 ASIDs have to be flushed when S2 parent changes */
 	if (nesting) {
-		if (!arm_smmu_master_build_inv(
-			    master, INV_TYPE_S2_VMID_S1_CLEAR,
-			    smmu_domain->s2_cfg.vmid, IOMMU_NO_PASID, 0))
+		if (!arm_smmu_master_build_inv(master,
+					       INV_TYPE_S2_VMID_S1_CLEAR,
+					       tag->id, IOMMU_NO_PASID, 0))
 			return NULL;
 	}
 
@@ -3280,7 +3293,9 @@ static int arm_smmu_attach_prepare_invs(struct arm_smmu_attach_state *state,
 	struct arm_smmu_domain *old_smmu_domain =
 		to_smmu_domain_devices(state->old_domain);
 	struct arm_smmu_master *master = state->master;
+	struct arm_smmu_device *smmu = master->smmu;
 	ioasid_t ssid = state->ssid;
+	int ret;
 
 	/*
 	 * At this point a NULL domain indicates the domain doesn't use the
@@ -3294,8 +3309,16 @@ static int arm_smmu_attach_prepare_invs(struct arm_smmu_attach_state *state,
 		invst->old_invs = rcu_dereference_protected(
 			new_smmu_domain->invs,
 			lockdep_is_held(&arm_smmu_asid_lock));
-		build_invs = arm_smmu_master_build_invs(
-			master, state->ats_enabled, ssid, new_smmu_domain);
+
+		ret = arm_smmu_domain_get_iotlb_tag(new_smmu_domain, smmu,
+						    &invst->tag);
+		if (ret)
+			return ret;
+
+		build_invs = arm_smmu_master_build_invs(master,
+							state->ats_enabled,
+							ssid, new_smmu_domain,
+							&invst->tag);
 		if (!build_invs)
 			return -EINVAL;
 
@@ -3316,9 +3339,16 @@ static int arm_smmu_attach_prepare_invs(struct arm_smmu_attach_state *state,
 			invst->old_invs = rcu_dereference_protected(
 				old_smmu_domain->invs,
 				lockdep_is_held(&arm_smmu_asid_lock));
+
+		ret = arm_smmu_domain_get_iotlb_tag(old_smmu_domain, smmu,
+						    &invst->tag);
+		if (WARN_ON(ret))
+			return ret;
+
 		/* For old_smmu_domain, new_invs points to master->build_invs */
 		invst->new_invs = arm_smmu_master_build_invs(
-			master, master->ats_enabled, ssid, old_smmu_domain);
+			master, master->ats_enabled, ssid, old_smmu_domain,
+			&invst->tag);
 	}
 
 	return 0;
-- 
2.43.0
Re: [PATCH v2 01/10] iommu/arm-smmu-v3: Store IOTLB cache tags in struct arm_smmu_attach_state
Posted by Jason Gunthorpe 1 week, 5 days ago
On Wed, Jan 21, 2026 at 05:24:19PM -0800, Nicolin Chen wrote:
> +int arm_smmu_domain_get_iotlb_tag(struct arm_smmu_domain *smmu_domain,
> +				  struct arm_smmu_device *smmu,
> +				  struct arm_smmu_inv *tag)
> +{
> +	/* Decide the type of the iotlb cache tag */
> +	switch (smmu_domain->stage) {
> +	case ARM_SMMU_DOMAIN_SVA:
> +	case ARM_SMMU_DOMAIN_S1:
> +		tag->type = INV_TYPE_S1_ASID;
> +		break;
> +	case ARM_SMMU_DOMAIN_S2:
> +		tag->type = INV_TYPE_S2_VMID;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	tag->smmu = smmu;
> +
> +	if (tag->type == INV_TYPE_S1_ASID)
> +		tag->id = smmu_domain->cd.asid;
> +	else
> +		tag->id = smmu_domain->s2_cfg.vmid;

Would be tidier to move these up into the case

Otherwise looks OK

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason
Re: [PATCH v2 01/10] iommu/arm-smmu-v3: Store IOTLB cache tags in struct arm_smmu_attach_state
Posted by Nicolin Chen 1 week, 5 days ago
On Mon, Jan 26, 2026 at 04:44:43PM -0400, Jason Gunthorpe wrote:
> On Wed, Jan 21, 2026 at 05:24:19PM -0800, Nicolin Chen wrote:
> > +int arm_smmu_domain_get_iotlb_tag(struct arm_smmu_domain *smmu_domain,
> > +				  struct arm_smmu_device *smmu,
> > +				  struct arm_smmu_inv *tag)
> > +{
> > +	/* Decide the type of the iotlb cache tag */
> > +	switch (smmu_domain->stage) {
> > +	case ARM_SMMU_DOMAIN_SVA:
> > +	case ARM_SMMU_DOMAIN_S1:
> > +		tag->type = INV_TYPE_S1_ASID;
> > +		break;
> > +	case ARM_SMMU_DOMAIN_S2:
> > +		tag->type = INV_TYPE_S2_VMID;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	tag->smmu = smmu;
> > +
> > +	if (tag->type == INV_TYPE_S1_ASID)
> > +		tag->id = smmu_domain->cd.asid;
> > +	else
> > +		tag->id = smmu_domain->s2_cfg.vmid;
> 
> Would be tidier to move these up into the case

This will act as a fake allocation path in PATCh-3 and eventually
get replaced with a real allocation piece in PATCH-4. So, I wrote
like this in a transitory manner.

I can add a line of comments to clarify it.

Thanks
Nicolin
Re: [PATCH v2 01/10] iommu/arm-smmu-v3: Store IOTLB cache tags in struct arm_smmu_attach_state
Posted by Jason Gunthorpe 1 week, 4 days ago
On Mon, Jan 26, 2026 at 07:23:58PM -0800, Nicolin Chen wrote:
> On Mon, Jan 26, 2026 at 04:44:43PM -0400, Jason Gunthorpe wrote:
> > On Wed, Jan 21, 2026 at 05:24:19PM -0800, Nicolin Chen wrote:
> > > +int arm_smmu_domain_get_iotlb_tag(struct arm_smmu_domain *smmu_domain,
> > > +				  struct arm_smmu_device *smmu,
> > > +				  struct arm_smmu_inv *tag)
> > > +{
> > > +	/* Decide the type of the iotlb cache tag */
> > > +	switch (smmu_domain->stage) {
> > > +	case ARM_SMMU_DOMAIN_SVA:
> > > +	case ARM_SMMU_DOMAIN_S1:
> > > +		tag->type = INV_TYPE_S1_ASID;
> > > +		break;
> > > +	case ARM_SMMU_DOMAIN_S2:
> > > +		tag->type = INV_TYPE_S2_VMID;
> > > +		break;
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	tag->smmu = smmu;
> > > +
> > > +	if (tag->type == INV_TYPE_S1_ASID)
> > > +		tag->id = smmu_domain->cd.asid;
> > > +	else
> > > +		tag->id = smmu_domain->s2_cfg.vmid;
> > 
> > Would be tidier to move these up into the case
> 
> This will act as a fake allocation path in PATCh-3 and eventually
> get replaced with a real allocation piece in PATCH-4. So, I wrote
> like this in a transitory manner.

I'd still write it in the nice way, you can remove the lines later
when you add the allocation logic.

Jason
Re: [PATCH v2 01/10] iommu/arm-smmu-v3: Store IOTLB cache tags in struct arm_smmu_attach_state
Posted by Nicolin Chen 1 week, 4 days ago
On Tue, Jan 27, 2026 at 11:08:48AM -0400, Jason Gunthorpe wrote:
> On Mon, Jan 26, 2026 at 07:23:58PM -0800, Nicolin Chen wrote:
> > On Mon, Jan 26, 2026 at 04:44:43PM -0400, Jason Gunthorpe wrote:
> > > On Wed, Jan 21, 2026 at 05:24:19PM -0800, Nicolin Chen wrote:
> > > > +int arm_smmu_domain_get_iotlb_tag(struct arm_smmu_domain *smmu_domain,
> > > > +				  struct arm_smmu_device *smmu,
> > > > +				  struct arm_smmu_inv *tag)
> > > > +{
> > > > +	/* Decide the type of the iotlb cache tag */
> > > > +	switch (smmu_domain->stage) {
> > > > +	case ARM_SMMU_DOMAIN_SVA:
> > > > +	case ARM_SMMU_DOMAIN_S1:
> > > > +		tag->type = INV_TYPE_S1_ASID;
> > > > +		break;
> > > > +	case ARM_SMMU_DOMAIN_S2:
> > > > +		tag->type = INV_TYPE_S2_VMID;
> > > > +		break;
> > > > +	default:
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > > +	tag->smmu = smmu;
> > > > +
> > > > +	if (tag->type == INV_TYPE_S1_ASID)
> > > > +		tag->id = smmu_domain->cd.asid;
> > > > +	else
> > > > +		tag->id = smmu_domain->s2_cfg.vmid;
> > > 
> > > Would be tidier to move these up into the case
> > 
> > This will act as a fake allocation path in PATCh-3 and eventually
> > get replaced with a real allocation piece in PATCH-4. So, I wrote
> > like this in a transitory manner.
> 
> I'd still write it in the nice way, you can remove the lines later
> when you add the allocation logic.

OK. I will keep them in the first switch statement.

This would instead make the get() in PATCH-3 a fake logic until
PATCH-4 replacing them with a real allocation logic. Should work
in the same way.

Thanks
Nicolin