* [PATCH 2/4] drm/i915: move vma sanity checking into i915_vma_bind
2016-12-13 20:32 [PATCH 1/4] drm/i915: introduce GEM_WARN_ON Matthew Auld
@ 2016-12-13 20:32 ` Matthew Auld
2016-12-13 21:34 ` Chris Wilson
2016-12-13 20:32 ` [PATCH 3/4] drm/i915: introduce range_overflows utility macros Matthew Auld
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Matthew Auld @ 2016-12-13 20:32 UTC (permalink / raw)
To: intel-gfx
If we move the sanity checking from gen8_alloc_va_range_3lvl and
gen6_alloc_va_range into i915_vma_bind, we will increase our coverage to
now both callbacks. We also convert each WARN_ON over to a GEM_WARN_ON.
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 12 ------------
drivers/gpu/drm/i915/i915_vma.c | 6 ++++++
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ef00d36680c9..4f405b445b6c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1303,15 +1303,6 @@ static int gen8_alloc_va_range_3lvl(struct i915_address_space *vm,
uint32_t pdpes = I915_PDPES_PER_PDP(dev_priv);
int ret;
- /* Wrap is never okay since we can only represent 48b, and we don't
- * actually use the other side of the canonical address space.
- */
- if (WARN_ON(start + length < start))
- return -ENODEV;
-
- if (WARN_ON(start + length > vm->total))
- return -ENODEV;
-
ret = alloc_gen8_temp_bitmaps(&new_page_dirs, &new_page_tables, pdpes);
if (ret)
return ret;
@@ -1929,9 +1920,6 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
uint32_t pde;
int ret;
- if (WARN_ON(start_in + length_in > ppgtt->base.total))
- return -ENODEV;
-
start = start_save = start_in;
length = length_save = length_in;
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 37c3eebe8316..9e121222c5eb 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -176,6 +176,12 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
if (bind_flags == 0)
return 0;
+ if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
+ return -ENODEV;
+
+ if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
+ return -ENODEV;
+
if (vma_flags == 0 && vma->vm->allocate_va_range) {
trace_i915_va_alloc(vma);
ret = vma->vm->allocate_va_range(vma->vm,
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] drm/i915: move vma sanity checking into i915_vma_bind
2016-12-13 20:32 ` [PATCH 2/4] drm/i915: move vma sanity checking into i915_vma_bind Matthew Auld
@ 2016-12-13 21:34 ` Chris Wilson
0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2016-12-13 21:34 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx
On Tue, Dec 13, 2016 at 08:32:20PM +0000, Matthew Auld wrote:
> If we move the sanity checking from gen8_alloc_va_range_3lvl and
> gen6_alloc_va_range into i915_vma_bind, we will increase our coverage to
> now both callbacks. We also convert each WARN_ON over to a GEM_WARN_ON.
>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] drm/i915: introduce range_overflows utility macros
2016-12-13 20:32 [PATCH 1/4] drm/i915: introduce GEM_WARN_ON Matthew Auld
2016-12-13 20:32 ` [PATCH 2/4] drm/i915: move vma sanity checking into i915_vma_bind Matthew Auld
@ 2016-12-13 20:32 ` Matthew Auld
2016-12-13 21:31 ` Chris Wilson
2016-12-13 20:32 ` [PATCH 4/4] drm/i915: convert to using range_overflows Matthew Auld
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Matthew Auld @ 2016-12-13 20:32 UTC (permalink / raw)
To: intel-gfx
In a number places we hand-roll the overflow sanity check for ranges, so
roll that into single macro, conceived by Chris, along with its typed
variant.
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 20bc04d5e617..47b5a9f1c9ed 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -218,6 +218,18 @@ static inline const char *enableddisabled(bool v)
return v ? "enabled" : "disabled";
}
+#define range_overflows(start, size, max) ({ \
+ typeof(start) start__ = (start); \
+ typeof(size) size__ = (size); \
+ typeof(max) max__ = (max); \
+ (void)(&start__ == &size__); \
+ (void)(&start__ == &max__); \
+ start > max || size > max - start; \
+})
+
+#define range_overflows_t(type, start, size, max) \
+ range_overflows(((type)start), ((type)size), ((type)max))
+
enum pipe {
INVALID_PIPE = -1,
PIPE_A = 0,
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/4] drm/i915: introduce range_overflows utility macros
2016-12-13 20:32 ` [PATCH 3/4] drm/i915: introduce range_overflows utility macros Matthew Auld
@ 2016-12-13 21:31 ` Chris Wilson
0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2016-12-13 21:31 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx
On Tue, Dec 13, 2016 at 08:32:21PM +0000, Matthew Auld wrote:
> In a number places we hand-roll the overflow sanity check for ranges, so
> roll that into single macro, conceived by Chris, along with its typed
> variant.
>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 20bc04d5e617..47b5a9f1c9ed 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -218,6 +218,18 @@ static inline const char *enableddisabled(bool v)
> return v ? "enabled" : "disabled";
> }
>
> +#define range_overflows(start, size, max) ({ \
> + typeof(start) start__ = (start); \
> + typeof(size) size__ = (size); \
> + typeof(max) max__ = (max); \
> + (void)(&start__ == &size__); \
> + (void)(&start__ == &max__); \
> + start > max || size > max - start; \
> +})
> +
> +#define range_overflows_t(type, start, size, max) \
> + range_overflows(((type)start), ((type)size), ((type)max))
Hmm, should be (type)(start) etc. Yeah, this is slightly more obvious
than min_t's method.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] drm/i915: convert to using range_overflows
2016-12-13 20:32 [PATCH 1/4] drm/i915: introduce GEM_WARN_ON Matthew Auld
2016-12-13 20:32 ` [PATCH 2/4] drm/i915: move vma sanity checking into i915_vma_bind Matthew Auld
2016-12-13 20:32 ` [PATCH 3/4] drm/i915: introduce range_overflows utility macros Matthew Auld
@ 2016-12-13 20:32 ` Matthew Auld
2016-12-13 21:32 ` Chris Wilson
2016-12-13 21:15 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: introduce GEM_WARN_ON Patchwork
2016-12-13 21:35 ` [PATCH 1/4] " Chris Wilson
4 siblings, 1 reply; 10+ messages in thread
From: Matthew Auld @ 2016-12-13 20:32 UTC (permalink / raw)
To: intel-gfx
Convert some of the obvious hand-rolled ranged overflow sanity checks to
our shiny new range_overflows macro.
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 6 ++----
drivers/gpu/drm/i915/i915_vma.c | 6 ++----
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f86a71d9fe37..782be625d37d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1140,8 +1140,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
return -ENOENT;
/* Bounds check source. */
- if (args->offset > obj->base.size ||
- args->size > obj->base.size - args->offset) {
+ if (range_overflows_t(u64, args->offset, args->size, obj->base.size)) {
ret = -EINVAL;
goto out;
}
@@ -1454,8 +1453,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
return -ENOENT;
/* Bounds check destination. */
- if (args->offset > obj->base.size ||
- args->size > obj->base.size - args->offset) {
+ if (range_overflows_t(u64, args->offset, args->size, obj->base.size)) {
ret = -EINVAL;
goto err;
}
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 9e121222c5eb..47599390dca7 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -176,10 +176,8 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
if (bind_flags == 0)
return 0;
- if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
- return -ENODEV;
-
- if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
+ if (GEM_WARN_ON(range_overflows(vma->node.start, vma->node.size,
+ vma->vm->total)))
return -ENODEV;
if (vma_flags == 0 && vma->vm->allocate_va_range) {
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread* ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: introduce GEM_WARN_ON
2016-12-13 20:32 [PATCH 1/4] drm/i915: introduce GEM_WARN_ON Matthew Auld
` (2 preceding siblings ...)
2016-12-13 20:32 ` [PATCH 4/4] drm/i915: convert to using range_overflows Matthew Auld
@ 2016-12-13 21:15 ` Patchwork
2016-12-16 21:28 ` Chris Wilson
2016-12-13 21:35 ` [PATCH 1/4] " Chris Wilson
4 siblings, 1 reply; 10+ messages in thread
From: Patchwork @ 2016-12-13 21:15 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/4] drm/i915: introduce GEM_WARN_ON
URL : https://patchwork.freedesktop.org/series/16758/
State : success
== Summary ==
Series 16758v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/16758/revisions/1/mbox/
fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14
fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39
fi-bxt-t5700 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27
fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27
fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31
fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19
fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19
fi-ilk-650 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52
fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-kbl-7500u total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13
fi-skl-6700hq total:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20
fi-skl-6700k total:247 pass:224 dwarn:3 dfail:0 fail:0 skip:20
fi-skl-6770hq total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13
fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31
fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32
57b72b4a6d7b787aedeb7b776ab35c3b2d7f58e9 drm-tip: 2016y-12m-13d-17h-59m-26s UTC integration manifest
ec9dba0 drm/i915: convert to using range_overflows
2e82cdf drm/i915: introduce range_overflows utility macros
b2b8e00 drm/i915: move vma sanity checking into i915_vma_bind
16aef35 drm/i915: introduce GEM_WARN_ON
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3282/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: introduce GEM_WARN_ON
2016-12-13 21:15 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: introduce GEM_WARN_ON Patchwork
@ 2016-12-16 21:28 ` Chris Wilson
0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2016-12-16 21:28 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
On Tue, Dec 13, 2016 at 09:15:35PM -0000, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [1/4] drm/i915: introduce GEM_WARN_ON
> URL : https://patchwork.freedesktop.org/series/16758/
> State : success
>
> == Summary ==
>
> Series 16758v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/16758/revisions/1/mbox/
>
>
> fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14
> fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39
> fi-bxt-t5700 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27
> fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27
> fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31
> fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19
> fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19
> fi-ilk-650 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52
> fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
> fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
> fi-kbl-7500u total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21
> fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13
> fi-skl-6700hq total:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20
> fi-skl-6700k total:247 pass:224 dwarn:3 dfail:0 fail:0 skip:20
> fi-skl-6770hq total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13
> fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31
> fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32
>
> 57b72b4a6d7b787aedeb7b776ab35c3b2d7f58e9 drm-tip: 2016y-12m-13d-17h-59m-26s UTC integration manifest
> ec9dba0 drm/i915: convert to using range_overflows
> 2e82cdf drm/i915: introduce range_overflows utility macros
> b2b8e00 drm/i915: move vma sanity checking into i915_vma_bind
> 16aef35 drm/i915: introduce GEM_WARN_ON
And pushed. Thanks,
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] drm/i915: introduce GEM_WARN_ON
2016-12-13 20:32 [PATCH 1/4] drm/i915: introduce GEM_WARN_ON Matthew Auld
` (3 preceding siblings ...)
2016-12-13 21:15 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: introduce GEM_WARN_ON Patchwork
@ 2016-12-13 21:35 ` Chris Wilson
4 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2016-12-13 21:35 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx
On Tue, Dec 13, 2016 at 08:32:19PM +0000, Matthew Auld wrote:
> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, with the
> simple goal of expressing warnings which are truly insane, and so are
> only really useful for CI where we have some abusive tests.
>
> v2:
> - use BUILD_BUG_ON_INVALID for !DEBUG_GEM
> - clarify commit message
>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread