Re: [PATCH] drm/amdgpu: Fix return types of certain NBIOv7.9 callbacks

From: Alex Deucher <alexdeucher@gmail.com>
To: Nathan Chancellor <nathan@kernel.org>
Cc: llvm@lists.linux.dev, trix@redhat.com, Xinhui.Pan@amd.com,
	ndesaulniers@google.com, patches@lists.linux.dev,
	dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
	alexander.deucher@amd.com, christian.koenig@amd.com
Subject: Re: [PATCH] drm/amdgpu: Fix return types of certain NBIOv7.9 callbacks
Date: Wed, 24 May 2023 14:50:51 -0400	[thread overview]
Message-ID: <CADnq5_OXzN3_9r8_X9ESx_axvGw-sNdGfPbU0NZuwQ+gFzZEag@mail.gmail.com> (raw)
In-Reply-To: <20230524-nbio_v7_9-wincompatible-function-pointer-types-strict-v1-1-da17e01907cf@kernel.org>

Applied.  Thanks!

On Wed, May 24, 2023 at 12:44 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> When building with clang's -Wincompatible-function-pointer-types-strict,
> which ensures that function pointer signatures match exactly to avoid
> tripping clang's Control Flow Integrity (kCFI) checks at run time and
> will eventually be turned on for the kernel, the following instances
> appear in the NBIOv7.9 code:
>
>   drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c:465:32: error: incompatible function pointer types initializing 'int (*)(struct amdgpu_device *)' with an expression of type 'enum amdgpu_gfx_partition (struct amdgpu_device *)' [-Werror,-Wincompatible-function-pointer-types-strict]
>           .get_compute_partition_mode = nbio_v7_9_get_compute_partition_mode,
>                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c:467:31: error: incompatible function pointer types initializing 'u32 (*)(struct amdgpu_device *, u32 *)' (aka 'unsigned int (*)(struct amdgpu_device *, unsigned int *)') with an expression of type 'enum amdgpu_memory_partition (struct amdgpu_device *, u32 *)' (aka 'enum amdgpu_memory_partition (struct amdgpu_device *, unsigned int *)') [-Werror,-Wincompatible-function-pointer-types-strict]
>           .get_memory_partition_mode = nbio_v7_9_get_memory_partition_mode,
>                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   2 errors generated.
>
> Change the return types of these callbacks to match the prototypes to
> clear up the warning and avoid tripping kCFI at run time. Both functions
> return a value from ffs(), which is an integer that can fit into either
> int or unsigned int.
>
> Fixes: 11f64eb1472f ("drm/amdgpu: add sysfs node for compute partition mode")
> Fixes: 41a717ea8afc ("drm/amdgpu: detect current GPU memory partition mode")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c b/drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c
> index e082f6343d20..d19325476752 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_9.c
> @@ -382,7 +382,7 @@ static void nbio_v7_9_enable_doorbell_interrupt(struct amdgpu_device *adev,
>                               DOORBELL_INTERRUPT_DISABLE, enable ? 0 : 1);
>  }
>
> -static enum amdgpu_gfx_partition nbio_v7_9_get_compute_partition_mode(struct amdgpu_device *adev)
> +static int nbio_v7_9_get_compute_partition_mode(struct amdgpu_device *adev)
>  {
>         u32 tmp, px;
>
> @@ -408,8 +408,8 @@ static void nbio_v7_9_set_compute_partition_mode(struct amdgpu_device *adev,
>         WREG32_SOC15(NBIO, 0, regBIF_BX_PF0_PARTITION_COMPUTE_STATUS, tmp);
>  }
>
> -static enum amdgpu_memory_partition
> -nbio_v7_9_get_memory_partition_mode(struct amdgpu_device *adev, u32 *supp_modes)
> +static u32 nbio_v7_9_get_memory_partition_mode(struct amdgpu_device *adev,
> +                                              u32 *supp_modes)
>  {
>         u32 tmp;
>
>
> ---
> base-commit: fd8f7bb391fa9c1979232cb5ab5bedb08abc855d
> change-id: 20230524-nbio_v7_9-wincompatible-function-pointer-types-strict-c894636ce146
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
>