Re: [PATCH RFC v4 2/4] iommu/arm-smmu-v3: Support shared SIDs in insert/remove_master

[Peng Fan]

On Thu, Oct 01, 2026 at 10:17:22AM -0700, Nicolin Chen wrote:
>On Thu, Oct 01, 2026 at 08:49:59PM +0800, Peng Fan (OSS) wrote:
>> -static int arm_smmu_streams_cmp_node(struct rb_node *lhs,
>> -				     const struct rb_node *rhs)
>> +static int arm_smmu_streams_cmp_node(struct rb_node *lhs, const struct rb_node *rhs)
>
>Why is this changed?

Just thought 100 chars is fine here, so ...
I will drop this change in v5.

>
>> @@ -4136,23 +4151,33 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
>>  		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;
>> +			struct arm_smmu_stream *existing_stream =
>> +				rb_entry(existing, struct arm_smmu_stream, node);
>>  
>>  			/* Bridged PCI devices may end up with duplicated IDs */
>> -			if (existing_master == master)
>> +			if (existing_stream->master == master)
>>  				continue;
>>  
>> -			dev_warn(master->dev,
>> -				 "Aliasing StreamID 0x%x (from %s) unsupported, expect DMA to be broken\n",
>> -				 sid, dev_name(existing_master->dev));
>> -			ret = -ENODEV;
>> -			break;
>> +			if (master->num_streams != 1 ||
>> +			    existing_stream->master->num_streams != 1) {
>> +				dev_warn(master->dev,
>> +					 "Shared StreamID 0x%x not supported for multi-SID masters\n",
>> +					 sid);
>> +				ret = -ENODEV;
>> +				break;
>> +			}
>> +
>> +			if (list_empty(&existing_stream->shared_masters))
>> +				list_add_tail(&existing_stream->master->shared_masters_elm,
>> +					      &existing_stream->shared_masters);
>> +			list_add_tail(&master->shared_masters_elm,
>> +				      &existing_stream->shared_masters);
>
>Masters still hold duplicated streams for the shared SID. And their
>ste_installed flags are out of sync..
>
>    master_a->stream[0]->a's stream0 {ste_installed=true}
>    master_b->stream[0]->b's stream0 {ste_installed=false}
>
>What I have been suggesting is to have a shared stream:
>    master_a->stream[0]-->shared stream0 {ste_installed=true}
>    master_b->stream[0]-|
>
>So, the driver would need a rework first changing master:
>-       struct arm_smmu_stream          *streams;
>+       struct arm_smmu_stream          **streams;

Ah. So I need a patch list saying:
1. iommu/arm-smmu-v3: Allocate streams individually. Change master->streams
   from an embedded array of struct arm_smmu_stream to an array of pointers,
   with each stream individually allocated, to prepare for shared-SID support
   where multiple masters will point to the same stream object
2. iommu/arm-smmu-v3: Support shared SIDs in insert/remove_master
3. iommu/arm-smmu-v3: Group aliasing devices into the same IOMMU group
4. iommu/arm-smmu-v3: Wire up shared-SID STE ordering and feature gating

>
>Also, arm_smmu_insert_master() needs to reverse its allocation:
>	for (i = 0; i < fwspec->num_ids; i++) {
>		stream = arm_smmu_find_stream(smmu, sid);
>		if (stream) {
>			// link to the existing stream
>		} else {
>			// allocate new stream
>		}
>		master->streams[i] = stream;
>	}
>	if (!ret)
>		sort();

Thanks for the details.

Thanks,
Peng

>
>Nicolin
>
>