linux-kernel - Re: [RFT v3] drm: use late_initcall() for amdkfd and radeon

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKMK7uHTAWOmrTsGHxwW1nGcNeEeAb8b2OPFKcJvQ3AXkbs+Kg@mail.gmail.com>
Date:	Sun, 29 May 2016 20:27:07 +0200
From:	Daniel Vetter <daniel@...ll.ch>
To:	"Luis R. Rodriguez" <mcgrof@...nel.org>
Cc:	vw@...mu.org, Oded Gabbay <oded.gabbay@...il.com>,
	Joerg Roedel <joro@...tes.org>,
	Christian König <christian.koenig@....com>,
	"alexander.deucher@....com" <alexander.deucher@....com>,
	Dave Airlie <airlied@...ux.ie>,
	iommu@...ts.linux-foundation.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	dri-devel <dri-devel@...ts.freedesktop.org>
Subject: Re: [RFT v3] drm: use late_initcall() for amdkfd and radeon

On Fri, May 27, 2016 at 3:18 AM, Luis R. Rodriguez <mcgrof@...nel.org> wrote:
> To get KFD support in radeon we need the following
> initialization to happen in this order, their
> respective driver file that has its init routine
> listed next to it:
>
> 0. AMD IOMMUv1:    arch/x86/kernel/pci-dma.c
> 1. AMD IOMMUv2:    drivers/iommu/amd_iommu_v2.c
> 2. AMD KFD:        drivers/gpu/drm/amd/amdkfd/kfd_module.c
> 3. AMD Radeon:     drivers/gpu/drm/radeon/radeon_drv.c
>
> Order is rather implicit, but these drivers can currently
> only do so much given the amount of leg room available.
> Below are the respective init routines and how they are
> initialized:
>
> arch/x86/kernel/pci-dma.c               rootfs_initcall(pci_iommu_init);
> drivers/iommu/amd_iommu_v2.c            module_init(amd_iommu_v2_init);
> drivers/gpu/drm/amd/amdkfd/kfd_module.c module_init(kfd_module_init);
> drivers/gpu/drm/radeon/radeon_drv.c     module_init(radeon_init);
>
> When a driver is built-in module_init() folds to use
> device_initcall(), and we have the following possible
> orders:
>
>         #define pure_initcall(fn)    __define_initcall(fn, 0)
>         #define core_initcall(fn)    __define_initcall(fn, 1)
>         #define postcore_initcall(fn)__define_initcall(fn, 2)
>         #define arch_initcall(fn)    __define_initcall(fn, 3)
>         #define subsys_initcall(fn)  __define_initcall(fn, 4)
>         #define fs_initcall(fn)      __define_initcall(fn, 5)
>         ---------------------------------------------------------
>         #define rootfs_initcall(fn)  __define_initcall(fn, rootfs)
>         #define device_initcall(fn)  __define_initcall(fn, 6)
>         #define late_initcall(fn)    __define_initcall(fn, 7)
>
> Since we start off from rootfs_initcall(), it gives us 3 more
> levels of leg room to play with for order semantics, this isn't
> enough to address all required levels of dependencies, this
> is specially true given that AMD-KFD needs to be loaded before
> the radeon driver -- -but this it not enforced by symbols.
> If the AMD-KFD driver is not loaded prior to the radeon driver
> because otherwise the radeon driver will not initialize the
> AMD-KFD driver and you get no KFD functionality in userspace.
>
> Commit 1bacc894c227fad8a7 ("drivers: Move iommu/ before gpu/ in
> Makefile") works around some of the possibe races between
> the AMD IOMMU v2 and GPU drivers by changing the link order.
> This is fragile, however its the bets we can do, given that
> making the GPU drivers use late_initcall() would also implicate
> a similar race between them. That possible race is fortunatley
> addressed given that the drm Makefile currently has amdkfd
> linked prior to radeon:
>
> drivers/gpu/drm/Makefile
> ...
> obj-$(CONFIG_HSA_AMD) += amd/amdkfd/
> obj-$(CONFIG_DRM_RADEON)+= radeon/
> ...
>
> Changing amdkfd and radeon to late_initcall() however is
> still the right call in orde to annotate explicitly a
> delayed dependency requirement between the GPU drivers
> and the IOMMUs.
>
> We can't address the fragile nature of the link order
> right now, but in the future that might be possible.
>
> Signed-off-by: Luis R. Rodriguez <mcgrof@...nel.org>
> ---
>
> Please note, the changes to drivers/Makefile are just
> for the sake of forcing the possible race to occur,
> if this works well the actual [PATCH] submission will
> skip those changes as its pointless to remove those
> work arounds as it stands, due to the limited nature
> of the levels available for addressing requirements.
>
> Also, if you are aware of further dependency hell
> things like these -- please do let me know as I am
> interested in looking at addressing them.

This should be fixed with -EDEFER_PROBE instead. Frobbing initcall
levels should then just be done as an optimization to avoid too much
reprobing.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch