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

Jason Gunthorpe
2025-04-12T13:57:27Z

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