[PATCH V7 01/11] iommu/of: Refactor of_iommu_configure() for error handling
sricharan at codeaurora.org
Tue Jan 31 05:11:44 PST 2017
>>>>>> I gave the whole patch set a try on ThunderX. really_probe() is failing
>>>>>> on dma_configure()->of_pci_iommu_init() for each PCI device.
>>>>> When you say "failing", do you mean cleanly, or with a crash? I've
>>>>> managed to hit __of_match_node() dereferencing NULL from
>>>>> of_iommu_xlate() in a horribly complicated chain of events, which I'm
>>>>> trying to figure out now, and I wonder if the two might be related.
>>>> Sorry that there is crash still. __of_match_node seems to checking
>>>> for NULL arguments , feels like some invalid pointer was passed in.
>>>> Is there any particular sequence to try for this ?
>>> Ah, I did figure it out - it wasn't actually a NULL dereference, but an
>>> unmapped address. Turns out __iommu_of_table is in initdata, so any
>>> driver probing after init, connected to an unprobed IOMMU (in this case
>>> disabled in DT), trips over trying to match the now-freed table. I'm
>>> working on the fix - technically the bug's in my patch (#2) anyway ;)
>> Ok, thanks for bringing this out. There is also an issue that
>> Sinan has mentioned while testing the ACPI hotplug path, probably
>> its related to the above, not sure. I will try to check more on that
>> in the meanwhile. Then, taking your fix and fixing the hotplug case
>> i will do one more repost.
>OK, I've finally settled on the below fixup for patch #2 - I have some
>follow-on ideas for eventually getting of the magic table altogether,
>but they can wait until we've got the baseline functionality sorted.
>Updated full patch here:
Thanks, will use this. Yes, the magic iommu_of_table makes little
use with probe deferral in place, except that we are using it at the
moment. And the other hotplug issue is not related this though.
>diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>index 349bd1d01612..1f92d98237d5 100644
>@@ -96,6 +96,19 @@ int of_get_dma_window(struct device_node *dn, const
>char *prefix, int index,
>+static bool of_iommu_driver_present(struct device_node *np)
>+ * If the IOMMU still isn't ready by the time we reach init, assume
>+ * it never will be. We don't want to defer indefinitely, nor attempt
>+ * to dereference __iommu_of_table after it's been freed.
>+ if (system_state > SYSTEM_BOOTING)
>+ return false;
>+ return of_match_node(&__iommu_of_table, np);
> static const struct iommu_ops
> *of_iommu_xlate(struct device *dev, struct of_phandle_args *iommu_spec)
>@@ -105,7 +118,7 @@ static const struct iommu_ops
> ops = iommu_get_instance(fwnode);
> if ((ops && !ops->of_xlate) ||
>- (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))
>+ (!ops && !of_iommu_driver_present(iommu_spec->np)))
> return NULL;
> err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
>To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
>the body of a message to majordomo at vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
More information about the linux-arm-kernel