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

Joonas Lahtinen
2015-06-01T15:33:38Z

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?

>
> bitmap_zero(new_page_tables, I915_PDES);