Re: [PATCH 1/3] drm/i915: Protect the argument expansion in LRI and SRM macros

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Apr 07, 2014 at 01:47:05PM -0700, Ben Widawsky wrote:
> On Mon, Apr 07, 2014 at 08:24:32PM +0100, Damien Lespiau wrote:
> > It seems like it wouldn't be too unlikely to be wanting to use a an
> > expression in the macro argument and things could go very wrong.
> > 
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 2315366..22d8b14 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -265,8 +265,8 @@
> >   * - One can actually load arbitrary many arbitrary registers: Simply issue x
> >   *   address/value pairs. Don't overdue it, though, x <= 2^4 must hold!
> >   */
> > -#define MI_LOAD_REGISTER_IMM(x)	MI_INSTR(0x22, 2*x-1)
> > -#define MI_STORE_REGISTER_MEM(x) MI_INSTR(0x24, 2*x-1)
> > +#define MI_LOAD_REGISTER_IMM(x)	MI_INSTR(0x22, 2*(x)-1)
> > +#define MI_STORE_REGISTER_MEM(x) MI_INSTR(0x24, 2*(x)-1)
> >  #define   MI_SRM_LRM_GLOBAL_GTT		(1<<22)
> >  #define MI_FLUSH_DW		MI_INSTR(0x26, 1) /* for GEN6 */
> >  #define   MI_FLUSH_DW_STORE_INDEX	(1<<21)
> 
> I was just looking at this macro myself recently (well, LRI, not SRM). I
> think it would be good to WARN_ON values of X which are too large. (I'd
> rather BUG, but we don't have to go that route). Any thoughts on that?
> 
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch