Re: [PATCH] iommu/arm-smmu-v3: Allow stream table to have nodes with the same ID

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Apr 11, 2025 at 08:39:36PM -0700, Nicolin Chen wrote:
> 
> I figured that we could do something smaller too in SMMUv3 driver,
> somewhat similar to what Robin did for STE in commit 563b5cbe334e
> ("iommu/arm-smmu-v3: Cope with duplicated Stream IDs"):
> ----------------------------------------------
> 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 b4c21aaed1266..4b66e8ebef539 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -3393,6 +3393,10 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
>                 new_stream->id = sid;
>                 new_stream->master = master;
> 
> +               /* Bridged PCI devices may end up with duplicated IDs */
> +               if (master == arm_smmu_find_master(smmu, sid))
> +                       continue;
> +
>                 ret = arm_smmu_init_sid_strtab(smmu, sid);
>                 if (ret)
>                         break;
> ----------------------------------------------

I'd write the above like this though:

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 b4c21aaed1266a..97b9f8d7fb3340 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3389,6 +3389,7 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
 	for (i = 0; i < fwspec->num_ids; i++) {
 		struct arm_smmu_stream *new_stream = &master->streams[i];
 		u32 sid = fwspec->ids[i];
+		struct rb_node *existing;
 
 		new_stream->id = sid;
 		new_stream->master = master;
@@ -3398,10 +3399,20 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
 			break;
 
 		/* Insert into SID tree */
-		if (rb_find_add(&new_stream->node, &smmu->streams,
-				arm_smmu_streams_cmp_node)) {
-			dev_warn(master->dev, "stream %u already in tree\n",
-				 sid);
+		existing = rb_find_add(&new_stream->node, &smmu->streams,
+				       arm_smmu_streams_cmp_node);
+		if (existing) {
+			struct arm_smmu_master *existing_master =
+				rb_entry(existing, struct arm_smmu_stream, node)
+					->master;
+
+			/* Bridged PCI devices may end up with duplicated IDs */
+			if (existing_master == master)
+				continue;
+
+			dev_warn(master->dev,
+				 "stream %u already in tree from dev %s\n", sid,
+				 dev_name(existing_master->dev));
 			ret = -EINVAL;
 			break;
 		}

It seems like a reasonable rc fix, and if nobody has cared about DMA
Alias support since it was broken in 2021 maybe we just leave it.

Jason