Re: [PATCH 11/13] drm/i915: Reject commands that would store to global HWS page

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 29 Jan 2014, bradley.d.volkin@intel.com wrote:
> From: Brad Volkin <bradley.d.volkin@intel.com>
>
> PIPE_CONTROL and MI_FLUSH_DW have bits that would write to the
> hardware status page. The driver stores request tracking info
> there, so don't let userspace overwrite it.
>
> Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_cmd_parser.c | 30 ++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/i915_reg.h        |  1 +
>  2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index 26072a2..b93df1c 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -141,7 +141,8 @@ static const struct drm_i915_cmd_descriptor render_cmds[] = {
>  	      },
>  	      {
>  			.offset = 1,
> -		        .mask = PIPE_CONTROL_GLOBAL_GTT_IVB,
> +		        .mask = (PIPE_CONTROL_GLOBAL_GTT_IVB |
> +				 PIPE_CONTROL_STORE_DATA_INDEX),
>  			.expected = 0,
>  			.condition_offset = 1,
>  			.condition_mask = PIPE_CONTROL_POST_SYNC_OP_MASK
> @@ -192,8 +193,15 @@ static const struct drm_i915_cmd_descriptor video_cmds[] = {
>  			.expected = 0,
>  			.condition_offset = 0,
>  			.condition_mask = MI_FLUSH_DW_OP_MASK
> +	      },
> +	      {
> +			.offset = 0,
> +			.mask = MI_FLUSH_DW_STORE_INDEX,
> +			.expected = 0,
> +			.condition_offset = 0,
> +			.condition_mask = MI_FLUSH_DW_OP_MASK
>  	      }},
> -	      .bits_count = 2                                          ),
> +	      .bits_count = 3                                          ),

I'm disliking this separate .bits_count more at every change...

>  	CMD(  MI_CONDITIONAL_BATCH_BUFFER_END,  SMI,   !F,  0xFF,   B,
>  	      .bits = {{
>  			.offset = 0,
> @@ -231,8 +239,15 @@ static const struct drm_i915_cmd_descriptor vecs_cmds[] = {
>  			.expected = 0,
>  			.condition_offset = 0,
>  			.condition_mask = MI_FLUSH_DW_OP_MASK
> +	      },
> +	      {
> +			.offset = 0,
> +			.mask = MI_FLUSH_DW_STORE_INDEX,
> +			.expected = 0,
> +			.condition_offset = 0,
> +			.condition_mask = MI_FLUSH_DW_OP_MASK
>  	      }},
> -	      .bits_count = 2					       ),
> +	      .bits_count = 3					       ),
>  	CMD(  MI_CONDITIONAL_BATCH_BUFFER_END,  SMI,   !F,  0xFF,   B,
>  	      .bits = {{
>  			.offset = 0,
> @@ -264,8 +279,15 @@ static const struct drm_i915_cmd_descriptor blt_cmds[] = {
>  			.expected = 0,
>  			.condition_offset = 0,
>  			.condition_mask = MI_FLUSH_DW_OP_MASK
> +	      },
> +	      {
> +			.offset = 0,
> +			.mask = MI_FLUSH_DW_STORE_INDEX,
> +			.expected = 0,
> +			.condition_offset = 0,
> +			.condition_mask = MI_FLUSH_DW_OP_MASK
>  	      }},
> -	      .bits_count = 2					       ),
> +	      .bits_count = 3					       ),
>  	CMD(  COLOR_BLT,                        S2D,   !F,  0x3F,   S  ),
>  	CMD(  SRC_COPY_BLT,                     S2D,   !F,  0x3F,   S  ),
>  };
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ff263f4..5f77cb6 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -324,6 +324,7 @@
>  #define GFX_OP_PIPE_CONTROL(len)	((0x3<<29)|(0x3<<27)|(0x2<<24)|(len-2))
>  #define   PIPE_CONTROL_GLOBAL_GTT_IVB			(1<<24) /* gen7+ */
>  #define   PIPE_CONTROL_MMIO_WRITE			(1<<23)
> +#define   PIPE_CONTROL_STORE_DATA_INDEX			(1<<21)
>  #define   PIPE_CONTROL_CS_STALL				(1<<20)
>  #define   PIPE_CONTROL_TLB_INVALIDATE			(1<<18)
>  #define   PIPE_CONTROL_QW_WRITE				(1<<14)
> -- 
> 1.8.5.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center