Re: [PATCH 03/21] drm/i915/gtt: Check va range against vm size

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Joonas Lahtinen <joonas.lahtinen@linux.intel.com> writes:

> On pe, 2015-05-22 at 20:04 +0300, Mika Kuoppala wrote:
>> Check the allocation area against the known end
>> of address space instead of against fixed value.
>> 
>> v2: Return ENODEV on internal bugs (Chris)
>> 
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++++-------
>>  1 file changed, 11 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> index 1a5ad4c..76de781 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> @@ -756,9 +756,6 @@ static int gen8_ppgtt_alloc_page_directories(struct i915_hw_ppgtt *ppgtt,
>>  
>>  	WARN_ON(!bitmap_empty(new_pds, GEN8_LEGACY_PDPES));
>>  
>> -	/* FIXME: upper bound must not overflow 32 bits  */
>> -	WARN_ON((start + length) > (1ULL << 32));
>> -
>>  	gen8_for_each_pdpe(pd, pdp, start, length, temp, pdpe) {
>>  		if (pd)
>>  			continue;
>> @@ -857,7 +854,10 @@ static int gen8_alloc_va_range(struct i915_address_space *vm,
>>  	 * actually use the other side of the canonical address space.
>>  	 */
>>  	if (WARN_ON(start + length < start))
>> -		return -ERANGE;
>> +		return -ENODEV;
>> +
>> +	if (WARN_ON(start + length > ppgtt->base.total))
>> +		return -ENODEV;
>>  
>>  	ret = alloc_gen8_temp_bitmaps(&new_page_dirs, &new_page_tables);
>>  	if (ret)
>> @@ -1341,7 +1341,7 @@ static void gen6_initialize_pt(struct i915_address_space *vm,
>>  }
>>  
>>  static int gen6_alloc_va_range(struct i915_address_space *vm,
>> -			       uint64_t start, uint64_t length)
>> +			       uint64_t start_in, uint64_t length_in)
>>  {
>>  	DECLARE_BITMAP(new_page_tables, I915_PDES);
>>  	struct drm_device *dev = vm->dev;
>> @@ -1349,11 +1349,15 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
>>  	struct i915_hw_ppgtt *ppgtt =
>>  				container_of(vm, struct i915_hw_ppgtt, base);
>>  	struct i915_page_table *pt;
>> -	const uint32_t start_save = start, length_save = length;
>> +	uint32_t start, length, start_save, length_save;
>>  	uint32_t pde, temp;
>>  	int ret;
>>  
>> -	WARN_ON(upper_32_bits(start));
>> +	if (WARN_ON(start_in + length_in > ppgtt->base.total))
>> +		return -ENODEV;
>> +
>> +	start = start_save = start_in;
>> +	length = length_save = length_in;
>
> Why is it not enough just to change the WARN_ON test?
>

Might have be cleaner yes. I just wanted to keep the pde iteration
loop using 32bit arithmetic like it used to.
-Mika

>>  
>>  	bitmap_zero(new_page_tables, I915_PDES);
>>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx