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

[Nicolin Chen]

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