Re: [Intel-gfx] [PATCH] drm/i915/gem: Support parsing of oversize batches

[Matthew Auld <matthew.auld@intel.com>]

On 15/10/2020 12:59, Chris Wilson wrote:
> Matthew Auld noted that on more recent systems (such as the parser for
> gen9) we may have objects that are larger than expected by the GEM uAPI
> (i.e. greater than u32). These objects would have incorrect implicit
> batch lengths, causing the parser to reject them for being incomplete,
> or worse.
> 
> Based on a patch by Matthew Auld.
> 
> Reported-by: Matthew Auld <matthew.auld@intel.com>
> Fixes: 435e8fc059db ("drm/i915: Allow parsing of unsized batches")
> Testcase: igt/gem_exec_params/larger-than-life-batch
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 4b09bcd70cf4..44b4558d5e86 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -287,8 +287,8 @@ struct i915_execbuffer {
>   	u64 invalid_flags; /** Set of execobj.flags that are invalid */
>   	u32 context_flags; /** Set of execobj.flags to insert from the ctx */
>   
> +	u64 batch_len; /** Length of batch within object */
>   	u32 batch_start_offset; /** Location within object of batch */
> -	u32 batch_len; /** Length of batch within object */
>   	u32 batch_flags; /** Flags composed for emit_bb_start() */
>   	struct intel_gt_buffer_pool_node *batch_pool; /** pool node for batch buffer */
>   
> @@ -871,6 +871,10 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
>   
>   	if (eb->batch_len == 0)
>   		eb->batch_len = eb->batch->vma->size - eb->batch_start_offset;
> +	if (eb->batch_len == 0) {
> +		drm_dbg(&i915->drm, "Invalid batch length\n");
> +		return -EINVAL;
> +	}

This one should be impossible, or at least we should have hit the 
range_overflows check first?

Anyway, looks good to me,
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx