Commit 4722e2eb authored by Nathan Chancellor's avatar Nathan Chancellor Committed by Andi Shyti

drm/i915/gt: Fix second parameter type of pre-gen8 pte_encode callbacks

When booting a kernel compiled with CONFIG_CFI_CLANG (kCFI), there is a
CFI failure in ggtt_probe_common() when trying to call hsw_pte_encode()
via an indirect call:

  [    5.030027] CFI failure at ggtt_probe_common+0xd1/0x130 [i915] (target: hsw_pte_encode+0x0/0x30 [i915]; expected type: 0xf5c1d0fc)

With kCFI, indirect calls are validated against their expected type
versus actual type and failures occur when the two types do not match.

clang's -Wincompatible-function-pointer-types-strict can catch this at
compile time but it is not enabled for the kernel yet:

  drivers/gpu/drm/i915/gt/intel_ggtt.c:1155:23: error: incompatible function pointer types assigning to 'u64 (*)(dma_addr_t, unsigned int, u32)' (aka 'unsigned long long (*)(unsigned int, unsigned int, unsigned int)') from 'u64 (dma_addr_t,
  enum i915_cache_level, u32)' (aka 'unsigned long long (unsigned int, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict]
                  ggtt->vm.pte_encode = iris_pte_encode;
                                      ^ ~~~~~~~~~~~~~~~
  drivers/gpu/drm/i915/gt/intel_ggtt.c:1157:23: error: incompatible function pointer types assigning to 'u64 (*)(dma_addr_t, unsigned int, u32)' (aka 'unsigned long long (*)(unsigned int, unsigned int, unsigned int)') from 'u64 (dma_addr_t,
  enum i915_cache_level, u32)' (aka 'unsigned long long (unsigned int, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict]
                  ggtt->vm.pte_encode = hsw_pte_encode;
                                      ^ ~~~~~~~~~~~~~~
  drivers/gpu/drm/i915/gt/intel_ggtt.c:1159:23: error: incompatible function pointer types assigning to 'u64 (*)(dma_addr_t, unsigned int, u32)' (aka 'unsigned long long (*)(unsigned int, unsigned int, unsigned int)') from 'u64 (dma_addr_t,
  enum i915_cache_level, u32)' (aka 'unsigned long long (unsigned int, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict]
                  ggtt->vm.pte_encode = byt_pte_encode;
                                      ^ ~~~~~~~~~~~~~~
  drivers/gpu/drm/i915/gt/intel_ggtt.c:1161:23: error: incompatible function pointer types assigning to 'u64 (*)(dma_addr_t, unsigned int, u32)' (aka 'unsigned long long (*)(unsigned int, unsigned int, unsigned int)') from 'u64 (dma_addr_t,
  enum i915_cache_level, u32)' (aka 'unsigned long long (unsigned int, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict]
                  ggtt->vm.pte_encode = ivb_pte_encode;
                                      ^ ~~~~~~~~~~~~~~
  drivers/gpu/drm/i915/gt/intel_ggtt.c:1163:23: error: incompatible function pointer types assigning to 'u64 (*)(dma_addr_t, unsigned int, u32)' (aka 'unsigned long long (*)(unsigned int, unsigned int, unsigned int)') from 'u64 (dma_addr_t,
  enum i915_cache_level, u32)' (aka 'unsigned long long (unsigned int, enum i915_cache_level, unsigned int)') [-Werror,-Wincompatible-function-pointer-types-strict]
                  ggtt->vm.pte_encode = snb_pte_encode;
                                      ^ ~~~~~~~~~~~~~~
  5 errors generated.

In this case, the pre-gen8 pte_encode functions have a second parameter
type of 'enum i915_cache_level' whereas the function pointer prototype
in 'struct i915_address_space' expects a second parameter type of
'unsigned int'.

Update the second parameter of the callbacks and the comment above them
noting that these statements are still valid, which matches other
functions and files, to clear up the kCFI failures at run time.

Fixes: 9275277d ("drm/i915: use pat_index instead of cache_level")
Signed-off-by: default avatarNathan Chancellor <nathan@kernel.org>
Reviewed-by: default avatarAndi Shyti <andi.shyti@linux.intel.com>
Reviewed-by: default avatarFei Yang <fei.yang@intel.com>
Signed-off-by: default avatarAndi Shyti <andi.shyti@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230530-i915-gt-cache_level-wincompatible-function-pointer-types-strict-v1-1-54501d598229@kernel.org
parent edad9ee9
...@@ -1015,16 +1015,16 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) ...@@ -1015,16 +1015,16 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
/* /*
* For pre-gen8 platforms pat_index is the same as enum i915_cache_level, * For pre-gen8 platforms pat_index is the same as enum i915_cache_level,
* so these PTE encode functions are left with using cache_level. * so the switch-case statements in these PTE encode functions are still valid.
* See translation table LEGACY_CACHELEVEL. * See translation table LEGACY_CACHELEVEL.
*/ */
static u64 snb_pte_encode(dma_addr_t addr, static u64 snb_pte_encode(dma_addr_t addr,
enum i915_cache_level level, unsigned int pat_index,
u32 flags) u32 flags)
{ {
gen6_pte_t pte = GEN6_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID; gen6_pte_t pte = GEN6_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID;
switch (level) { switch (pat_index) {
case I915_CACHE_L3_LLC: case I915_CACHE_L3_LLC:
case I915_CACHE_LLC: case I915_CACHE_LLC:
pte |= GEN6_PTE_CACHE_LLC; pte |= GEN6_PTE_CACHE_LLC;
...@@ -1033,19 +1033,19 @@ static u64 snb_pte_encode(dma_addr_t addr, ...@@ -1033,19 +1033,19 @@ static u64 snb_pte_encode(dma_addr_t addr,
pte |= GEN6_PTE_UNCACHED; pte |= GEN6_PTE_UNCACHED;
break; break;
default: default:
MISSING_CASE(level); MISSING_CASE(pat_index);
} }
return pte; return pte;
} }
static u64 ivb_pte_encode(dma_addr_t addr, static u64 ivb_pte_encode(dma_addr_t addr,
enum i915_cache_level level, unsigned int pat_index,
u32 flags) u32 flags)
{ {
gen6_pte_t pte = GEN6_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID; gen6_pte_t pte = GEN6_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID;
switch (level) { switch (pat_index) {
case I915_CACHE_L3_LLC: case I915_CACHE_L3_LLC:
pte |= GEN7_PTE_CACHE_L3_LLC; pte |= GEN7_PTE_CACHE_L3_LLC;
break; break;
...@@ -1056,14 +1056,14 @@ static u64 ivb_pte_encode(dma_addr_t addr, ...@@ -1056,14 +1056,14 @@ static u64 ivb_pte_encode(dma_addr_t addr,
pte |= GEN6_PTE_UNCACHED; pte |= GEN6_PTE_UNCACHED;
break; break;
default: default:
MISSING_CASE(level); MISSING_CASE(pat_index);
} }
return pte; return pte;
} }
static u64 byt_pte_encode(dma_addr_t addr, static u64 byt_pte_encode(dma_addr_t addr,
enum i915_cache_level level, unsigned int pat_index,
u32 flags) u32 flags)
{ {
gen6_pte_t pte = GEN6_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID; gen6_pte_t pte = GEN6_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID;
...@@ -1071,31 +1071,31 @@ static u64 byt_pte_encode(dma_addr_t addr, ...@@ -1071,31 +1071,31 @@ static u64 byt_pte_encode(dma_addr_t addr,
if (!(flags & PTE_READ_ONLY)) if (!(flags & PTE_READ_ONLY))
pte |= BYT_PTE_WRITEABLE; pte |= BYT_PTE_WRITEABLE;
if (level != I915_CACHE_NONE) if (pat_index != I915_CACHE_NONE)
pte |= BYT_PTE_SNOOPED_BY_CPU_CACHES; pte |= BYT_PTE_SNOOPED_BY_CPU_CACHES;
return pte; return pte;
} }
static u64 hsw_pte_encode(dma_addr_t addr, static u64 hsw_pte_encode(dma_addr_t addr,
enum i915_cache_level level, unsigned int pat_index,
u32 flags) u32 flags)
{ {
gen6_pte_t pte = HSW_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID; gen6_pte_t pte = HSW_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID;
if (level != I915_CACHE_NONE) if (pat_index != I915_CACHE_NONE)
pte |= HSW_WB_LLC_AGE3; pte |= HSW_WB_LLC_AGE3;
return pte; return pte;
} }
static u64 iris_pte_encode(dma_addr_t addr, static u64 iris_pte_encode(dma_addr_t addr,
enum i915_cache_level level, unsigned int pat_index,
u32 flags) u32 flags)
{ {
gen6_pte_t pte = HSW_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID; gen6_pte_t pte = HSW_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID;
switch (level) { switch (pat_index) {
case I915_CACHE_NONE: case I915_CACHE_NONE:
break; break;
case I915_CACHE_WT: case I915_CACHE_WT:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment