On Tue, Jan 12, 2021 at 12:57 AM Robin Murphy <robin.murphy@xxxxxxx> wrote:
Thanks to Will and Robin for reviewing this. I am pretty new to PCI,
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.
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.
The pci_endpoint_test picks up the same of_ DMA config node as the PCI RCI 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 ;)
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?
I hope the support will be added soon. Also, can you point me to few driversThere 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@xxxxxxxxxxx>
---
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.
which already handle this type of stream-ID aliasing?