Messages in this thread | | | From | Ajay Kumar <> | Date | Wed, 13 Jan 2021 01:59:37 +0530 | Subject | Re: [PATCH] iommu/arm-smmu-v3: Handle duplicated Stream IDs from other masters |
| |
On Tue, Jan 12, 2021 at 12:57 AM Robin Murphy <robin.murphy@arm.com> wrote: > > On 2021-01-07 13:03, Will Deacon wrote: > > On Thu, Jan 07, 2021 at 03:03:40PM +0530, Ajay Kumar wrote: > >> When PCI function drivers(ex:pci-endpoint-test) are probed for already > >> initialized PCIe-RC(Root Complex), and PCIe-RC is already bound to SMMU, > >> then we encounter a situation where the function driver tries to attach > >> itself to the smmu with the same stream-id as PCIe-RC and re-initialize > >> an already initialized STE. This causes ste_live BUG_ON() in the driver. > > Note that this is actually expected behaviour, since Stream ID aliasing > has remained officially not supported until a sufficiently compelling > reason to do so appears. I always thought the most likely scenario would > be a legacy PCI bridge with multiple devices behind it, but even that > seems increasingly improbable for a modern SMMUv3-based system to ever see. Thanks to Will and Robin for reviewing this. I am pretty new to PCI, sorry about that. I assumed that the support for stream-id alias is already handled as part of this patch: https://www.spinics.net/lists/arm-kernel/msg626087.html which prevents STE re-initialization. But, what I do not understand is why the path taken by the arm-smmu-v3 driver misses the aforementioned check for my usecase.
> > I don't understand why the endpoint is using the same stream ID as the root > > complex in this case. Why is that? Is the grouping logic not working > > properly? > > It's not so much that it isn't working properly, it's more that it needs > to be implemented at all ;) The pci_endpoint_test picks up the same of_ DMA config node as the PCI RC because they sit on the same PCI bus [via pci_dma_configure( )] While in the arm-smmu-v3 driver, I can see that the pci_device_group( ) hands over the same iommu group as the Root Complex to the newly added master device (pci_endpoint_test in our case) because they share the same stream ID. Shouldn't they?
> >> There is an already existing check in the driver to manage duplicated ids > >> if duplicated ids are added in same master device, but there can be > >> scenarios like above where we need to extend the check for other masters > >> using the same stream-id. > >> > >> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> > >> --- > >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 33 +++++++++++++++++++++ > >> 1 file changed, 33 insertions(+) > > > > It doesn't feel like the driver is the right place to fix this, as the same > > issue could surely occur for other IOMMUs too, right? In which case, I think > > we should avoid getting into the situation where different groups have > > overlapping stream IDs. > > Yes, this patch does not represent the correct thing to do either way. > The main reason that Stream ID aliasing hasn't been supported so far is > that the required Stream ID to group lookup is rather awkward, and > adding all of that complexity just for the sake of a rather unlikely > possibility seemed dubious. However, PRI support has always had a more > pressing need to implement almost the same thing (Stream ID to device), > so once that lands we can finally get round to adding the rest of proper > group support relatively easily. I hope the support will be added soon. Also, can you point me to few drivers which already handle this type of stream-ID aliasing?
Thanks, Ajay Kumar > ______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu
| |