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

[Ben Widawsky <benjamin.widawsky@intel.com>]

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>


-- 
Ben Widawsky, Intel Open Source Technology Center