public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Felix Kuehling <felix.kuehling@amd.com>
To: "Arnd Bergmann" <arnd@kernel.org>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>
Cc: Arnd Bergmann <arnd@arndb.de>, David Airlie <airlied@linux.ie>,
	Daniel Vetter <daniel@ffwll.ch>,
	Philip Yang <philip.yang@amd.com>,
	Oded Gabbay <oded.gabbay@gmail.com>,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [variant b] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
Date: Mon, 8 Mar 2021 22:20:40 -0500	[thread overview]
Message-ID: <4c692eff-9d57-278e-8da4-36bc2c293506@amd.com> (raw)
In-Reply-To: <20210308204606.2634726-1-arnd@kernel.org>

Am 2021-03-08 um 3:45 p.m. schrieb Arnd Bergmann:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Using 'imply AMD_IOMMU_V2' does not guarantee that the driver can link
> against the exported functions. If the GPU driver is built-in but the
> IOMMU driver is a loadable module, the kfd_iommu.c file is indeed
> built but does not work:
>
> x86_64-linux-ld: drivers/gpu/drm/amd/amdkfd/kfd_iommu.o: in function `kfd_iommu_bind_process_to_device':
> kfd_iommu.c:(.text+0x516): undefined reference to `amd_iommu_bind_pasid'
> x86_64-linux-ld: drivers/gpu/drm/amd/amdkfd/kfd_iommu.o: in function `kfd_iommu_unbind_process':
> kfd_iommu.c:(.text+0x691): undefined reference to `amd_iommu_unbind_pasid'
> x86_64-linux-ld: drivers/gpu/drm/amd/amdkfd/kfd_iommu.o: in function `kfd_iommu_suspend':
> kfd_iommu.c:(.text+0x966): undefined reference to `amd_iommu_set_invalidate_ctx_cb'
> x86_64-linux-ld: kfd_iommu.c:(.text+0x97f): undefined reference to `amd_iommu_set_invalid_ppr_cb'
> x86_64-linux-ld: kfd_iommu.c:(.text+0x9a4): undefined reference to `amd_iommu_free_device'
> x86_64-linux-ld: drivers/gpu/drm/amd/amdkfd/kfd_iommu.o: in function `kfd_iommu_resume':
> kfd_iommu.c:(.text+0xa9a): undefined reference to `amd_iommu_init_device'
> x86_64-linux-ld: kfd_iommu.c:(.text+0xadc): undefined reference to `amd_iommu_set_invalidate_ctx_cb'
> x86_64-linux-ld: kfd_iommu.c:(.text+0xaff): undefined reference to `amd_iommu_set_invalid_ppr_cb'
> x86_64-linux-ld: kfd_iommu.c:(.text+0xc72): undefined reference to `amd_iommu_bind_pasid'
> x86_64-linux-ld: kfd_iommu.c:(.text+0xe08): undefined reference to `amd_iommu_set_invalidate_ctx_cb'
> x86_64-linux-ld: kfd_iommu.c:(.text+0xe26): undefined reference to `amd_iommu_set_invalid_ppr_cb'
> x86_64-linux-ld: kfd_iommu.c:(.text+0xe42): undefined reference to `amd_iommu_free_device'
>
> Change the 'imply' to a weak dependency that still allows compiling
> in all other configurations but disallows the configuration that
> causes a link failure.

I don't like this solution. It hides the HSA_AMD option in menuconfig
and it's not intuitively obvious to someone configuring a kernel why
it's not available. They may not even notice that it's missing and then
wonder later, why KFD functionality is missing in their kernel.

What I'm trying to achieve is, that KFD can be compiled without
AMD-IOMMU-V2 support in this case. I just tested my version using
IS_REACHABLE. I'll reply with that patch in a moment.

Regards,
  Felix


>
> Fixes: 64d1c3a43a6f ("drm/amdkfd: Centralize IOMMUv2 code and make it conditional")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/gpu/drm/amd/amdkfd/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/Kconfig b/drivers/gpu/drm/amd/amdkfd/Kconfig
> index f02c938f75da..d01dba2af3bb 100644
> --- a/drivers/gpu/drm/amd/amdkfd/Kconfig
> +++ b/drivers/gpu/drm/amd/amdkfd/Kconfig
> @@ -6,7 +6,7 @@
>  config HSA_AMD
>  	bool "HSA kernel driver for AMD GPU devices"
>  	depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
> -	imply AMD_IOMMU_V2 if X86_64
> +	depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
>  	select HMM_MIRROR
>  	select MMU_NOTIFIER
>  	select DRM_AMDGPU_USERPTR

  reply	other threads:[~2021-03-09  3:21 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-08 20:45 [PATCH] [variant b] drm/amdkfd: fix build error with missing AMD_IOMMU_V2 Arnd Bergmann
2021-03-09  3:20 ` Felix Kuehling [this message]
2021-03-09  3:23   ` [PATCH 1/1] drm/amdkfd: fix build error with AMD_IOMMU_V2=m Felix Kuehling
2021-03-09  8:58     ` Arnd Bergmann
2021-03-09 16:30       ` Felix Kuehling
2021-03-09 17:33         ` Jean-Philippe Brucker
2021-03-09 17:59           ` Alex Deucher
2021-03-09 18:34             ` Christian König
2021-03-11 10:02               ` Arnd Bergmann
2021-03-09 16:50   ` [PATCH v2 " Felix Kuehling
2021-03-10 22:13     ` Felix Kuehling
2021-03-11  8:50       ` Christian König

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4c692eff-9d57-278e-8da4-36bc2c293506@amd.com \
    --to=felix.kuehling@amd.com \
    --cc=airlied@linux.ie \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=arnd@arndb.de \
    --cc=arnd@kernel.org \
    --cc=christian.koenig@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oded.gabbay@gmail.com \
    --cc=philip.yang@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox