> Subject: [PATCH 1/2] 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: 9275277d5324 ("drm/i915: use pat_index instead of cache_level") > Signed-off-by: Nathan Chancellor <nat...@kernel.org>
Reviewed-by: Fei Yang <fei.y...@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_ggtt.c | 26 +++++++++++++------------- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c > b/drivers/gpu/drm/i915/gt/intel_ggtt.c > index 2a7942fac798..122197737ef2 100644 > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c > @@ -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, > - * 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. > */ > static u64 snb_pte_encode(dma_addr_t addr, > - enum i915_cache_level level, > + unsigned int pat_index, > u32 flags) > { > 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_LLC: > pte |= GEN6_PTE_CACHE_LLC; > @@ -1033,19 +1033,19 @@ static u64 snb_pte_encode(dma_addr_t addr, > pte |= GEN6_PTE_UNCACHED; > break; > default: > - MISSING_CASE(level); > + MISSING_CASE(pat_index); > } > > return pte; > } > > static u64 ivb_pte_encode(dma_addr_t addr, > - enum i915_cache_level level, > + unsigned int pat_index, > u32 flags) > { > gen6_pte_t pte = GEN6_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID; > > - switch (level) { > + switch (pat_index) { > case I915_CACHE_L3_LLC: > pte |= GEN7_PTE_CACHE_L3_LLC; > break; > @@ -1056,14 +1056,14 @@ static u64 ivb_pte_encode(dma_addr_t addr, > pte |= GEN6_PTE_UNCACHED; > break; > default: > - MISSING_CASE(level); > + MISSING_CASE(pat_index); > } > > return pte; > } > > static u64 byt_pte_encode(dma_addr_t addr, > - enum i915_cache_level level, > + unsigned int pat_index, > u32 flags) > { > 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, > if (!(flags & PTE_READ_ONLY)) > pte |= BYT_PTE_WRITEABLE; > > - if (level != I915_CACHE_NONE) > + if (pat_index != I915_CACHE_NONE) > pte |= BYT_PTE_SNOOPED_BY_CPU_CACHES; > > return pte; > } > > static u64 hsw_pte_encode(dma_addr_t addr, > - enum i915_cache_level level, > + unsigned int pat_index, > u32 flags) > { > 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; > > return pte; > } > > static u64 iris_pte_encode(dma_addr_t addr, > - enum i915_cache_level level, > + unsigned int pat_index, > u32 flags) > { > gen6_pte_t pte = HSW_PTE_ADDR_ENCODE(addr) | GEN6_PTE_VALID; > > - switch (level) { > + switch (pat_index) { > case I915_CACHE_NONE: > break; > case I915_CACHE_WT: > > -- > 2.40.1