Messages in this thread | | | From | Saravana Kannan <> | Date | Wed, 6 Nov 2019 22:11:55 -0800 | Subject | Re: [PATCH 0/7] iommu: Permit modular builds of ARM SMMU[v3] drivers |
| |
On Mon, Nov 4, 2019 at 5:29 AM Robin Murphy <robin.murphy@arm.com> wrote: > > On 04/11/2019 12:16, John Garry wrote: > > On 01/11/2019 21:13, Saravana Kannan wrote: > >> On Fri, Nov 1, 2019 at 3:28 AM John Garry <john.garry@huawei.com> wrote: > >>> > >>> On 31/10/2019 23:34, Saravana Kannan via iommu wrote: > >>>> I looked into the iommu-map property and it shouldn't be too hard to > >>>> add support for it. Looks like we can simply hold off on probing the > >>>> root bridge device till all the iommus in its iommu-map are probed and > >>>> we should be fine. > >>>> > >>>>> I'm also unsure about distro vendors agreeing to a mandatory kernel > >>>>> parameter (of_devlink). Do you plan to eventually enable it by > >>>>> default? > >>>>> > >>>>>> static const struct supplier_bindings of_supplier_bindings[] = { > >>>>>> { .parse_prop = parse_clocks, }, > >>>>>> { .parse_prop = parse_interconnects, }, > >>>>>> { .parse_prop = parse_regulators, }, > >>>>>> + { .parse_prop = parse_iommus, }, > >>>>>> {}, > >>>>>> }; > >>>>>> > >>>>>> I plan to upstream this pretty soon, but I have other patches in > >>>>>> flight that touch the same file and I'm waiting for those to get > >>>>>> accepted. I also want to clean up the code a bit to reduce some > >>>>>> repetition before I add support for more bindings. > >>>>> I'm also wondering about ACPI support. > >>>> I'd love to add ACPI support too, but I have zero knowledge of ACPI. > >>>> I'd be happy to help anyone who wants to add ACPI support that allows > >>>> ACPI to add device links. > >>> > >>> If possible to add, that may be useful for remedying this: > >>> > >>> https://lore.kernel.org/linux-iommu/9625faf4-48ef-2dd3-d82f-931d9cf26976@huawei.com/ > >>> > >> > >> I'm happy that this change might fix that problem, but isn't the > >> problem reported in that thread more to do with child devices getting > >> added before the parent probes successfully? That doesn't make sense > >> to me. > > > > So the pcieport device and then the child device are added in the PCI > > scan, but only some time later do the device drivers probe for these > > devices; so it's not that the that pcieport driver creates the child > > device. > > > > The problem then occurs in that the ordering the of device driver probe > > is such that we have this: pcieport probe + defer (as no IOMMU group > > registered), SMMU probe (registers the IOMMU group), child device probe, > > pcieport really probe. > > > > Can't the piceport driver not add its child devices before it > >> probes successfully? Or more specifically, who adds the child devices > >> of the pcieport before the pcieport itself probes? > > > > The devices are actually added in order pcieport, child device, but not > > really probed in that same order, as above. > > Right, in short the fundamental problem is that of_iommu_configure() now > does the wrong thing. Deferring probe of the entire host bridge/root > complex based on "iommu-map" would indeed happen to solve the problem by > brute force, I think, but could lead to a dependency cycle for PCI-based > IOMMUs as Jean points out.
Sorry for the late reply. Got caught up on other work.
I didn't think the SMMU itself was PCI based in the example Jean gave. I thought it just happened to be the case where the SMMU probes after the pcieport but before the other children. If the SMMU itself is a child of the pcieport, how can it be required for the parent to function? How will suspend/resume even work?! I feel like I'm missing some context wrt to PCI that everyone else seems to know (which isn't surprising).
> I hope to have time this week to work a bit > more on pulling of_iommu_configure() apart to fix it properly, after > which of_devlink *should* only have to worry about the child devices > themselves...
Worry about child devices in what sense? From a non-iommu perspective?
-Saravana
| |