All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 15:33 ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2021-03-08 15:33 UTC (permalink / raw)
  To: Felix Kuehling, Alex Deucher, Christian König, David Airlie,
	Daniel Vetter, Oded Gabbay
  Cc: Arnd Bergmann, Colin Ian King, Philip Yang, amd-gfx, dri-devel,
	linux-kernel

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'

Use a stronger 'select' instead.

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 | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/Kconfig b/drivers/gpu/drm/amd/amdkfd/Kconfig
index f02c938f75da..91f85dfb7ba6 100644
--- a/drivers/gpu/drm/amd/amdkfd/Kconfig
+++ b/drivers/gpu/drm/amd/amdkfd/Kconfig
@@ -5,8 +5,9 @@
 
 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 DRM_AMDGPU && ((X86_64 && IOMMU_SUPPORT && ACPI) || ARM64 || PPC64)
+	select AMD_IOMMU if X86_64
+	select AMD_IOMMU_V2 if X86_64
 	select HMM_MIRROR
 	select MMU_NOTIFIER
 	select DRM_AMDGPU_USERPTR
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 15:33 ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2021-03-08 15:33 UTC (permalink / raw)
  To: Felix Kuehling, Alex Deucher, Christian König, David Airlie,
	Daniel Vetter, Oded Gabbay
  Cc: Philip Yang, Arnd Bergmann, linux-kernel, dri-devel, amd-gfx,
	Colin Ian King

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'

Use a stronger 'select' instead.

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 | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/Kconfig b/drivers/gpu/drm/amd/amdkfd/Kconfig
index f02c938f75da..91f85dfb7ba6 100644
--- a/drivers/gpu/drm/amd/amdkfd/Kconfig
+++ b/drivers/gpu/drm/amd/amdkfd/Kconfig
@@ -5,8 +5,9 @@
 
 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 DRM_AMDGPU && ((X86_64 && IOMMU_SUPPORT && ACPI) || ARM64 || PPC64)
+	select AMD_IOMMU if X86_64
+	select AMD_IOMMU_V2 if X86_64
 	select HMM_MIRROR
 	select MMU_NOTIFIER
 	select DRM_AMDGPU_USERPTR
-- 
2.29.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 15:33 ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2021-03-08 15:33 UTC (permalink / raw)
  To: Felix Kuehling, Alex Deucher, Christian König, David Airlie,
	Daniel Vetter, Oded Gabbay
  Cc: Philip Yang, Arnd Bergmann, linux-kernel, dri-devel, amd-gfx,
	Colin Ian King

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'

Use a stronger 'select' instead.

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 | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/Kconfig b/drivers/gpu/drm/amd/amdkfd/Kconfig
index f02c938f75da..91f85dfb7ba6 100644
--- a/drivers/gpu/drm/amd/amdkfd/Kconfig
+++ b/drivers/gpu/drm/amd/amdkfd/Kconfig
@@ -5,8 +5,9 @@
 
 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 DRM_AMDGPU && ((X86_64 && IOMMU_SUPPORT && ACPI) || ARM64 || PPC64)
+	select AMD_IOMMU if X86_64
+	select AMD_IOMMU_V2 if X86_64
 	select HMM_MIRROR
 	select MMU_NOTIFIER
 	select DRM_AMDGPU_USERPTR
-- 
2.29.2

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
  2021-03-08 15:33 ` Arnd Bergmann
  (?)
@ 2021-03-08 16:24   ` Felix Kuehling
  -1 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2021-03-08 16:24 UTC (permalink / raw)
  To: Arnd Bergmann, Alex Deucher, Christian König, David Airlie,
	Daniel Vetter, Oded Gabbay
  Cc: Arnd Bergmann, Colin Ian King, Philip Yang, amd-gfx, dri-devel,
	linux-kernel

The driver build should work without IOMMUv2. In amdkfd/Makefile, we
have this condition:

ifneq ($(CONFIG_AMD_IOMMU_V2),)
AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
endif

In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
causing your link-failures if IOMMU_V2 is not enabled:

#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
... function declarations ...
#else
... stubs ...
#endif

Regards,
  Felix

Am 2021-03-08 um 10:33 a.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'
>
> Use a stronger 'select' instead.
>
> 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 | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/Kconfig b/drivers/gpu/drm/amd/amdkfd/Kconfig
> index f02c938f75da..91f85dfb7ba6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/Kconfig
> +++ b/drivers/gpu/drm/amd/amdkfd/Kconfig
> @@ -5,8 +5,9 @@
>  
>  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 DRM_AMDGPU && ((X86_64 && IOMMU_SUPPORT && ACPI) || ARM64 || PPC64)
> +	select AMD_IOMMU if X86_64
> +	select AMD_IOMMU_V2 if X86_64
>  	select HMM_MIRROR
>  	select MMU_NOTIFIER
>  	select DRM_AMDGPU_USERPTR

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 16:24   ` Felix Kuehling
  0 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2021-03-08 16:24 UTC (permalink / raw)
  To: Arnd Bergmann, Alex Deucher, Christian König, David Airlie,
	Daniel Vetter, Oded Gabbay
  Cc: Philip Yang, Arnd Bergmann, linux-kernel, dri-devel, amd-gfx,
	Colin Ian King

The driver build should work without IOMMUv2. In amdkfd/Makefile, we
have this condition:

ifneq ($(CONFIG_AMD_IOMMU_V2),)
AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
endif

In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
causing your link-failures if IOMMU_V2 is not enabled:

#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
... function declarations ...
#else
... stubs ...
#endif

Regards,
  Felix

Am 2021-03-08 um 10:33 a.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'
>
> Use a stronger 'select' instead.
>
> 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 | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/Kconfig b/drivers/gpu/drm/amd/amdkfd/Kconfig
> index f02c938f75da..91f85dfb7ba6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/Kconfig
> +++ b/drivers/gpu/drm/amd/amdkfd/Kconfig
> @@ -5,8 +5,9 @@
>  
>  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 DRM_AMDGPU && ((X86_64 && IOMMU_SUPPORT && ACPI) || ARM64 || PPC64)
> +	select AMD_IOMMU if X86_64
> +	select AMD_IOMMU_V2 if X86_64
>  	select HMM_MIRROR
>  	select MMU_NOTIFIER
>  	select DRM_AMDGPU_USERPTR
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 16:24   ` Felix Kuehling
  0 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2021-03-08 16:24 UTC (permalink / raw)
  To: Arnd Bergmann, Alex Deucher, Christian König, David Airlie,
	Daniel Vetter, Oded Gabbay
  Cc: Philip Yang, Arnd Bergmann, linux-kernel, dri-devel, amd-gfx,
	Colin Ian King

The driver build should work without IOMMUv2. In amdkfd/Makefile, we
have this condition:

ifneq ($(CONFIG_AMD_IOMMU_V2),)
AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
endif

In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
causing your link-failures if IOMMU_V2 is not enabled:

#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
... function declarations ...
#else
... stubs ...
#endif

Regards,
  Felix

Am 2021-03-08 um 10:33 a.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'
>
> Use a stronger 'select' instead.
>
> 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 | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/Kconfig b/drivers/gpu/drm/amd/amdkfd/Kconfig
> index f02c938f75da..91f85dfb7ba6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/Kconfig
> +++ b/drivers/gpu/drm/amd/amdkfd/Kconfig
> @@ -5,8 +5,9 @@
>  
>  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 DRM_AMDGPU && ((X86_64 && IOMMU_SUPPORT && ACPI) || ARM64 || PPC64)
> +	select AMD_IOMMU if X86_64
> +	select AMD_IOMMU_V2 if X86_64
>  	select HMM_MIRROR
>  	select MMU_NOTIFIER
>  	select DRM_AMDGPU_USERPTR
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
  2021-03-08 16:24   ` Felix Kuehling
  (?)
@ 2021-03-08 19:05     ` Arnd Bergmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2021-03-08 19:05 UTC (permalink / raw)
  To: Felix Kuehling
  Cc: Alex Deucher, Christian König, David Airlie, Daniel Vetter,
	Oded Gabbay, Colin Ian King, Philip Yang, amd-gfx list,
	dri-devel, linux-kernel

On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>
> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
> have this condition:
>
> ifneq ($(CONFIG_AMD_IOMMU_V2),)
> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
> endif
>
> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
> causing your link-failures if IOMMU_V2 is not enabled:
>
> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
> ... function declarations ...
> #else
> ... stubs ...
> #endif

Right, that is the problem I tried to explain in my patch description.

Should we just drop the 'imply' then and add a proper dependency like this?

      depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
      depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m

I can send a v2 after some testing if you prefer this version.

        Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 19:05     ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2021-03-08 19:05 UTC (permalink / raw)
  To: Felix Kuehling
  Cc: Philip Yang, David Airlie, linux-kernel, amd-gfx list, dri-devel,
	Alex Deucher, Colin Ian King, Christian König

On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>
> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
> have this condition:
>
> ifneq ($(CONFIG_AMD_IOMMU_V2),)
> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
> endif
>
> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
> causing your link-failures if IOMMU_V2 is not enabled:
>
> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
> ... function declarations ...
> #else
> ... stubs ...
> #endif

Right, that is the problem I tried to explain in my patch description.

Should we just drop the 'imply' then and add a proper dependency like this?

      depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
      depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m

I can send a v2 after some testing if you prefer this version.

        Arnd
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 19:05     ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2021-03-08 19:05 UTC (permalink / raw)
  To: Felix Kuehling
  Cc: Oded Gabbay, Philip Yang, David Airlie, linux-kernel,
	amd-gfx list, dri-devel, Daniel Vetter, Alex Deucher,
	Colin Ian King, Christian König

On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>
> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
> have this condition:
>
> ifneq ($(CONFIG_AMD_IOMMU_V2),)
> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
> endif
>
> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
> causing your link-failures if IOMMU_V2 is not enabled:
>
> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
> ... function declarations ...
> #else
> ... stubs ...
> #endif

Right, that is the problem I tried to explain in my patch description.

Should we just drop the 'imply' then and add a proper dependency like this?

      depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
      depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m

I can send a v2 after some testing if you prefer this version.

        Arnd
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
  2021-03-08 19:05     ` Arnd Bergmann
  (?)
@ 2021-03-08 19:09       ` Felix Kuehling
  -1 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2021-03-08 19:09 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Alex Deucher, Christian König, David Airlie, Daniel Vetter,
	Oded Gabbay, Colin Ian King, Philip Yang, amd-gfx list,
	dri-devel, linux-kernel

Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
> On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
>> have this condition:
>>
>> ifneq ($(CONFIG_AMD_IOMMU_V2),)
>> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
>> endif
>>
>> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
>> causing your link-failures if IOMMU_V2 is not enabled:
>>
>> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>> ... function declarations ...
>> #else
>> ... stubs ...
>> #endif
> Right, that is the problem I tried to explain in my patch description.
>
> Should we just drop the 'imply' then and add a proper dependency like this?
>
>       depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
>       depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
>
> I can send a v2 after some testing if you prefer this version.

No. My point is, there should not be a hard dependency. The build should
work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
working for you. It looks like you're building kfd_iommu.o, which should
not be happening when AMD_IOMMU_V2 is not enabled. The condition in
amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
your kernel config.

Regards,
  Felix


>
>         Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 19:09       ` Felix Kuehling
  0 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2021-03-08 19:09 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Philip Yang, David Airlie, linux-kernel, amd-gfx list, dri-devel,
	Alex Deucher, Colin Ian King, Christian König

Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
> On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
>> have this condition:
>>
>> ifneq ($(CONFIG_AMD_IOMMU_V2),)
>> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
>> endif
>>
>> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
>> causing your link-failures if IOMMU_V2 is not enabled:
>>
>> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>> ... function declarations ...
>> #else
>> ... stubs ...
>> #endif
> Right, that is the problem I tried to explain in my patch description.
>
> Should we just drop the 'imply' then and add a proper dependency like this?
>
>       depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
>       depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
>
> I can send a v2 after some testing if you prefer this version.

No. My point is, there should not be a hard dependency. The build should
work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
working for you. It looks like you're building kfd_iommu.o, which should
not be happening when AMD_IOMMU_V2 is not enabled. The condition in
amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
your kernel config.

Regards,
  Felix


>
>         Arnd
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 19:09       ` Felix Kuehling
  0 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2021-03-08 19:09 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Oded Gabbay, Philip Yang, David Airlie, linux-kernel,
	amd-gfx list, dri-devel, Daniel Vetter, Alex Deucher,
	Colin Ian King, Christian König

Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
> On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
>> have this condition:
>>
>> ifneq ($(CONFIG_AMD_IOMMU_V2),)
>> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
>> endif
>>
>> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
>> causing your link-failures if IOMMU_V2 is not enabled:
>>
>> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>> ... function declarations ...
>> #else
>> ... stubs ...
>> #endif
> Right, that is the problem I tried to explain in my patch description.
>
> Should we just drop the 'imply' then and add a proper dependency like this?
>
>       depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
>       depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
>
> I can send a v2 after some testing if you prefer this version.

No. My point is, there should not be a hard dependency. The build should
work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
working for you. It looks like you're building kfd_iommu.o, which should
not be happening when AMD_IOMMU_V2 is not enabled. The condition in
amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
your kernel config.

Regards,
  Felix


>
>         Arnd
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
  2021-03-08 19:09       ` Felix Kuehling
  (?)
@ 2021-03-08 19:33         ` Arnd Bergmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2021-03-08 19:33 UTC (permalink / raw)
  To: Felix Kuehling
  Cc: Alex Deucher, Christian König, David Airlie, Daniel Vetter,
	Oded Gabbay, Colin Ian King, Philip Yang, amd-gfx list,
	dri-devel, linux-kernel

On Mon, Mar 8, 2021 at 8:11 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>
> Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
> > On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
> >> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
> >> have this condition:
> >>
> >> ifneq ($(CONFIG_AMD_IOMMU_V2),)
> >> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
> >> endif
> >>
> >> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
> >> causing your link-failures if IOMMU_V2 is not enabled:
> >>
> >> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
> >> ... function declarations ...
> >> #else
> >> ... stubs ...
> >> #endif
> > Right, that is the problem I tried to explain in my patch description.
> >
> > Should we just drop the 'imply' then and add a proper dependency like this?
> >
> >       depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
> >       depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
> >
> > I can send a v2 after some testing if you prefer this version.
>
> No. My point is, there should not be a hard dependency. The build should
> work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
> working for you. It looks like you're building kfd_iommu.o, which should
> not be happening when AMD_IOMMU_V2 is not enabled. The condition in
> amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
> your kernel config.

Again, as I explained in the changelog text, AMD_IOMMU_V2 configured as
a loadable module, while AMDGPU is configured as built-in.

The causes a link failure for the vmlinux file, because the linker cannot
resolve addresses of loadable modules at compile time -- they have
not been loaded yet.

      Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 19:33         ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2021-03-08 19:33 UTC (permalink / raw)
  To: Felix Kuehling
  Cc: Philip Yang, David Airlie, linux-kernel, amd-gfx list, dri-devel,
	Alex Deucher, Colin Ian King, Christian König

On Mon, Mar 8, 2021 at 8:11 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>
> Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
> > On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
> >> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
> >> have this condition:
> >>
> >> ifneq ($(CONFIG_AMD_IOMMU_V2),)
> >> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
> >> endif
> >>
> >> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
> >> causing your link-failures if IOMMU_V2 is not enabled:
> >>
> >> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
> >> ... function declarations ...
> >> #else
> >> ... stubs ...
> >> #endif
> > Right, that is the problem I tried to explain in my patch description.
> >
> > Should we just drop the 'imply' then and add a proper dependency like this?
> >
> >       depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
> >       depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
> >
> > I can send a v2 after some testing if you prefer this version.
>
> No. My point is, there should not be a hard dependency. The build should
> work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
> working for you. It looks like you're building kfd_iommu.o, which should
> not be happening when AMD_IOMMU_V2 is not enabled. The condition in
> amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
> your kernel config.

Again, as I explained in the changelog text, AMD_IOMMU_V2 configured as
a loadable module, while AMDGPU is configured as built-in.

The causes a link failure for the vmlinux file, because the linker cannot
resolve addresses of loadable modules at compile time -- they have
not been loaded yet.

      Arnd
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 19:33         ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2021-03-08 19:33 UTC (permalink / raw)
  To: Felix Kuehling
  Cc: Oded Gabbay, Philip Yang, David Airlie, linux-kernel,
	amd-gfx list, dri-devel, Daniel Vetter, Alex Deucher,
	Colin Ian King, Christian König

On Mon, Mar 8, 2021 at 8:11 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>
> Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
> > On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
> >> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
> >> have this condition:
> >>
> >> ifneq ($(CONFIG_AMD_IOMMU_V2),)
> >> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
> >> endif
> >>
> >> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
> >> causing your link-failures if IOMMU_V2 is not enabled:
> >>
> >> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
> >> ... function declarations ...
> >> #else
> >> ... stubs ...
> >> #endif
> > Right, that is the problem I tried to explain in my patch description.
> >
> > Should we just drop the 'imply' then and add a proper dependency like this?
> >
> >       depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
> >       depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
> >
> > I can send a v2 after some testing if you prefer this version.
>
> No. My point is, there should not be a hard dependency. The build should
> work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
> working for you. It looks like you're building kfd_iommu.o, which should
> not be happening when AMD_IOMMU_V2 is not enabled. The condition in
> amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
> your kernel config.

Again, as I explained in the changelog text, AMD_IOMMU_V2 configured as
a loadable module, while AMDGPU is configured as built-in.

The causes a link failure for the vmlinux file, because the linker cannot
resolve addresses of loadable modules at compile time -- they have
not been loaded yet.

      Arnd
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
  2021-03-08 19:33         ` Arnd Bergmann
  (?)
@ 2021-03-08 20:02           ` Felix Kuehling
  -1 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2021-03-08 20:02 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Philip Yang, David Airlie, linux-kernel, amd-gfx list, dri-devel,
	Alex Deucher, Colin Ian King, Christian König

Am 2021-03-08 um 2:33 p.m. schrieb Arnd Bergmann:
> On Mon, Mar 8, 2021 at 8:11 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>> Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
>>> On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>>>> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
>>>> have this condition:
>>>>
>>>> ifneq ($(CONFIG_AMD_IOMMU_V2),)
>>>> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
>>>> endif
>>>>
>>>> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
>>>> causing your link-failures if IOMMU_V2 is not enabled:
>>>>
>>>> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>>> ... function declarations ...
>>>> #else
>>>> ... stubs ...
>>>> #endif
>>> Right, that is the problem I tried to explain in my patch description.
>>>
>>> Should we just drop the 'imply' then and add a proper dependency like this?
>>>
>>>       depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
>>>       depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
>>>
>>> I can send a v2 after some testing if you prefer this version.
>> No. My point is, there should not be a hard dependency. The build should
>> work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
>> working for you. It looks like you're building kfd_iommu.o, which should
>> not be happening when AMD_IOMMU_V2 is not enabled. The condition in
>> amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
>> your kernel config.
> Again, as I explained in the changelog text, AMD_IOMMU_V2 configured as
> a loadable module, while AMDGPU is configured as built-in.
I'm sorry, I didn't read it carefully. And I thought "imply" was meant
to fix exactly this kind of issue.

I don't want to create a hard dependency on AMD_IOMMU_V2 if I can avoid
it, because it is only really needed for a small number of AMD APUs, and
even there it is now optional for more recent ones.

Is there a better way to avoid build failures without creating a hard
dependency?  The documentation in
Documentation/kbuild/kconfig-language.rst suggests using if
(IS_REACHABLE(CONFIG_AMD_IOMMU_V2)) to guard those problematic function
calls. I think more generally, we could guard all of kfd_iommu.c with

    #if IS_REACHABLE(CONFIG_AMD_IOMMU_V2)

And use the same condition to define the stubs in kfd_iommu.h.

Regards,
  Felix


>
> The causes a link failure for the vmlinux file, because the linker cannot
> resolve addresses of loadable modules at compile time -- they have
> not been loaded yet.
>
>       Arnd
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 20:02           ` Felix Kuehling
  0 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2021-03-08 20:02 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Philip Yang, David Airlie, linux-kernel, dri-devel, amd-gfx list,
	Alex Deucher, Colin Ian King, Christian König

Am 2021-03-08 um 2:33 p.m. schrieb Arnd Bergmann:
> On Mon, Mar 8, 2021 at 8:11 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>> Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
>>> On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>>>> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
>>>> have this condition:
>>>>
>>>> ifneq ($(CONFIG_AMD_IOMMU_V2),)
>>>> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
>>>> endif
>>>>
>>>> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
>>>> causing your link-failures if IOMMU_V2 is not enabled:
>>>>
>>>> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>>> ... function declarations ...
>>>> #else
>>>> ... stubs ...
>>>> #endif
>>> Right, that is the problem I tried to explain in my patch description.
>>>
>>> Should we just drop the 'imply' then and add a proper dependency like this?
>>>
>>>       depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
>>>       depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
>>>
>>> I can send a v2 after some testing if you prefer this version.
>> No. My point is, there should not be a hard dependency. The build should
>> work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
>> working for you. It looks like you're building kfd_iommu.o, which should
>> not be happening when AMD_IOMMU_V2 is not enabled. The condition in
>> amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
>> your kernel config.
> Again, as I explained in the changelog text, AMD_IOMMU_V2 configured as
> a loadable module, while AMDGPU is configured as built-in.
I'm sorry, I didn't read it carefully. And I thought "imply" was meant
to fix exactly this kind of issue.

I don't want to create a hard dependency on AMD_IOMMU_V2 if I can avoid
it, because it is only really needed for a small number of AMD APUs, and
even there it is now optional for more recent ones.

Is there a better way to avoid build failures without creating a hard
dependency?  The documentation in
Documentation/kbuild/kconfig-language.rst suggests using if
(IS_REACHABLE(CONFIG_AMD_IOMMU_V2)) to guard those problematic function
calls. I think more generally, we could guard all of kfd_iommu.c with

    #if IS_REACHABLE(CONFIG_AMD_IOMMU_V2)

And use the same condition to define the stubs in kfd_iommu.h.

Regards,
  Felix


>
> The causes a link failure for the vmlinux file, because the linker cannot
> resolve addresses of loadable modules at compile time -- they have
> not been loaded yet.
>
>       Arnd
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 20:02           ` Felix Kuehling
  0 siblings, 0 replies; 24+ messages in thread
From: Felix Kuehling @ 2021-03-08 20:02 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Philip Yang, David Airlie, linux-kernel, dri-devel, amd-gfx list,
	Alex Deucher, Colin Ian King, Christian König

Am 2021-03-08 um 2:33 p.m. schrieb Arnd Bergmann:
> On Mon, Mar 8, 2021 at 8:11 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>> Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
>>> On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>>>> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
>>>> have this condition:
>>>>
>>>> ifneq ($(CONFIG_AMD_IOMMU_V2),)
>>>> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
>>>> endif
>>>>
>>>> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
>>>> causing your link-failures if IOMMU_V2 is not enabled:
>>>>
>>>> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>>> ... function declarations ...
>>>> #else
>>>> ... stubs ...
>>>> #endif
>>> Right, that is the problem I tried to explain in my patch description.
>>>
>>> Should we just drop the 'imply' then and add a proper dependency like this?
>>>
>>>       depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
>>>       depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
>>>
>>> I can send a v2 after some testing if you prefer this version.
>> No. My point is, there should not be a hard dependency. The build should
>> work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
>> working for you. It looks like you're building kfd_iommu.o, which should
>> not be happening when AMD_IOMMU_V2 is not enabled. The condition in
>> amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
>> your kernel config.
> Again, as I explained in the changelog text, AMD_IOMMU_V2 configured as
> a loadable module, while AMDGPU is configured as built-in.
I'm sorry, I didn't read it carefully. And I thought "imply" was meant
to fix exactly this kind of issue.

I don't want to create a hard dependency on AMD_IOMMU_V2 if I can avoid
it, because it is only really needed for a small number of AMD APUs, and
even there it is now optional for more recent ones.

Is there a better way to avoid build failures without creating a hard
dependency?  The documentation in
Documentation/kbuild/kconfig-language.rst suggests using if
(IS_REACHABLE(CONFIG_AMD_IOMMU_V2)) to guard those problematic function
calls. I think more generally, we could guard all of kfd_iommu.c with

    #if IS_REACHABLE(CONFIG_AMD_IOMMU_V2)

And use the same condition to define the stubs in kfd_iommu.h.

Regards,
  Felix


>
> The causes a link failure for the vmlinux file, because the linker cannot
> resolve addresses of loadable modules at compile time -- they have
> not been loaded yet.
>
>       Arnd
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
  2021-03-08 20:02           ` Felix Kuehling
  (?)
@ 2021-03-08 20:12             ` Christian König
  -1 siblings, 0 replies; 24+ messages in thread
From: Christian König @ 2021-03-08 20:12 UTC (permalink / raw)
  To: Felix Kuehling, Arnd Bergmann
  Cc: Philip Yang, David Airlie, linux-kernel, dri-devel, amd-gfx list,
	Alex Deucher, Colin Ian King, Christian König

Am 08.03.21 um 21:02 schrieb Felix Kuehling:
> Am 2021-03-08 um 2:33 p.m. schrieb Arnd Bergmann:
>> On Mon, Mar 8, 2021 at 8:11 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>>> Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
>>>> On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>>>>> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
>>>>> have this condition:
>>>>>
>>>>> ifneq ($(CONFIG_AMD_IOMMU_V2),)
>>>>> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
>>>>> endif
>>>>>
>>>>> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
>>>>> causing your link-failures if IOMMU_V2 is not enabled:
>>>>>
>>>>> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>>>> ... function declarations ...
>>>>> #else
>>>>> ... stubs ...
>>>>> #endif
>>>> Right, that is the problem I tried to explain in my patch description.
>>>>
>>>> Should we just drop the 'imply' then and add a proper dependency like this?
>>>>
>>>>        depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
>>>>        depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
>>>>
>>>> I can send a v2 after some testing if you prefer this version.
>>> No. My point is, there should not be a hard dependency. The build should
>>> work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
>>> working for you. It looks like you're building kfd_iommu.o, which should
>>> not be happening when AMD_IOMMU_V2 is not enabled. The condition in
>>> amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
>>> your kernel config.
>> Again, as I explained in the changelog text, AMD_IOMMU_V2 configured as
>> a loadable module, while AMDGPU is configured as built-in.
> I'm sorry, I didn't read it carefully. And I thought "imply" was meant
> to fix exactly this kind of issue.
>
> I don't want to create a hard dependency on AMD_IOMMU_V2 if I can avoid
> it, because it is only really needed for a small number of AMD APUs, and
> even there it is now optional for more recent ones.
>
> Is there a better way to avoid build failures without creating a hard
> dependency?

What you need is the same trick we used for AGP on radeon/nouveau:

depends on AMD_IOMMU_V2 || !AMD_IOMMU_V2

This way when AMD_IOMMU_V2 is build as a module DRM_AMDGPU will be build 
as a module as well. When it is disabled completely we don't care.

Regards,
Christian.

>    The documentation in
> Documentation/kbuild/kconfig-language.rst suggests using if
> (IS_REACHABLE(CONFIG_AMD_IOMMU_V2)) to guard those problematic function
> calls. I think more generally, we could guard all of kfd_iommu.c with
>
>      #if IS_REACHABLE(CONFIG_AMD_IOMMU_V2)
>
> And use the same condition to define the stubs in kfd_iommu.h.
>
> Regards,
>    Felix
>
>
>> The causes a link failure for the vmlinux file, because the linker cannot
>> resolve addresses of loadable modules at compile time -- they have
>> not been loaded yet.
>>
>>        Arnd
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 20:12             ` Christian König
  0 siblings, 0 replies; 24+ messages in thread
From: Christian König @ 2021-03-08 20:12 UTC (permalink / raw)
  To: Felix Kuehling, Arnd Bergmann
  Cc: Philip Yang, David Airlie, linux-kernel, amd-gfx list, dri-devel,
	Alex Deucher, Colin Ian King, Christian König

Am 08.03.21 um 21:02 schrieb Felix Kuehling:
> Am 2021-03-08 um 2:33 p.m. schrieb Arnd Bergmann:
>> On Mon, Mar 8, 2021 at 8:11 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>>> Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
>>>> On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>>>>> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
>>>>> have this condition:
>>>>>
>>>>> ifneq ($(CONFIG_AMD_IOMMU_V2),)
>>>>> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
>>>>> endif
>>>>>
>>>>> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
>>>>> causing your link-failures if IOMMU_V2 is not enabled:
>>>>>
>>>>> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>>>> ... function declarations ...
>>>>> #else
>>>>> ... stubs ...
>>>>> #endif
>>>> Right, that is the problem I tried to explain in my patch description.
>>>>
>>>> Should we just drop the 'imply' then and add a proper dependency like this?
>>>>
>>>>        depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
>>>>        depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
>>>>
>>>> I can send a v2 after some testing if you prefer this version.
>>> No. My point is, there should not be a hard dependency. The build should
>>> work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
>>> working for you. It looks like you're building kfd_iommu.o, which should
>>> not be happening when AMD_IOMMU_V2 is not enabled. The condition in
>>> amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
>>> your kernel config.
>> Again, as I explained in the changelog text, AMD_IOMMU_V2 configured as
>> a loadable module, while AMDGPU is configured as built-in.
> I'm sorry, I didn't read it carefully. And I thought "imply" was meant
> to fix exactly this kind of issue.
>
> I don't want to create a hard dependency on AMD_IOMMU_V2 if I can avoid
> it, because it is only really needed for a small number of AMD APUs, and
> even there it is now optional for more recent ones.
>
> Is there a better way to avoid build failures without creating a hard
> dependency?

What you need is the same trick we used for AGP on radeon/nouveau:

depends on AMD_IOMMU_V2 || !AMD_IOMMU_V2

This way when AMD_IOMMU_V2 is build as a module DRM_AMDGPU will be build 
as a module as well. When it is disabled completely we don't care.

Regards,
Christian.

>    The documentation in
> Documentation/kbuild/kconfig-language.rst suggests using if
> (IS_REACHABLE(CONFIG_AMD_IOMMU_V2)) to guard those problematic function
> calls. I think more generally, we could guard all of kfd_iommu.c with
>
>      #if IS_REACHABLE(CONFIG_AMD_IOMMU_V2)
>
> And use the same condition to define the stubs in kfd_iommu.h.
>
> Regards,
>    Felix
>
>
>> The causes a link failure for the vmlinux file, because the linker cannot
>> resolve addresses of loadable modules at compile time -- they have
>> not been loaded yet.
>>
>>        Arnd
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 20:12             ` Christian König
  0 siblings, 0 replies; 24+ messages in thread
From: Christian König @ 2021-03-08 20:12 UTC (permalink / raw)
  To: Felix Kuehling, Arnd Bergmann
  Cc: Philip Yang, David Airlie, linux-kernel, amd-gfx list, dri-devel,
	Alex Deucher, Colin Ian King, Christian König

Am 08.03.21 um 21:02 schrieb Felix Kuehling:
> Am 2021-03-08 um 2:33 p.m. schrieb Arnd Bergmann:
>> On Mon, Mar 8, 2021 at 8:11 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>>> Am 2021-03-08 um 2:05 p.m. schrieb Arnd Bergmann:
>>>> On Mon, Mar 8, 2021 at 5:24 PM Felix Kuehling <felix.kuehling@amd.com> wrote:
>>>>> The driver build should work without IOMMUv2. In amdkfd/Makefile, we
>>>>> have this condition:
>>>>>
>>>>> ifneq ($(CONFIG_AMD_IOMMU_V2),)
>>>>> AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
>>>>> endif
>>>>>
>>>>> In amdkfd/kfd_iommu.h we define inline stubs of the functions that are
>>>>> causing your link-failures if IOMMU_V2 is not enabled:
>>>>>
>>>>> #if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>>>> ... function declarations ...
>>>>> #else
>>>>> ... stubs ...
>>>>> #endif
>>>> Right, that is the problem I tried to explain in my patch description.
>>>>
>>>> Should we just drop the 'imply' then and add a proper dependency like this?
>>>>
>>>>        depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
>>>>        depends on AMD_IOMMU_V2=y || DRM_AMDGPU=m
>>>>
>>>> I can send a v2 after some testing if you prefer this version.
>>> No. My point is, there should not be a hard dependency. The build should
>>> work without CONFIG_AMD_IOMMU_V2. I don't understand why it's not
>>> working for you. It looks like you're building kfd_iommu.o, which should
>>> not be happening when AMD_IOMMU_V2 is not enabled. The condition in
>>> amdkfd/Makefile should make sure that kfd_iommu.o doesn't get built with
>>> your kernel config.
>> Again, as I explained in the changelog text, AMD_IOMMU_V2 configured as
>> a loadable module, while AMDGPU is configured as built-in.
> I'm sorry, I didn't read it carefully. And I thought "imply" was meant
> to fix exactly this kind of issue.
>
> I don't want to create a hard dependency on AMD_IOMMU_V2 if I can avoid
> it, because it is only really needed for a small number of AMD APUs, and
> even there it is now optional for more recent ones.
>
> Is there a better way to avoid build failures without creating a hard
> dependency?

What you need is the same trick we used for AGP on radeon/nouveau:

depends on AMD_IOMMU_V2 || !AMD_IOMMU_V2

This way when AMD_IOMMU_V2 is build as a module DRM_AMDGPU will be build 
as a module as well. When it is disabled completely we don't care.

Regards,
Christian.

>    The documentation in
> Documentation/kbuild/kconfig-language.rst suggests using if
> (IS_REACHABLE(CONFIG_AMD_IOMMU_V2)) to guard those problematic function
> calls. I think more generally, we could guard all of kfd_iommu.c with
>
>      #if IS_REACHABLE(CONFIG_AMD_IOMMU_V2)
>
> And use the same condition to define the stubs in kfd_iommu.h.
>
> Regards,
>    Felix
>
>
>> The causes a link failure for the vmlinux file, because the linker cannot
>> resolve addresses of loadable modules at compile time -- they have
>> not been loaded yet.
>>
>>        Arnd
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
  2021-03-08 20:12             ` Christian König
  (?)
@ 2021-03-08 20:35               ` Arnd Bergmann
  -1 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2021-03-08 20:35 UTC (permalink / raw)
  To: Christian König
  Cc: Felix Kuehling, Philip Yang, David Airlie, linux-kernel,
	dri-devel, amd-gfx list, Alex Deucher, Colin Ian King,
	Christian König

On Mon, Mar 8, 2021 at 9:12 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
> Am 08.03.21 um 21:02 schrieb Felix Kuehling:
> > Am 2021-03-08 um 2:33 p.m. schrieb Arnd Bergmann:

> > I don't want to create a hard dependency on AMD_IOMMU_V2 if I can avoid
> > it, because it is only really needed for a small number of AMD APUs, and
> > even there it is now optional for more recent ones.
> >
> > Is there a better way to avoid build failures without creating a hard
> > dependency?
>
> What you need is the same trick we used for AGP on radeon/nouveau:
>
> depends on AMD_IOMMU_V2 || !AMD_IOMMU_V2
>
> This way when AMD_IOMMU_V2 is build as a module DRM_AMDGPU will be build
> as a module as well. When it is disabled completely we don't care.

Note that this trick only works if you put it into the DRM_AMDGPU Kconfig option
itself, since that decides if the driver is built-in or a loadable module. If
HSA_AMD is disabled, that dependency is not really necessary.

The version I suggested  -- adding "depends on AMD_IOMMU_V2=y ||
DRM_AMDGPU=m" to the HSA_AMD option -- might be slightly nicer
since it lets you still build a kernel with DRM_AMDGPU=y and
AMD_IOMMU_V2=m, but without the HSA_AMD.

I can send a patch with either of those two options to replace my
original patch.

> >    The documentation in
> > Documentation/kbuild/kconfig-language.rst suggests using if
> > (IS_REACHABLE(CONFIG_AMD_IOMMU_V2)) to guard those problematic function
> > calls. I think more generally, we could guard all of kfd_iommu.c with
> >
> >      #if IS_REACHABLE(CONFIG_AMD_IOMMU_V2)
> >
> > And use the same condition to define the stubs in kfd_iommu.h.

This would fix the compile-time error, but it's also the one I'd least
recommend out of all the options, because that causes the driver to
silently not work as expected. This seems even worse than failing
the build.

       Arnd

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 20:35               ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2021-03-08 20:35 UTC (permalink / raw)
  To: Christian König
  Cc: Philip Yang, David Airlie, Felix Kuehling, linux-kernel,
	amd-gfx list, dri-devel, Alex Deucher, Colin Ian King,
	Christian König

On Mon, Mar 8, 2021 at 9:12 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
> Am 08.03.21 um 21:02 schrieb Felix Kuehling:
> > Am 2021-03-08 um 2:33 p.m. schrieb Arnd Bergmann:

> > I don't want to create a hard dependency on AMD_IOMMU_V2 if I can avoid
> > it, because it is only really needed for a small number of AMD APUs, and
> > even there it is now optional for more recent ones.
> >
> > Is there a better way to avoid build failures without creating a hard
> > dependency?
>
> What you need is the same trick we used for AGP on radeon/nouveau:
>
> depends on AMD_IOMMU_V2 || !AMD_IOMMU_V2
>
> This way when AMD_IOMMU_V2 is build as a module DRM_AMDGPU will be build
> as a module as well. When it is disabled completely we don't care.

Note that this trick only works if you put it into the DRM_AMDGPU Kconfig option
itself, since that decides if the driver is built-in or a loadable module. If
HSA_AMD is disabled, that dependency is not really necessary.

The version I suggested  -- adding "depends on AMD_IOMMU_V2=y ||
DRM_AMDGPU=m" to the HSA_AMD option -- might be slightly nicer
since it lets you still build a kernel with DRM_AMDGPU=y and
AMD_IOMMU_V2=m, but without the HSA_AMD.

I can send a patch with either of those two options to replace my
original patch.

> >    The documentation in
> > Documentation/kbuild/kconfig-language.rst suggests using if
> > (IS_REACHABLE(CONFIG_AMD_IOMMU_V2)) to guard those problematic function
> > calls. I think more generally, we could guard all of kfd_iommu.c with
> >
> >      #if IS_REACHABLE(CONFIG_AMD_IOMMU_V2)
> >
> > And use the same condition to define the stubs in kfd_iommu.h.

This would fix the compile-time error, but it's also the one I'd least
recommend out of all the options, because that causes the driver to
silently not work as expected. This seems even worse than failing
the build.

       Arnd
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2
@ 2021-03-08 20:35               ` Arnd Bergmann
  0 siblings, 0 replies; 24+ messages in thread
From: Arnd Bergmann @ 2021-03-08 20:35 UTC (permalink / raw)
  To: Christian König
  Cc: Philip Yang, David Airlie, Felix Kuehling, linux-kernel,
	amd-gfx list, dri-devel, Alex Deucher, Colin Ian King,
	Christian König

On Mon, Mar 8, 2021 at 9:12 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
> Am 08.03.21 um 21:02 schrieb Felix Kuehling:
> > Am 2021-03-08 um 2:33 p.m. schrieb Arnd Bergmann:

> > I don't want to create a hard dependency on AMD_IOMMU_V2 if I can avoid
> > it, because it is only really needed for a small number of AMD APUs, and
> > even there it is now optional for more recent ones.
> >
> > Is there a better way to avoid build failures without creating a hard
> > dependency?
>
> What you need is the same trick we used for AGP on radeon/nouveau:
>
> depends on AMD_IOMMU_V2 || !AMD_IOMMU_V2
>
> This way when AMD_IOMMU_V2 is build as a module DRM_AMDGPU will be build
> as a module as well. When it is disabled completely we don't care.

Note that this trick only works if you put it into the DRM_AMDGPU Kconfig option
itself, since that decides if the driver is built-in or a loadable module. If
HSA_AMD is disabled, that dependency is not really necessary.

The version I suggested  -- adding "depends on AMD_IOMMU_V2=y ||
DRM_AMDGPU=m" to the HSA_AMD option -- might be slightly nicer
since it lets you still build a kernel with DRM_AMDGPU=y and
AMD_IOMMU_V2=m, but without the HSA_AMD.

I can send a patch with either of those two options to replace my
original patch.

> >    The documentation in
> > Documentation/kbuild/kconfig-language.rst suggests using if
> > (IS_REACHABLE(CONFIG_AMD_IOMMU_V2)) to guard those problematic function
> > calls. I think more generally, we could guard all of kfd_iommu.c with
> >
> >      #if IS_REACHABLE(CONFIG_AMD_IOMMU_V2)
> >
> > And use the same condition to define the stubs in kfd_iommu.h.

This would fix the compile-time error, but it's also the one I'd least
recommend out of all the options, because that causes the driver to
silently not work as expected. This seems even worse than failing
the build.

       Arnd
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2021-03-08 20:36 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 15:33 [PATCH] drm/amdkfd: fix build error with missing AMD_IOMMU_V2 Arnd Bergmann
2021-03-08 15:33 ` Arnd Bergmann
2021-03-08 15:33 ` Arnd Bergmann
2021-03-08 16:24 ` Felix Kuehling
2021-03-08 16:24   ` Felix Kuehling
2021-03-08 16:24   ` Felix Kuehling
2021-03-08 19:05   ` Arnd Bergmann
2021-03-08 19:05     ` Arnd Bergmann
2021-03-08 19:05     ` Arnd Bergmann
2021-03-08 19:09     ` Felix Kuehling
2021-03-08 19:09       ` Felix Kuehling
2021-03-08 19:09       ` Felix Kuehling
2021-03-08 19:33       ` Arnd Bergmann
2021-03-08 19:33         ` Arnd Bergmann
2021-03-08 19:33         ` Arnd Bergmann
2021-03-08 20:02         ` Felix Kuehling
2021-03-08 20:02           ` Felix Kuehling
2021-03-08 20:02           ` Felix Kuehling
2021-03-08 20:12           ` Christian König
2021-03-08 20:12             ` Christian König
2021-03-08 20:12             ` Christian König
2021-03-08 20:35             ` Arnd Bergmann
2021-03-08 20:35               ` Arnd Bergmann
2021-03-08 20:35               ` Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.