On 12/26/2014 11:19 AM, Laurent Pinchart wrote:
Hi Thierry,
On Thursday 25 December 2014 14:20:59 Thierry Reding wrote:
On Mon, Dec 22, 2014 at 01:07:13PM +0200, Oded Gabbay wrote:
This small patch-set, was created to solve the bug described at
https://bugzilla.kernel.org/show_bug.cgi?id=89661 (Kernel panic when
trying use amdkfd driver on Kaveri). It replaces the previous patch-set
called [PATCH 0/3] Use workqueue for device init in amdkfd
(http://lists.freedesktop.org/archives/dri-devel/2014-December/074401.html
)
That bug appears only when radeon, amdkfd and amd_iommu_v2 are compiled
inside the kernel (not as modules). In that case, the correct loading
order, as determined by the exported symbol used by each driver, is
not enforced anymore and the kernel loads them based on who was linked
first. That makes radeon load first, amdkfd second and amd_iommu_v2
third.
Because the initialization of a device in amdkfd is initiated by radeon,
and can only be completed if amdkfd and amd_iommu_v2 were loaded and
initialized, then in the case mentioned above, this initalization fails
and there is a kernel panic as some pointers are not initialized but
used nontheless.
To solve this bug, this patch-set moves iommu/ before gpu/ in
drivers/Makefile and also moves amdkfd/ before radeon/ in
drivers/gpu/drm/Makefile.
The rationale is that in general, AMD GPU devices are dependent on AMD
IOMMU controller functionality to allow the GPU to access a process's
virtual memory address space, without the need for pinning the memory.
That's why it makes sense to initialize the iommu/ subsystem ahead of the
gpu/ subsystem.
I strongly object to this patch set. This makes assumptions about how
the build system influences probe order. That's bad because seemingly
unrelated changes could easily break this in the future.
We already have ways to solve this kind of dependency (driver probe
deferral), and I think you should be using it to solve this particular
problem rather than some linking order hack.
While I agree with you that probe deferral is the way to go, I believe linkage
ordering can still be used as an optimization to avoid deferring probe in the
most common cases. I'm thus not opposed to moving iommu/ earlier in link order
(provided we can properly test for side effects, as the jump is pretty large),
but not as a replacement for probe deferral.
My thoughts exactly. If this was some extreme use case, than it would be
justified to solve it with probe deferral. But I think that for most common
cases, GPU are dependent on IOMMU and *not* vice-versa.
BTW, my first try at solving this was to use probe deferral (using workqueue),
but the feedback I got from Christian and Dave was that moving iommu/ linkage
before gpu/ was a much more simpler solution.
In addition, Linus said he doesn't object to this "band-aid". See:
https://lkml.org/lkml/2014/12/25/152
Oded
Coincidentally there's a separate thread currently going on that deals
with IOMMUs and probe order. The solution being worked on is currently
somewhat ARM-specific, so adding a couple of folks for visibility. It
looks like we're going to need something more generic since this is a
problem that even the "big" architectures need to solve.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
