* [drm-intel:for-linux-next 479/497] drivers/gpu/drm/i915/i915_gem_gtt.c:1086:26: sparse: Using plain integer as NULL pointer @ 2015-08-06 20:17 kbuild test robot 2015-08-06 22:00 ` Daniel Vetter 0 siblings, 1 reply; 5+ messages in thread From: kbuild test robot @ 2015-08-06 20:17 UTC (permalink / raw) To: Michel Thierry Cc: Akash Goel, Daniel Vetter, Ben Widawsky, intel-gfx, kbuild-all tree: git://anongit.freedesktop.org/drm-intel for-linux-next head: 176b15ab89a89f5bd5f73f96afcc5c4a6105fa5e commit: fb6c09f3ae620c5e0db5b4c2058a9c25b52197f9 [479/497] drm/i915/gen8: Make pdp allocation more dynamic reproduce: # apt-get install sparse git checkout fb6c09f3ae620c5e0db5b4c2058a9c25b52197f9 make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/gpu/drm/i915/i915_gem_gtt.c:1086:26: sparse: Using plain integer as NULL pointer vim +1086 drivers/gpu/drm/i915/i915_gem_gtt.c 1070 if (IS_ENABLED(CONFIG_X86_32)) 1071 /* While we have a proliferation of size_t variables 1072 * we cannot represent the full ppgtt size on 32bit, 1073 * so limit it to the same size as the GGTT (currently 1074 * 2GiB). 1075 */ 1076 ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total; 1077 ppgtt->base.cleanup = gen8_ppgtt_cleanup; 1078 ppgtt->base.allocate_va_range = gen8_alloc_va_range; 1079 ppgtt->base.insert_entries = gen8_ppgtt_insert_entries; 1080 ppgtt->base.clear_range = gen8_ppgtt_clear_range; 1081 ppgtt->base.unbind_vma = ppgtt_unbind_vma; 1082 ppgtt->base.bind_vma = ppgtt_bind_vma; 1083 1084 ppgtt->switch_mm = gen8_mm_switch; 1085 > 1086 ret = __pdp_init(false, &ppgtt->pdp); 1087 1088 if (ret) 1089 goto free_scratch; 1090 1091 return 0; 1092 1093 free_scratch: 1094 gen8_free_scratch(&ppgtt->base); --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [drm-intel:for-linux-next 479/497] drivers/gpu/drm/i915/i915_gem_gtt.c:1086:26: sparse: Using plain integer as NULL pointer 2015-08-06 20:17 [drm-intel:for-linux-next 479/497] drivers/gpu/drm/i915/i915_gem_gtt.c:1086:26: sparse: Using plain integer as NULL pointer kbuild test robot @ 2015-08-06 22:00 ` Daniel Vetter 2015-08-07 9:21 ` Michel Thierry 0 siblings, 1 reply; 5+ messages in thread From: Daniel Vetter @ 2015-08-06 22:00 UTC (permalink / raw) To: kbuild test robot; +Cc: Akash Goel, Ben Widawsky, intel-gfx, kbuild-all On Thu, Aug 6, 2015 at 10:17 PM, kbuild test robot <fengguang.wu@intel.com> wrote: > 1070 if (IS_ENABLED(CONFIG_X86_32)) > 1071 /* While we have a proliferation of size_t variables > 1072 * we cannot represent the full ppgtt size on 32bit, > 1073 * so limit it to the same size as the GGTT (currently > 1074 * 2GiB). > 1075 */ > 1076 ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total; > 1077 ppgtt->base.cleanup = gen8_ppgtt_cleanup; > 1078 ppgtt->base.allocate_va_range = gen8_alloc_va_range; > 1079 ppgtt->base.insert_entries = gen8_ppgtt_insert_entries; > 1080 ppgtt->base.clear_range = gen8_ppgtt_clear_range; > 1081 ppgtt->base.unbind_vma = ppgtt_unbind_vma; > 1082 ppgtt->base.bind_vma = ppgtt_bind_vma; > 1083 > 1084 ppgtt->switch_mm = gen8_mm_switch; > 1085 >> 1086 ret = __pdp_init(false, &ppgtt->pdp); So the first argument of pdp_init ist struct drm_device *dev and yes the first thing it does is deref it. How exactly was this tested again? Oh and the hunk right below with the CONFIG_X86_32 needs to got too - if we're 48b safe then we should be 32b safe too ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [drm-intel:for-linux-next 479/497] drivers/gpu/drm/i915/i915_gem_gtt.c:1086:26: sparse: Using plain integer as NULL pointer 2015-08-06 22:00 ` Daniel Vetter @ 2015-08-07 9:21 ` Michel Thierry 2015-08-07 10:21 ` Chris Wilson 0 siblings, 1 reply; 5+ messages in thread From: Michel Thierry @ 2015-08-07 9:21 UTC (permalink / raw) To: Daniel Vetter, kbuild test robot Cc: Akash Goel, Ben Widawsky, intel-gfx, kbuild-all On 8/6/2015 11:00 PM, Daniel Vetter wrote: > On Thu, Aug 6, 2015 at 10:17 PM, kbuild test robot > <fengguang.wu@intel.com> wrote: >> 1070 if (IS_ENABLED(CONFIG_X86_32)) >> 1071 /* While we have a proliferation of size_t variables >> 1072 * we cannot represent the full ppgtt size on 32bit, >> 1073 * so limit it to the same size as the GGTT (currently >> 1074 * 2GiB). >> 1075 */ >> 1076 ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total; >> 1077 ppgtt->base.cleanup = gen8_ppgtt_cleanup; >> 1078 ppgtt->base.allocate_va_range = gen8_alloc_va_range; >> 1079 ppgtt->base.insert_entries = gen8_ppgtt_insert_entries; >> 1080 ppgtt->base.clear_range = gen8_ppgtt_clear_range; >> 1081 ppgtt->base.unbind_vma = ppgtt_unbind_vma; >> 1082 ppgtt->base.bind_vma = ppgtt_bind_vma; >> 1083 >> 1084 ppgtt->switch_mm = gen8_mm_switch; >> 1085 >>> 1086 ret = __pdp_init(false, &ppgtt->pdp); > > So the first argument of pdp_init ist struct drm_device *dev and yes > the first thing it does is deref it. > *dev is used only for I915_PDPES_PER_PDP/USES_FULL_48BIT_PPGTT, which in this path is always false. I didn't expect kbuild to complain. I'll change it with the other modifications I'm about to send. > How exactly was this tested again? > > Oh and the hunk right below with the CONFIG_X86_32 needs to got too - > if we're 48b safe then we should be 32b safe too ;-) Yes, after the offset change it'll be completely safe. I left it in this patch as that was only moving code around. I'll include a proper "revert-me" patch, saying why it is safe now. -Michel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [drm-intel:for-linux-next 479/497] drivers/gpu/drm/i915/i915_gem_gtt.c:1086:26: sparse: Using plain integer as NULL pointer 2015-08-07 9:21 ` Michel Thierry @ 2015-08-07 10:21 ` Chris Wilson 2015-08-07 11:40 ` Daniel Vetter 0 siblings, 1 reply; 5+ messages in thread From: Chris Wilson @ 2015-08-07 10:21 UTC (permalink / raw) To: Michel Thierry Cc: Ben Widawsky, Daniel Vetter, intel-gfx, Akash Goel, kbuild-all, kbuild test robot On Fri, Aug 07, 2015 at 10:21:34AM +0100, Michel Thierry wrote: > On 8/6/2015 11:00 PM, Daniel Vetter wrote: > >On Thu, Aug 6, 2015 at 10:17 PM, kbuild test robot > ><fengguang.wu@intel.com> wrote: > >> 1070 if (IS_ENABLED(CONFIG_X86_32)) > >> 1071 /* While we have a proliferation of size_t variables > >> 1072 * we cannot represent the full ppgtt size on 32bit, > >> 1073 * so limit it to the same size as the GGTT (currently > >> 1074 * 2GiB). > >> 1075 */ > >> 1076 ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total; > >> 1077 ppgtt->base.cleanup = gen8_ppgtt_cleanup; > >> 1078 ppgtt->base.allocate_va_range = gen8_alloc_va_range; > >> 1079 ppgtt->base.insert_entries = gen8_ppgtt_insert_entries; > >> 1080 ppgtt->base.clear_range = gen8_ppgtt_clear_range; > >> 1081 ppgtt->base.unbind_vma = ppgtt_unbind_vma; > >> 1082 ppgtt->base.bind_vma = ppgtt_bind_vma; > >> 1083 > >> 1084 ppgtt->switch_mm = gen8_mm_switch; > >> 1085 > >>>1086 ret = __pdp_init(false, &ppgtt->pdp); > > > >So the first argument of pdp_init ist struct drm_device *dev and yes > >the first thing it does is deref it. > > > > *dev is used only for I915_PDPES_PER_PDP/USES_FULL_48BIT_PPGTT, > which in this path is always false. I didn't expect kbuild to > complain. I'll change it with the other modifications I'm about to > send. Wrong. dev is never deferenced even though it is passed around. diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 41c5e7c9c8ab..37283d5a8a4a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2549,12 +2549,12 @@ struct drm_i915_cmd_table { #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) #define HAS_LOGICAL_RING_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 8) -#define USES_PPGTT(dev) (i915.enable_ppgtt) -#define USES_FULL_PPGTT(dev) (i915.enable_ppgtt >= 2) +#define USES_PPGTT() (i915.enable_ppgtt) +#define USES_FULL_PPGTT() (i915.enable_ppgtt >= 2) #ifdef CONFIG_X86_64 -# define USES_FULL_48BIT_PPGTT(dev) (i915.enable_ppgtt == 3) +# define USES_FULL_48BIT_PPGTT() (i915.enable_ppgtt == 3) #else -# define USES_FULL_48BIT_PPGTT(dev) false +# define USES_FULL_48BIT_PPGTT() false #endif -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [drm-intel:for-linux-next 479/497] drivers/gpu/drm/i915/i915_gem_gtt.c:1086:26: sparse: Using plain integer as NULL pointer 2015-08-07 10:21 ` Chris Wilson @ 2015-08-07 11:40 ` Daniel Vetter 0 siblings, 0 replies; 5+ messages in thread From: Daniel Vetter @ 2015-08-07 11:40 UTC (permalink / raw) To: Chris Wilson, Michel Thierry, Daniel Vetter, kbuild test robot, Akash Goel, Ben Widawsky, intel-gfx, kbuild-all On Fri, Aug 7, 2015 at 12:21 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Fri, Aug 07, 2015 at 10:21:34AM +0100, Michel Thierry wrote: >> On 8/6/2015 11:00 PM, Daniel Vetter wrote: >> >On Thu, Aug 6, 2015 at 10:17 PM, kbuild test robot >> ><fengguang.wu@intel.com> wrote: >> >> 1070 if (IS_ENABLED(CONFIG_X86_32)) >> >> 1071 /* While we have a proliferation of size_t variables >> >> 1072 * we cannot represent the full ppgtt size on 32bit, >> >> 1073 * so limit it to the same size as the GGTT (currently >> >> 1074 * 2GiB). >> >> 1075 */ >> >> 1076 ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total; >> >> 1077 ppgtt->base.cleanup = gen8_ppgtt_cleanup; >> >> 1078 ppgtt->base.allocate_va_range = gen8_alloc_va_range; >> >> 1079 ppgtt->base.insert_entries = gen8_ppgtt_insert_entries; >> >> 1080 ppgtt->base.clear_range = gen8_ppgtt_clear_range; >> >> 1081 ppgtt->base.unbind_vma = ppgtt_unbind_vma; >> >> 1082 ppgtt->base.bind_vma = ppgtt_bind_vma; >> >> 1083 >> >> 1084 ppgtt->switch_mm = gen8_mm_switch; >> >> 1085 >> >>>1086 ret = __pdp_init(false, &ppgtt->pdp); >> > >> >So the first argument of pdp_init ist struct drm_device *dev and yes >> >the first thing it does is deref it. >> > >> >> *dev is used only for I915_PDPES_PER_PDP/USES_FULL_48BIT_PPGTT, >> which in this path is always false. I didn't expect kbuild to >> complain. I'll change it with the other modifications I'm about to >> send. > > Wrong. dev is never deferenced even though it is passed around. Yeah I spotted that too right after sending out my mail. > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 41c5e7c9c8ab..37283d5a8a4a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2549,12 +2549,12 @@ struct drm_i915_cmd_table { > > #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) > #define HAS_LOGICAL_RING_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 8) > -#define USES_PPGTT(dev) (i915.enable_ppgtt) > -#define USES_FULL_PPGTT(dev) (i915.enable_ppgtt >= 2) > +#define USES_PPGTT() (i915.enable_ppgtt) > +#define USES_FULL_PPGTT() (i915.enable_ppgtt >= 2) > #ifdef CONFIG_X86_64 > -# define USES_FULL_48BIT_PPGTT(dev) (i915.enable_ppgtt == 3) > +# define USES_FULL_48BIT_PPGTT() (i915.enable_ppgtt == 3) > #else > -# define USES_FULL_48BIT_PPGTT(dev) false > +# define USES_FULL_48BIT_PPGTT() false > #endif I'd vote to abolish all these macros and just add an enum for ppgtt modes: enum i915_ppgtt_mode { PPGTT_DISABLED = 0, ALIASING_PPGTT = 1, FULL_PPGTT = 2, FULL_48B_PPGTT = 3 }; Then just open-code the checks. Of course separate patch from anything else really. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-08-07 11:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-06 20:17 [drm-intel:for-linux-next 479/497] drivers/gpu/drm/i915/i915_gem_gtt.c:1086:26: sparse: Using plain integer as NULL pointer kbuild test robot 2015-08-06 22:00 ` Daniel Vetter 2015-08-07 9:21 ` Michel Thierry 2015-08-07 10:21 ` Chris Wilson 2015-08-07 11:40 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox