Re: [PATCH 0/7] iommu: Permit modular builds of ARM SMMU[v3] drivers

[Robin Murphy <robin.murphy@arm.com>]

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. 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...

Robin.

> I'll add you to that thread if you want to discuss further.
> 
> Thanks,
> John
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu