Messages in this thread | | | Date | Thu, 6 Sep 2018 10:13:37 +0200 | From | Lukas Wunner <> | Subject | Re: [PATCH 2/2] thunderbolt: Initialize after IOMMUs |
| |
On Wed, Sep 05, 2018 at 12:46:02PM +0300, Mika Westerberg wrote: > On Wed, Sep 05, 2018 at 10:47:46AM +0200, Lukas Wunner wrote: > > On Mon, Sep 03, 2018 at 04:20:12PM +0300, Mika Westerberg wrote: > > > If IOMMU is enabled and Thunderbolt driver is built into the kernel > > > image, it will be probed before IOMMUs are attached to the PCI bus. > > > Because of this DMA mappings the driver does will not go through IOMMU > > > and start failing right after IOMMUs are enabled. > > > > > > For this reason move the Thunderbolt driver initialization happen at > > > rootfs level. > > > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > --- > > > drivers/thunderbolt/nhi.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c > > > index 88cff05a1808..5cd6bdfa068f 100644 > > > --- a/drivers/thunderbolt/nhi.c > > > +++ b/drivers/thunderbolt/nhi.c > > > @@ -1191,5 +1191,5 @@ static void __exit nhi_unload(void) > > > tb_domain_exit(); > > > } > > > > > > -fs_initcall(nhi_init); > > > +rootfs_initcall(nhi_init); > > > module_exit(nhi_unload); > > > > I think the dependency on the IOMMU should be open coded by returning > > -EPROBE_DEFER from the ->probe hook if it's not yet attached. > > Shuffling around initcall order is just applying duct tape. > > It is not a dependency. The same thing can happen with any other driver > if they happen to initialize any DMA with the device before IOMMUs are > initialized. > > > Commit acb40d841257 already changed module_init() to fs_initcall() > > and now it has to be changed again. Shows how fragile this is. > > It is a bit fragile but I don't see any other way to handle this than > trusting on the link ordering. Both -EPROBE_DEFER and device_links are > out of the question AFAICT.
So with this patch, you rely on the linker ordering nhi_init() after ir_dev_scope_init(), however to the best of my knowledge the link order is not guaranteed.
In that sense, commit acb40d841257 was already flawed because it executed nhi_init() at "fs" initcall level, the *same* level used by map_properties() in drivers/firmware/efi/apple-properties.c, which retrieves the DROM device property needed by tb_drom_copy_efi().
That was arguably a regression which the above patch cures because "rootfs" is guaranteed to run after "fs". Still, the fragility remains that ir_dev_scope_init() isn't guaranteed to run before nhi_init().
Looking at commit acb40d841257, which started this, I'm wondering why you did not simply export tbnet_init() and call it from the thunderbolt driver after the property stuff has been fully set up? After all, thunderbolt-net is useless without thunderbolt or am I missing something? Then you could revert back to module_init().
Thanks,
Lukas
| |