Hi Nathan, On Tue, May 30, 2023 at 11:24:38AM -0700, Nathan Chancellor wrote: > 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 <nathan@xxxxxxxxxx> That's correct! Reviewed-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx> Thanks, Andi