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

Nicolin Chen
2025-04-12T01:13:27Z

On Fri, Apr 11, 2025 at 08:44:19PM -0300, Jason Gunthorpe wrote:
> On Fri, Apr 11, 2025 at 04:33:44PM -0700, Nicolin Chen wrote:
>
> > > The bridge *does* claim its own RID, and per the aliasing rules the
> > > devices behind it claim both their own RID and the alias to function
> > > 00.0 on the bridge's secondary bus, like so in action:
> >
> > Yea, I just found out that the bridge does have a different SID.
> > It was actually the VGA controller itself having two fwspec->ids
> > populated by the IORT code. Then, the SMMU driver allocated two
> > separate streams with the same set of device pointer and SID:
> > pci 0008:06:00.0: arm_smmu_insert_master: fwspec index=0, sid=0x10600
> > pci 0008:06:00.0: Adding to iommu group 21
> > pci 0008:07:00.0: arm_smmu_insert_master: fwspec index=0, sid=0x10700
> > pci 0008:07:00.0: arm_smmu_insert_master: fwspec index=1, sid=0x10700
> > pci 0008:07:00.0: Adding to iommu group 21
> >
> > Perhaps the duplicated fwspec->id should be avoided in the IORT
> > code at the first place v.s. bypassing the fwspec->ids[1]?
>
> It is a much easier fix if all you have to do is ignore hits in the RB
> tree that match to the same master, just don't fail on duplicates and
> don't add the duplicate rb node at all?
>
> Seem strange though. Where did the duplicate come from?

iort_iommu_configure_id
|=> pci_for_each_dma_alias
|=> iort_pci_iommu_init
| |=> iort_iommu_xlate
| |=> acpi_iommu_fwspec_init
| |=> iommu_fwspec_add_id(sid=0x10700) //index=0
|=> iort_pci_iommu_init
|=> iort_iommu_xlate
|=> acpi_iommu_fwspec_init
|=> iommu_fwspec_add_id(sid=0x10700) //index=1

We could add a piece of code in iommu_fwspec_add_ids() rejecting
a duplicated ID, which would be smaller than we do in SMMU driver
that requires a device_group change too?

Thanks
Nicolin