diff mbox series

[2/2] drm/i915/gt: Fix parameter in gmch_ggtt_insert_{entries, page}()

Message ID 20230530-i915-gt-cache_level-wincompatible-function-pointer-types-strict-v1-2-54501d598229@kernel.org (mailing list archive)
State New, archived
Headers show
Series drm/i915/gt: Fix recent kCFI violations | expand

Commit Message

Nathan Chancellor May 30, 2023, 6:24 p.m. UTC
When building with clang's -Wincompatible-function-pointer-types-strict,
the following warnings occur:

  drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c:102:23: error: incompatible function pointer types assigning to 'void (*)(struct i915_address_space *, dma_addr_t, u64, unsigned int, u32)' (aka 'void (*)(struct i915_address_space *, unsigned int, unsigned long long, unsigned int, unsigned int)') from 'void (struct i915_address_space *, dma_addr_t, u64, enum i915_cache_level, u32)' (aka 'void (struct i915_address_space *, unsigned int, unsigned long long, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict]
          ggtt->vm.insert_page = gmch_ggtt_insert_page;
                               ^ ~~~~~~~~~~~~~~~~~~~~~
  drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c:103:26: error: incompatible function pointer types assigning to 'void (*)(struct i915_address_space *, struct i915_vma_resource *, unsigned int, u32)' (aka 'void (*)(struct i915_address_space *, struct i915_vma_resource *, unsigned int, unsigned int)') from 'void (struct i915_address_space *, struct i915_vma_resource *, enum i915_cache_level, u32)' (aka 'void (struct i915_address_space *, struct i915_vma_resource *, enum i915_cache_level, unsigned int)') [-Werror, -Wincompatible-function-pointer-types-strict]
          ggtt->vm.insert_entries = gmch_ggtt_insert_entries;
                                  ^ ~~~~~~~~~~~~~~~~~~~~~~~~
  2 errors generated.

The warning is pointing out that while 'enum i915_cache_level' and
'unsigned int' are ABI compatible, these indirect calls will fail
clang's kernel Control Flow Integrity (kCFI) checks, as the callback's
signature does not exactly match the prototype's signature.

To fix this, replace the cache_level parameter with pat_index, as was
done in other places within i915 where there is no difference between
cache_level and pat_index on certain generations.

Fixes: 9275277d5324 ("drm/i915: use pat_index instead of cache_level")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Andi Shyti May 30, 2023, 6:55 p.m. UTC | #1
Hi Nathan,

On Tue, May 30, 2023 at 11:24:39AM -0700, Nathan Chancellor wrote:
> When building with clang's -Wincompatible-function-pointer-types-strict,
> the following warnings occur:
> 
>   drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c:102:23: error: incompatible function pointer types assigning to 'void (*)(struct i915_address_space *, dma_addr_t, u64, unsigned int, u32)' (aka 'void (*)(struct i915_address_space *, unsigned int, unsigned long long, unsigned int, unsigned int)') from 'void (struct i915_address_space *, dma_addr_t, u64, enum i915_cache_level, u32)' (aka 'void (struct i915_address_space *, unsigned int, unsigned long long, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict]
>           ggtt->vm.insert_page = gmch_ggtt_insert_page;
>                                ^ ~~~~~~~~~~~~~~~~~~~~~
>   drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c:103:26: error: incompatible function pointer types assigning to 'void (*)(struct i915_address_space *, struct i915_vma_resource *, unsigned int, u32)' (aka 'void (*)(struct i915_address_space *, struct i915_vma_resource *, unsigned int, unsigned int)') from 'void (struct i915_address_space *, struct i915_vma_resource *, enum i915_cache_level, u32)' (aka 'void (struct i915_address_space *, struct i915_vma_resource *, enum i915_cache_level, unsigned int)') [-Werror, -Wincompatible-function-pointer-types-strict]
>           ggtt->vm.insert_entries = gmch_ggtt_insert_entries;
>                                   ^ ~~~~~~~~~~~~~~~~~~~~~~~~
>   2 errors generated.
> 
> The warning is pointing out that while 'enum i915_cache_level' and
> 'unsigned int' are ABI compatible, these indirect calls will fail
> clang's kernel Control Flow Integrity (kCFI) checks, as the callback's
> signature does not exactly match the prototype's signature.
> 
> To fix this, replace the cache_level parameter with pat_index, as was
> done in other places within i915 where there is no difference between
> cache_level and pat_index on certain generations.
> 
> Fixes: 9275277d5324 ("drm/i915: use pat_index instead of cache_level")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

same clang issue as before, I'm OK with this patch, from my side:

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> 

Thanks,
Andi
Yang, Fei May 30, 2023, 7:08 p.m. UTC | #2
> Subject: [PATCH 2/2] drm/i915/gt: Fix parameter in gmch_ggtt_insert_{entries,page}()
>
> When building with clang's -Wincompatible-function-pointer-types-strict,
> the following warnings occur:
>
>   drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c:102:23: error: incompatible function pointer types assigning to 'void (*)(struct i915_address_space *, dma_addr_t, u64, unsigned int, u32)' (aka 'void (*)(struct i915_address_space *, unsigned int, unsigned long long, unsigned int, unsigned int)') from 'void (struct i915_address_space *, dma_addr_t, u64, enum i915_cache_level, u32)' (aka 'void (struct i915_address_space *, unsigned int, unsigned long long, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict]
>           ggtt->vm.insert_page = gmch_ggtt_insert_page;
>                                ^ ~~~~~~~~~~~~~~~~~~~~~
>   drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c:103:26: error: incompatible function pointer types assigning to 'void (*)(struct i915_address_space *, struct i915_vma_resource *, unsigned int, u32)' (aka 'void (*)(struct i915_address_space *, struct i915_vma_resource *, unsigned int, unsigned int)') from 'void (struct i915_address_space *, struct i915_vma_resource *, enum i915_cache_level, u32)' (aka 'void (struct i915_address_space *, struct i915_vma_resource *, enum i915_cache_level, unsigned int)') [-Werror, -Wincompatible-function-pointer-types-strict]
>           ggtt->vm.insert_entries = gmch_ggtt_insert_entries;
>                                   ^ ~~~~~~~~~~~~~~~~~~~~~~~~
>   2 errors generated.
>
> The warning is pointing out that while 'enum i915_cache_level' and 'unsigned int' are ABI compatible, these indirect calls will fail clang's kernel Control Flow Integrity (kCFI) checks, as the callback's signature does not exactly match the prototype's signature.
>
> To fix this, replace the cache_level parameter with pat_index, as was done in other places within i915 where there is no difference between cache_level and pat_index on certain generations.
>
> Fixes: 9275277d5324 ("drm/i915: use pat_index instead of cache_level")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Reviewed-by: Fei Yang <fei.yang@intel.com>

> ---
>  drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c b/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c
> index d6a74ae2527b..866c416afb73 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c
> @@ -18,10 +18,10 @@
>  static void gmch_ggtt_insert_page(struct i915_address_space *vm,
>                                 dma_addr_t addr,
>                                 u64 offset,
> -                               enum i915_cache_level cache_level,
> +                               unsigned int pat_index,
>                                 u32 unused)
>  {
> -     unsigned int flags = (cache_level == I915_CACHE_NONE) ?
> +     unsigned int flags = (pat_index == I915_CACHE_NONE) ?
>               AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY;
>
>       intel_gmch_gtt_insert_page(addr, offset >> PAGE_SHIFT, flags); @@ -29,10 +29,10 @@ static void gmch_ggtt_insert_page(struct i915_address_space *vm,
>
>  static void gmch_ggtt_insert_entries(struct i915_address_space *vm,
>                                    struct i915_vma_resource *vma_res,
> -                                  enum i915_cache_level cache_level,
> +                                  unsigned int pat_index,
>                                    u32 unused)
>  {
> -     unsigned int flags = (cache_level == I915_CACHE_NONE) ?
> +     unsigned int flags = (pat_index == I915_CACHE_NONE) ?
>               AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY;
>
>       intel_gmch_gtt_insert_sg_entries(vma_res->bi.pages, vma_res->start >> PAGE_SHIFT,
>
> --
> 2.40.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c b/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c
index d6a74ae2527b..866c416afb73 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt_gmch.c
@@ -18,10 +18,10 @@ 
 static void gmch_ggtt_insert_page(struct i915_address_space *vm,
 				  dma_addr_t addr,
 				  u64 offset,
-				  enum i915_cache_level cache_level,
+				  unsigned int pat_index,
 				  u32 unused)
 {
-	unsigned int flags = (cache_level == I915_CACHE_NONE) ?
+	unsigned int flags = (pat_index == I915_CACHE_NONE) ?
 		AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY;
 
 	intel_gmch_gtt_insert_page(addr, offset >> PAGE_SHIFT, flags);
@@ -29,10 +29,10 @@  static void gmch_ggtt_insert_page(struct i915_address_space *vm,
 
 static void gmch_ggtt_insert_entries(struct i915_address_space *vm,
 				     struct i915_vma_resource *vma_res,
-				     enum i915_cache_level cache_level,
+				     unsigned int pat_index,
 				     u32 unused)
 {
-	unsigned int flags = (cache_level == I915_CACHE_NONE) ?
+	unsigned int flags = (pat_index == I915_CACHE_NONE) ?
 		AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY;
 
 	intel_gmch_gtt_insert_sg_entries(vma_res->bi.pages, vma_res->start >> PAGE_SHIFT,