From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ben Widawsky <benjamin.widawsky@intel.com>
Subject: Re: [PATCH 1/3] drm/i915: Protect the argument
 expansion in LRI and SRM macros
Date: Mon, 7 Apr 2014 13:47:05 -0700
Message-ID: <20140407204705.GC18726@intel.com>
References: <1396898674-22510-1-git-send-email-damien.lespiau@intel.com>
 <1396898674-22510-2-git-send-email-damien.lespiau@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces@lists.freedesktop.org>
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
 by gabe.freedesktop.org (Postfix) with ESMTP id E4CB86E7AB
 for <intel-gfx@lists.freedesktop.org>; Mon,  7 Apr 2014 16:11:57 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <1396898674-22510-2-git-send-email-damien.lespiau@intel.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
 <mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
 <mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>
To: Damien Lespiau <damien.lespiau@intel.com>
Cc: intel-gfx@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.org

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