Re: [PATCH RFC v6 4/5] iommu/arm-smmu-v3: Group aliasing devices into the same IOMMU group

[Robin Murphy]

On 2026-10-12 1:24 pm, Peng Fan wrote:
> On Sun, Oct 11, 2026 at 07:31:28PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@xxxxxxx>
> >
> > Devices sharing a SID must share a single IOMMU domain (and
> > therefore a single cd_table) because the STE can only point to one
> > cd_table at a time.
> >
> > Detect SID aliasing at group-assignment time by looking up the RB
> > tree for existing owners.  arm_smmu_device_group() is called before
> > arm_smmu_insert_master(), so any RB tree hit belongs to a
> > different, already-probed master.
> As Sashiko pointed, I need correct as below:
>
> Detect SID aliasing at group-assignment time by looking up the RB
> tree for existing owners. Since probe_device() inserts the master
> before device_group() is called, skip the device's own stream and
> look for a different, already-probed master that shares the same
> SID.
>
> > Assisted-by: LLM
> > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> > ---
> > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 33 ++++++++++++++++++++---------
> > 1 file changed, 23 insertions(+), 10 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 351ccdfa765b4..328d580935b96 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -4572,19 +4572,32 @@ static int arm_smmu_set_dirty_tracking(struct iommu_domain *domain,
> >
> > static struct iommu_group *arm_smmu_device_group(struct device *dev)
> > {
> > -	struct iommu_group *group;
> > +	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> > +	struct arm_smmu_device *smmu = master->smmu;
> > +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > +	struct iommu_group *group = NULL;
> > +	int i;
> > +
> > +	mutex_lock(&smmu->streams_mutex);
> > +	for (i = 0; i < fwspec->num_ids; i++) {
> > +		struct arm_smmu_stream *stream;
> > +
> > +		stream = arm_smmu_find_stream(smmu, fwspec->ids[i]);
> > +		if (!stream)
> > +			continue;
>
> Not very sure here, should I use below as check condition?
>    if (!stream || stream->master == master)
>        continue?

By this point, we must always find a valid stream for every ID, or 
probe_device() hasn't done its job correctly. The difference is if it 
only belongs to this device then it doesn't represent a group yet, 
otherwise if it is shared then the other device(s) must already have a 
group (since we're within the serialised part of probing) and we need to 
return that.

Thanks,
Robin.

> Per my understanding stream->master will never equals to master,
> but Sashiko concerns: Could this logic be adjusted to better handle the
> device's own stream?
>
> Thanks
> Peng
>
> > +
> > +		group = iommu_group_get(stream->master->dev);
> > +		break;
> > +	}
> > +	mutex_unlock(&smmu->streams_mutex);
> > +
> > +	if (group)
> > +		return group;
> >
> > -	/*
> > -	 * We don't support devices sharing stream IDs other than PCI RID
> > -	 * aliases, since the necessary ID-to-device lookup becomes rather
> > -	 * impractical given a potential sparse 32-bit stream ID space.
> > -	 */
> > 	if (dev_is_pci(dev))
> > -		group = pci_device_group(dev);
> > -	else
> > -		group = generic_device_group(dev);
> > +		return pci_device_group(dev);
> >
> > -	return group;
> > +	return generic_device_group(dev);
> > }
> >
> > static int arm_smmu_of_xlate(struct device *dev,
> >
> > --
> > 2.34.1