| Message ID | 1432314314-23530-4-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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); >
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); >>
On 6/11/2015 3:23 PM, Mika Kuoppala wrote: > 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(-) >>> >> >> 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. Unless Joonas has any more comments, Reviewed-by: Michel Thierry <michel.thierry@intel.com> > -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 >
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; bitmap_zero(new_page_tables, I915_PDES);
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(-)