| Message ID | 1467299569-10920-1-git-send-email-david.s.gordon@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 30, 2016 at 04:12:48PM +0100, Dave Gordon wrote: > Two different sets of flag bits are stored in the 'flags' member of a > 'struct drm_i915_gem_exec_object2', and they're defined in two different > source files, increasing the risk of an accidental clash. > > Some flags in this field are supplied by the user; these are defined in > i915_drm.h, and they start from the LSB and work up. > > Other flags are defined in i915_gem_execbuffer, for internal use within > that file only; they start from the MSB and work down. > > So here we add a compile-time check that the two sets of flags do not > overlap, which would cause all sorts of confusion. > > Signed-off-by: Dave Gordon <david.s.gordon@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 12 ++++++++---- > include/uapi/drm/i915_drm.h | 11 ++++++----- > 2 files changed, 14 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 7941f1f..608fdc4 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -34,10 +34,11 @@ > #include <linux/dma_remapping.h> > #include <linux/uaccess.h> > > -#define __EXEC_OBJECT_HAS_PIN (1<<31) > -#define __EXEC_OBJECT_HAS_FENCE (1<<30) > -#define __EXEC_OBJECT_NEEDS_MAP (1<<29) > -#define __EXEC_OBJECT_NEEDS_BIAS (1<<28) > +#define __EXEC_OBJECT_HAS_PIN (1<<31) > +#define __EXEC_OBJECT_HAS_FENCE (1<<30) > +#define __EXEC_OBJECT_NEEDS_MAP (1<<29) > +#define __EXEC_OBJECT_NEEDS_BIAS (1<<28) > +#define __EXEC_OBJECT_INTERNAL_FLAGS (0xf<<28) /* all of the above */ s/(0xf<<28)/(~(__EXEC_OBJECT_NEEDS_BIAS-1))/ if you feel like, with the comment retained. Either way: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > #define BATCH_OFFSET_BIAS (256*1024) > > @@ -1007,6 +1008,9 @@ static bool only_mappable_for_reloc(unsigned int flags) > unsigned invalid_flags; > int i; > > + /* INTERNAL flags must not overlap with external ones */ > + BUILD_BUG_ON(__EXEC_OBJECT_INTERNAL_FLAGS & ~__EXEC_OBJECT_UNKNOWN_FLAGS); > + > invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS; > if (USES_FULL_PPGTT(dev)) > invalid_flags |= EXEC_OBJECT_NEEDS_GTT; > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index c17d63d..079d274 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -696,12 +696,13 @@ struct drm_i915_gem_exec_object2 { > */ > __u64 offset; > > -#define EXEC_OBJECT_NEEDS_FENCE (1<<0) > -#define EXEC_OBJECT_NEEDS_GTT (1<<1) > -#define EXEC_OBJECT_WRITE (1<<2) > +#define EXEC_OBJECT_NEEDS_FENCE (1<<0) > +#define EXEC_OBJECT_NEEDS_GTT (1<<1) > +#define EXEC_OBJECT_WRITE (1<<2) > #define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3) > -#define EXEC_OBJECT_PINNED (1<<4) > -#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_PINNED<<1) > +#define EXEC_OBJECT_PINNED (1<<4) > +/* All remaining bits are MBZ and RESERVED FOR FUTURE USE */ > +#define __EXEC_OBJECT_UNKNOWN_FLAGS (-(EXEC_OBJECT_PINNED<<1)) > __u64 flags; > > __u64 rsvd1; > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 7941f1f..608fdc4 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -34,10 +34,11 @@ #include <linux/dma_remapping.h> #include <linux/uaccess.h> -#define __EXEC_OBJECT_HAS_PIN (1<<31) -#define __EXEC_OBJECT_HAS_FENCE (1<<30) -#define __EXEC_OBJECT_NEEDS_MAP (1<<29) -#define __EXEC_OBJECT_NEEDS_BIAS (1<<28) +#define __EXEC_OBJECT_HAS_PIN (1<<31) +#define __EXEC_OBJECT_HAS_FENCE (1<<30) +#define __EXEC_OBJECT_NEEDS_MAP (1<<29) +#define __EXEC_OBJECT_NEEDS_BIAS (1<<28) +#define __EXEC_OBJECT_INTERNAL_FLAGS (0xf<<28) /* all of the above */ #define BATCH_OFFSET_BIAS (256*1024) @@ -1007,6 +1008,9 @@ static bool only_mappable_for_reloc(unsigned int flags) unsigned invalid_flags; int i; + /* INTERNAL flags must not overlap with external ones */ + BUILD_BUG_ON(__EXEC_OBJECT_INTERNAL_FLAGS & ~__EXEC_OBJECT_UNKNOWN_FLAGS); + invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS; if (USES_FULL_PPGTT(dev)) invalid_flags |= EXEC_OBJECT_NEEDS_GTT; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index c17d63d..079d274 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -696,12 +696,13 @@ struct drm_i915_gem_exec_object2 { */ __u64 offset; -#define EXEC_OBJECT_NEEDS_FENCE (1<<0) -#define EXEC_OBJECT_NEEDS_GTT (1<<1) -#define EXEC_OBJECT_WRITE (1<<2) +#define EXEC_OBJECT_NEEDS_FENCE (1<<0) +#define EXEC_OBJECT_NEEDS_GTT (1<<1) +#define EXEC_OBJECT_WRITE (1<<2) #define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3) -#define EXEC_OBJECT_PINNED (1<<4) -#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_PINNED<<1) +#define EXEC_OBJECT_PINNED (1<<4) +/* All remaining bits are MBZ and RESERVED FOR FUTURE USE */ +#define __EXEC_OBJECT_UNKNOWN_FLAGS (-(EXEC_OBJECT_PINNED<<1)) __u64 flags; __u64 rsvd1;
Two different sets of flag bits are stored in the 'flags' member of a 'struct drm_i915_gem_exec_object2', and they're defined in two different source files, increasing the risk of an accidental clash. Some flags in this field are supplied by the user; these are defined in i915_drm.h, and they start from the LSB and work up. Other flags are defined in i915_gem_execbuffer, for internal use within that file only; they start from the MSB and work down. So here we add a compile-time check that the two sets of flags do not overlap, which would cause all sorts of confusion. Signed-off-by: Dave Gordon <david.s.gordon@intel.com> --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 12 ++++++++---- include/uapi/drm/i915_drm.h | 11 ++++++----- 2 files changed, 14 insertions(+), 9 deletions(-)