[PATCH 1/2] drm/i915: introduce GEM_WARN_ON

[PATCH 1/2] drm/i915: introduce GEM_WARN_ON

[Matthew Auld]

In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this
will enable us to freely add warnings which our CI will hopefully catch
but without fear of impacting production machines.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 51ec793f2e20..04c777e6d0bf 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -27,8 +27,10 @@
 
 #ifdef CONFIG_DRM_I915_DEBUG_GEM
 #define GEM_BUG_ON(expr) BUG_ON(expr)
+#define GEM_WARN_ON(expr) WARN_ON(expr)
 #else
 #define GEM_BUG_ON(expr) do { } while (0)
+#define GEM_WARN_ON(expr) do { } while (0)
 #endif
 
 #define I915_NUM_ENGINES 5
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: move vma sanity checking into i915_vma_bind

[Matthew Auld]

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.

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 02fb063302bf..e0746eb5dc54 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1304,15 +1304,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;
@@ -1930,9 +1921,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 4c91a68ecb6d..2c494191d7b3 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

Re: [PATCH 2/2] drm/i915: move vma sanity checking into i915_vma_bind

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 10800 bytes --]

Hi Matthew,

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20161202]
[cannot apply to v4.9-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matthew-Auld/drm-i915-introduce-GEM_WARN_ON/20161203-231346
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-s3-12040051 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/uapi/linux/stddef.h:1:0,
                    from include/linux/stddef.h:4,
                    from include/uapi/linux/posix_types.h:4,
                    from include/uapi/linux/types.h:13,
                    from include/linux/types.h:5,
                    from include/linux/io-mapping.h:21,
                    from drivers/gpu/drm/i915/i915_vma.h:28,
                    from drivers/gpu/drm/i915/i915_vma.c:25:
   drivers/gpu/drm/i915/i915_vma.c: In function 'i915_vma_bind':
   drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
    #define GEM_WARN_ON(expr) do { } while (0)
                              ^
   include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/gpu/drm/i915/i915_vma.c:179:2: note: in expansion of macro 'if'
     if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
     ^~
   drivers/gpu/drm/i915/i915_vma.c:179:6: note: in expansion of macro 'GEM_WARN_ON'
     if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
         ^~~~~~~~~~~
   drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
    #define GEM_WARN_ON(expr) do { } while (0)
                              ^
   include/linux/compiler.h:149:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
>> drivers/gpu/drm/i915/i915_vma.c:179:2: note: in expansion of macro 'if'
     if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
     ^~
   drivers/gpu/drm/i915/i915_vma.c:179:6: note: in expansion of macro 'GEM_WARN_ON'
     if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
         ^~~~~~~~~~~
   drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
    #define GEM_WARN_ON(expr) do { } while (0)
                              ^
   include/linux/compiler.h:160:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
>> drivers/gpu/drm/i915/i915_vma.c:179:2: note: in expansion of macro 'if'
     if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
     ^~
   drivers/gpu/drm/i915/i915_vma.c:179:6: note: in expansion of macro 'GEM_WARN_ON'
     if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
         ^~~~~~~~~~~
   drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
    #define GEM_WARN_ON(expr) do { } while (0)
                              ^
   include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
   drivers/gpu/drm/i915/i915_vma.c:182:2: note: in expansion of macro 'if'
     if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
     ^~
   drivers/gpu/drm/i915/i915_vma.c:182:6: note: in expansion of macro 'GEM_WARN_ON'
     if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
         ^~~~~~~~~~~
   drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
    #define GEM_WARN_ON(expr) do { } while (0)
                              ^
   include/linux/compiler.h:149:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
   drivers/gpu/drm/i915/i915_vma.c:182:2: note: in expansion of macro 'if'
     if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
     ^~
   drivers/gpu/drm/i915/i915_vma.c:182:6: note: in expansion of macro 'GEM_WARN_ON'
     if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
         ^~~~~~~~~~~
   drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
    #define GEM_WARN_ON(expr) do { } while (0)
                              ^
   include/linux/compiler.h:160:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
   drivers/gpu/drm/i915/i915_vma.c:182:2: note: in expansion of macro 'if'
     if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
     ^~
   drivers/gpu/drm/i915/i915_vma.c:182:6: note: in expansion of macro 'GEM_WARN_ON'
     if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
         ^~~~~~~~~~~

vim +/if +179 drivers/gpu/drm/i915/i915_vma.c

    19	 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
    20	 * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
    21	 * IN THE SOFTWARE.
    22	 *
    23	 */
    24	 
  > 25	#include "i915_vma.h"
    26	
    27	#include "i915_drv.h"
    28	#include "intel_ringbuffer.h"
    29	#include "intel_frontbuffer.h"
    30	
    31	#include <drm/drm_gem.h>
    32	
    33	static void
    34	i915_vma_retire(struct i915_gem_active *active,
    35			struct drm_i915_gem_request *rq)
    36	{
    37		const unsigned int idx = rq->engine->id;
    38		struct i915_vma *vma =
    39			container_of(active, struct i915_vma, last_read[idx]);
    40		struct drm_i915_gem_object *obj = vma->obj;
    41	
    42		GEM_BUG_ON(!i915_vma_has_active_engine(vma, idx));
    43	
    44		i915_vma_clear_active(vma, idx);
    45		if (i915_vma_is_active(vma))
    46			return;
    47	
    48		list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
    49		if (unlikely(i915_vma_is_closed(vma) && !i915_vma_is_pinned(vma)))
    50			WARN_ON(i915_vma_unbind(vma));
    51	
    52		GEM_BUG_ON(!i915_gem_object_is_active(obj));
    53		if (--obj->active_count)
    54			return;
    55	
    56		/* Bump our place on the bound list to keep it roughly in LRU order
    57		 * so that we don't steal from recently used but inactive objects
    58		 * (unless we are forced to ofc!)
    59		 */
    60		if (obj->bind_count)
    61			list_move_tail(&obj->global_link, &rq->i915->mm.bound_list);
    62	
    63		obj->mm.dirty = true; /* be paranoid  */
    64	
    65		if (i915_gem_object_has_active_reference(obj)) {
    66			i915_gem_object_clear_active_reference(obj);
    67			i915_gem_object_put(obj);
    68		}
    69	}
    70	
    71	static struct i915_vma *
    72	__i915_vma_create(struct drm_i915_gem_object *obj,
    73			  struct i915_address_space *vm,
    74			  const struct i915_ggtt_view *view)
    75	{
    76		struct i915_vma *vma;
    77		struct rb_node *rb, **p;
    78		int i;
    79	
    80		GEM_BUG_ON(vm->closed);
    81	
    82		vma = kmem_cache_zalloc(to_i915(obj->base.dev)->vmas, GFP_KERNEL);
    83		if (vma == NULL)
    84			return ERR_PTR(-ENOMEM);
    85	
    86		INIT_LIST_HEAD(&vma->exec_list);
    87		for (i = 0; i < ARRAY_SIZE(vma->last_read); i++)
    88			init_request_active(&vma->last_read[i], i915_vma_retire);
    89		init_request_active(&vma->last_fence, NULL);
    90		list_add(&vma->vm_link, &vm->unbound_list);
    91		vma->vm = vm;
    92		vma->obj = obj;
    93		vma->size = obj->base.size;
    94	
    95		if (view) {
    96			vma->ggtt_view = *view;
    97			if (view->type == I915_GGTT_VIEW_PARTIAL) {
    98				vma->size = view->params.partial.size;
    99				vma->size <<= PAGE_SHIFT;
   100			} else if (view->type == I915_GGTT_VIEW_ROTATED) {
   101				vma->size =
   102					intel_rotation_info_size(&view->params.rotated);
   103				vma->size <<= PAGE_SHIFT;
   104			}
   105		}
   106	
   107		if (i915_is_ggtt(vm)) {
   108			vma->flags |= I915_VMA_GGTT;
   109			list_add(&vma->obj_link, &obj->vma_list);
   110		} else {
   111			i915_ppgtt_get(i915_vm_to_ppgtt(vm));
   112			list_add_tail(&vma->obj_link, &obj->vma_list);
   113		}
   114	
   115		rb = NULL;
   116		p = &obj->vma_tree.rb_node;
   117		while (*p) {
   118			struct i915_vma *pos;
   119	
   120			rb = *p;
   121			pos = rb_entry(rb, struct i915_vma, obj_node);
   122			if (i915_vma_compare(pos, vm, view) < 0)
   123				p = &rb->rb_right;
   124			else
   125				p = &rb->rb_left;
   126		}
   127		rb_link_node(&vma->obj_node, rb, p);
   128		rb_insert_color(&vma->obj_node, &obj->vma_tree);
   129	
   130		return vma;
   131	}
   132	
   133	struct i915_vma *
   134	i915_vma_create(struct drm_i915_gem_object *obj,
   135			struct i915_address_space *vm,
   136			const struct i915_ggtt_view *view)
   137	{
   138		lockdep_assert_held(&obj->base.dev->struct_mutex);
   139		GEM_BUG_ON(view && !i915_is_ggtt(vm));
   140		GEM_BUG_ON(i915_gem_obj_to_vma(obj, vm, view));
   141	
   142		return __i915_vma_create(obj, vm, view);
   143	}
   144	
   145	/**
   146	 * i915_vma_bind - Sets up PTEs for an VMA in it's corresponding address space.
   147	 * @vma: VMA to map
   148	 * @cache_level: mapping cache level
   149	 * @flags: flags like global or local mapping
   150	 *
   151	 * DMA addresses are taken from the scatter-gather table of this object (or of
   152	 * this VMA in case of non-default GGTT views) and PTE entries set up.
   153	 * Note that DMA addresses are also the only part of the SG table we care about.
   154	 */
   155	int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
   156			  u32 flags)
   157	{
   158		u32 bind_flags;
   159		u32 vma_flags;
   160		int ret;
   161	
   162		if (WARN_ON(flags == 0))
   163			return -EINVAL;
   164	
   165		bind_flags = 0;
   166		if (flags & PIN_GLOBAL)
   167			bind_flags |= I915_VMA_GLOBAL_BIND;
   168		if (flags & PIN_USER)
   169			bind_flags |= I915_VMA_LOCAL_BIND;
   170	
   171		vma_flags = vma->flags & (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND);
   172		if (flags & PIN_UPDATE)
   173			bind_flags |= vma_flags;
   174		else
   175			bind_flags &= ~vma_flags;
   176		if (bind_flags == 0)
   177			return 0;
   178	
 > 179		if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
   180			return -ENODEV;
   181	
   182		if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26044 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: move vma sanity checking into i915_vma_bind

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7394 bytes --]

Hi Matthew,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20161202]
[cannot apply to v4.9-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Matthew-Auld/drm-i915-introduce-GEM_WARN_ON/20161203-231346
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/i915_gem_request.h:30:0,
                    from drivers/gpu/drm/i915/i915_gem_timeline.h:30,
                    from drivers/gpu/drm/i915/i915_gem_gtt.h:40,
                    from drivers/gpu/drm/i915/i915_vma.h:32,
                    from drivers/gpu/drm/i915/i915_vma.c:25:
   drivers/gpu/drm/i915/i915_vma.c: In function 'i915_vma_bind':
>> drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
    #define GEM_WARN_ON(expr) do { } while (0)
                              ^
>> drivers/gpu/drm/i915/i915_vma.c:179:6: note: in expansion of macro 'GEM_WARN_ON'
     if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
         ^~~~~~~~~~~
>> drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
    #define GEM_WARN_ON(expr) do { } while (0)
                              ^
   drivers/gpu/drm/i915/i915_vma.c:182:6: note: in expansion of macro 'GEM_WARN_ON'
     if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
         ^~~~~~~~~~~

vim +/GEM_WARN_ON +179 drivers/gpu/drm/i915/i915_vma.c

    19	 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
    20	 * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
    21	 * IN THE SOFTWARE.
    22	 *
    23	 */
    24	 
  > 25	#include "i915_vma.h"
    26	
    27	#include "i915_drv.h"
    28	#include "intel_ringbuffer.h"
    29	#include "intel_frontbuffer.h"
    30	
    31	#include <drm/drm_gem.h>
    32	
    33	static void
    34	i915_vma_retire(struct i915_gem_active *active,
    35			struct drm_i915_gem_request *rq)
    36	{
    37		const unsigned int idx = rq->engine->id;
    38		struct i915_vma *vma =
    39			container_of(active, struct i915_vma, last_read[idx]);
    40		struct drm_i915_gem_object *obj = vma->obj;
    41	
    42		GEM_BUG_ON(!i915_vma_has_active_engine(vma, idx));
    43	
    44		i915_vma_clear_active(vma, idx);
    45		if (i915_vma_is_active(vma))
    46			return;
    47	
    48		list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
    49		if (unlikely(i915_vma_is_closed(vma) && !i915_vma_is_pinned(vma)))
    50			WARN_ON(i915_vma_unbind(vma));
    51	
    52		GEM_BUG_ON(!i915_gem_object_is_active(obj));
    53		if (--obj->active_count)
    54			return;
    55	
    56		/* Bump our place on the bound list to keep it roughly in LRU order
    57		 * so that we don't steal from recently used but inactive objects
    58		 * (unless we are forced to ofc!)
    59		 */
    60		if (obj->bind_count)
    61			list_move_tail(&obj->global_link, &rq->i915->mm.bound_list);
    62	
    63		obj->mm.dirty = true; /* be paranoid  */
    64	
    65		if (i915_gem_object_has_active_reference(obj)) {
    66			i915_gem_object_clear_active_reference(obj);
    67			i915_gem_object_put(obj);
    68		}
    69	}
    70	
    71	static struct i915_vma *
    72	__i915_vma_create(struct drm_i915_gem_object *obj,
    73			  struct i915_address_space *vm,
    74			  const struct i915_ggtt_view *view)
    75	{
    76		struct i915_vma *vma;
    77		struct rb_node *rb, **p;
    78		int i;
    79	
    80		GEM_BUG_ON(vm->closed);
    81	
    82		vma = kmem_cache_zalloc(to_i915(obj->base.dev)->vmas, GFP_KERNEL);
    83		if (vma == NULL)
    84			return ERR_PTR(-ENOMEM);
    85	
    86		INIT_LIST_HEAD(&vma->exec_list);
    87		for (i = 0; i < ARRAY_SIZE(vma->last_read); i++)
    88			init_request_active(&vma->last_read[i], i915_vma_retire);
    89		init_request_active(&vma->last_fence, NULL);
    90		list_add(&vma->vm_link, &vm->unbound_list);
    91		vma->vm = vm;
    92		vma->obj = obj;
    93		vma->size = obj->base.size;
    94	
    95		if (view) {
    96			vma->ggtt_view = *view;
    97			if (view->type == I915_GGTT_VIEW_PARTIAL) {
    98				vma->size = view->params.partial.size;
    99				vma->size <<= PAGE_SHIFT;
   100			} else if (view->type == I915_GGTT_VIEW_ROTATED) {
   101				vma->size =
   102					intel_rotation_info_size(&view->params.rotated);
   103				vma->size <<= PAGE_SHIFT;
   104			}
   105		}
   106	
   107		if (i915_is_ggtt(vm)) {
   108			vma->flags |= I915_VMA_GGTT;
   109			list_add(&vma->obj_link, &obj->vma_list);
   110		} else {
   111			i915_ppgtt_get(i915_vm_to_ppgtt(vm));
   112			list_add_tail(&vma->obj_link, &obj->vma_list);
   113		}
   114	
   115		rb = NULL;
   116		p = &obj->vma_tree.rb_node;
   117		while (*p) {
   118			struct i915_vma *pos;
   119	
   120			rb = *p;
   121			pos = rb_entry(rb, struct i915_vma, obj_node);
   122			if (i915_vma_compare(pos, vm, view) < 0)
   123				p = &rb->rb_right;
   124			else
   125				p = &rb->rb_left;
   126		}
   127		rb_link_node(&vma->obj_node, rb, p);
   128		rb_insert_color(&vma->obj_node, &obj->vma_tree);
   129	
   130		return vma;
   131	}
   132	
   133	struct i915_vma *
   134	i915_vma_create(struct drm_i915_gem_object *obj,
   135			struct i915_address_space *vm,
   136			const struct i915_ggtt_view *view)
   137	{
   138		lockdep_assert_held(&obj->base.dev->struct_mutex);
   139		GEM_BUG_ON(view && !i915_is_ggtt(vm));
   140		GEM_BUG_ON(i915_gem_obj_to_vma(obj, vm, view));
   141	
   142		return __i915_vma_create(obj, vm, view);
   143	}
   144	
   145	/**
   146	 * i915_vma_bind - Sets up PTEs for an VMA in it's corresponding address space.
   147	 * @vma: VMA to map
   148	 * @cache_level: mapping cache level
   149	 * @flags: flags like global or local mapping
   150	 *
   151	 * DMA addresses are taken from the scatter-gather table of this object (or of
   152	 * this VMA in case of non-default GGTT views) and PTE entries set up.
   153	 * Note that DMA addresses are also the only part of the SG table we care about.
   154	 */
   155	int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
   156			  u32 flags)
   157	{
   158		u32 bind_flags;
   159		u32 vma_flags;
   160		int ret;
   161	
   162		if (WARN_ON(flags == 0))
   163			return -EINVAL;
   164	
   165		bind_flags = 0;
   166		if (flags & PIN_GLOBAL)
   167			bind_flags |= I915_VMA_GLOBAL_BIND;
   168		if (flags & PIN_USER)
   169			bind_flags |= I915_VMA_LOCAL_BIND;
   170	
   171		vma_flags = vma->flags & (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND);
   172		if (flags & PIN_UPDATE)
   173			bind_flags |= vma_flags;
   174		else
   175			bind_flags &= ~vma_flags;
   176		if (bind_flags == 0)
   177			return 0;
   178	
 > 179		if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
   180			return -ENODEV;
   181	
   182		if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56838 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: introduce GEM_WARN_ON

[Ville Syrjälä]

On Fri, Dec 02, 2016 at 01:23:13PM +0000, Matthew Auld wrote:
> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this
> will enable us to freely add warnings which our CI will hopefully catch
> but without fear of impacting production machines.

Impacting performance?

> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> index 51ec793f2e20..04c777e6d0bf 100644
> --- a/drivers/gpu/drm/i915/i915_gem.h
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -27,8 +27,10 @@
>  
>  #ifdef CONFIG_DRM_I915_DEBUG_GEM
>  #define GEM_BUG_ON(expr) BUG_ON(expr)
> +#define GEM_WARN_ON(expr) WARN_ON(expr)
>  #else
>  #define GEM_BUG_ON(expr) do { } while (0)
> +#define GEM_WARN_ON(expr) do { } while (0)
>  #endif
>  
>  #define I915_NUM_ENGINES 5
> -- 
> 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: introduce GEM_WARN_ON

[Chris Wilson]

On Fri, Dec 02, 2016 at 01:23:13PM +0000, Matthew Auld wrote:
> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this
> will enable us to freely add warnings which our CI will hopefully catch
> but without fear of impacting production machines.
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> index 51ec793f2e20..04c777e6d0bf 100644
> --- a/drivers/gpu/drm/i915/i915_gem.h
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -27,8 +27,10 @@
>  
>  #ifdef CONFIG_DRM_I915_DEBUG_GEM
>  #define GEM_BUG_ON(expr) BUG_ON(expr)
> +#define GEM_WARN_ON(expr) WARN_ON(expr)
>  #else
>  #define GEM_BUG_ON(expr) do { } while (0)
> +#define GEM_WARN_ON(expr) do { } while (0)

#define GEM_WARN_ON 0

i.e. so that if (GEM_WARN_ON(insane_condition()) compiles out correctly
without DEBUG_GEM enabled.
-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

Re: [PATCH 1/2] drm/i915: introduce GEM_WARN_ON

[Matthew Auld]

On 2 December 2016 at 13:54, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Fri, Dec 02, 2016 at 01:23:13PM +0000, Matthew Auld wrote:
>> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this
>> will enable us to freely add warnings which our CI will hopefully catch
>> but without fear of impacting production machines.
>>
>> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_gem.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
>> index 51ec793f2e20..04c777e6d0bf 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.h
>> +++ b/drivers/gpu/drm/i915/i915_gem.h
>> @@ -27,8 +27,10 @@
>>
>>  #ifdef CONFIG_DRM_I915_DEBUG_GEM
>>  #define GEM_BUG_ON(expr) BUG_ON(expr)
>> +#define GEM_WARN_ON(expr) WARN_ON(expr)
>>  #else
>>  #define GEM_BUG_ON(expr) do { } while (0)
>> +#define GEM_WARN_ON(expr) do { } while (0)
>
> #define GEM_WARN_ON 0
Oops. As an alternative, would you be offended with something like:

#define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0)

Then the compiler will still check the validity of the expression, but
will still compile everything out, and then we don't accidentally
break the build when compiling without DEBUG_GEM ?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: introduce GEM_WARN_ON

[Chris Wilson]

On Fri, Dec 02, 2016 at 03:11:05PM +0000, Matthew Auld wrote:
> On 2 December 2016 at 13:54, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Fri, Dec 02, 2016 at 01:23:13PM +0000, Matthew Auld wrote:
> >> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this
> >> will enable us to freely add warnings which our CI will hopefully catch
> >> but without fear of impacting production machines.
> >>
> >> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_gem.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> >> index 51ec793f2e20..04c777e6d0bf 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem.h
> >> +++ b/drivers/gpu/drm/i915/i915_gem.h
> >> @@ -27,8 +27,10 @@
> >>
> >>  #ifdef CONFIG_DRM_I915_DEBUG_GEM
> >>  #define GEM_BUG_ON(expr) BUG_ON(expr)
> >> +#define GEM_WARN_ON(expr) WARN_ON(expr)
> >>  #else
> >>  #define GEM_BUG_ON(expr) do { } while (0)
> >> +#define GEM_WARN_ON(expr) do { } while (0)
> >
> > #define GEM_WARN_ON 0
> Oops. As an alternative, would you be offended with something like:
> 
> #define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0)
> 
> Then the compiler will still check the validity of the expression, but
> will still compile everything out, and then we don't accidentally
> break the build when compiling without DEBUG_GEM ?

Ooh, new shiny. Uses sizeof()! Can you prepare a patch to fixup
GEM_BUG_ON as well?
-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

✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: introduce GEM_WARN_ON

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: introduce GEM_WARN_ON
URL   : https://patchwork.freedesktop.org/series/16278/
State : warning

== Summary ==

Series 16278v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/16278/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6770hq)

fi-bdw-5557u     total:245  pass:230  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:245  pass:205  dwarn:0   dfail:0   fail:0   skip:40 
fi-bxt-t5700     total:245  pass:217  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-j1900     total:245  pass:217  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-n2820     total:245  pass:213  dwarn:0   dfail:0   fail:0   skip:32 
fi-hsw-4770      total:245  pass:225  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:245  pass:225  dwarn:0   dfail:0   fail:0   skip:20 
fi-ilk-650       total:245  pass:192  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:245  pass:223  dwarn:0   dfail:0   fail:0   skip:22 
fi-ivb-3770      total:245  pass:223  dwarn:0   dfail:0   fail:0   skip:22 
fi-kbl-7500u     total:245  pass:223  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6260u     total:245  pass:231  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:245  pass:224  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6700k     total:245  pass:223  dwarn:1   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:245  pass:230  dwarn:1   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:245  pass:213  dwarn:0   dfail:0   fail:0   skip:32 
fi-snb-2600      total:245  pass:212  dwarn:0   dfail:0   fail:0   skip:33 

273202788eba907049316b3ccc2f4d1fd30aff1d drm-tip: 2016y-12m-02d-11h-27m-42s UTC integration manifest
05e8d2b drm/i915: move vma sanity checking into i915_vma_bind
91fae78 drm/i915: introduce GEM_WARN_ON

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3175/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915: introduce GEM_WARN_ON

[Matthew Auld]

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>
---
 drivers/gpu/drm/i915/i915_gem.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 8801a14a78a2..a585d47c420a 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -27,8 +27,10 @@
 
 #ifdef CONFIG_DRM_I915_DEBUG_GEM
 #define GEM_BUG_ON(expr) BUG_ON(expr)
+#define GEM_WARN_ON(expr) WARN_ON(expr)
 #else
 #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
+#define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0)
 #endif
 
 #define I915_NUM_ENGINES 5
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: introduce GEM_WARN_ON

[Joonas Lahtinen]

On ti, 2016-12-13 at 16:00 +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>

<SNIP>

> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -27,8 +27,10 @@
>  
>  #ifdef CONFIG_DRM_I915_DEBUG_GEM
>  #define GEM_BUG_ON(expr) BUG_ON(expr)
> +#define GEM_WARN_ON(expr) WARN_ON(expr)
>  #else
>  #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
> +#define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0)

+#define GEM_WARN_ON(expr) GEM_BUG_ON(expr)

Should be enough in the #else branch just like in linux/mmdebug.h

Regards, joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: introduce GEM_WARN_ON

[Matthew Auld]

On 13 December 2016 at 16:25, Joonas Lahtinen
<joonas.lahtinen@linux.intel.com> wrote:
> On ti, 2016-12-13 at 16:00 +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>
>
> <SNIP>
>
>> +++ b/drivers/gpu/drm/i915/i915_gem.h
>> @@ -27,8 +27,10 @@
>>
>>  #ifdef CONFIG_DRM_I915_DEBUG_GEM
>>  #define GEM_BUG_ON(expr) BUG_ON(expr)
>> +#define GEM_WARN_ON(expr) WARN_ON(expr)
>>  #else
>>  #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
>> +#define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0)
>
> +#define GEM_WARN_ON(expr) GEM_BUG_ON(expr)
>
> Should be enough in the #else branch just like in linux/mmdebug.h
BUILD_BUG_INVALID(expr) will do a (void) cast to discard the result of
the sizeof operation, but then we don't end up ignoring said result if
we use it in the context of an if condition, which will make the
compiler very unhappy.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx