[PATCH 1/3] drm/i915: Retire the VMA's fence tracker before unbinding

[PATCH 1/3] drm/i915: Retire the VMA's fence tracker before unbinding

[Chris Wilson]

Since we may track unfenced access (GPU access to the vma that
explicitly requires no fence), vma->last_fence may be set without any
attached fence (vma->fence) and so will not be flushed when we call
i915_vma_put_fence(). Since we stopped doing a full retire of the
activity trackers for unbind, we need to explicitly retire each tracker.

Fixes: b0decaf75bd9 ("drm/i915: Track active vma requests")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_vma.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 532c709febbd..1cfe137cdc32 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -672,6 +672,11 @@ int i915_vma_unbind(struct i915_vma *vma)
 				break;
 		}
 
+		if (!ret) {
+			ret = i915_gem_active_retire(&vma->last_fence,
+						     &vma->vm->i915->drm.struct_mutex);
+		}
+
 		__i915_vma_unpin(vma);
 		if (ret)
 			return ret;
-- 
2.11.0

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

[PATCH 2/3] drm/i915: Pass the right flags to i915_vma_move_to_active()

[Chris Wilson]

i915_vma_move_to_active() takes the execobject flags and not a boolean!
Instead of passing EXEC_OBJECT_WRITE we passed true [i.e.
EXEC_OBJECT_NEEDS_FENCE] causing us to start tracking the
vma->last_fence access and since we forgot to clear that on unbinding,
we caused a use-after-free.

[  321.263854] BUG: KASAN: use-after-free in i915_gem_request_retire+0x1728/0x1740 [i915]
[  321.264001] Read of size 8 at addr ffff880100fc67d8 by task gem_exec_reloc/2868

[  321.264181] CPU: 0 PID: 2868 Comm: gem_exec_reloc Not tainted 4.12.0-rc6-CI-Custom_2759+ #1
[  321.264195] Hardware name: GIGABYTE GB-BXBT-1900/MZBAYAB-00, BIOS F6 02/17/2015
[  321.264208] Call Trace:
[  321.264234]  dump_stack+0x67/0x99
[  321.264260]  print_address_description+0x77/0x290
[  321.264437]  ? i915_gem_request_retire+0x1728/0x1740 [i915]
[  321.264459]  kasan_report+0x269/0x350
[  321.264487]  __asan_report_load8_noabort+0x14/0x20
[  321.264660]  i915_gem_request_retire+0x1728/0x1740 [i915]
[  321.264841]  ? intel_ring_context_pin+0x131/0x690 [i915]
[  321.265021]  i915_gem_request_alloc+0x2c6/0x1220 [i915]
[  321.265044]  ? _raw_spin_unlock_irqrestore+0x3d/0x60
[  321.265226]  i915_gem_do_execbuffer+0xac0/0x2a20 [i915]
[  321.265250]  ? __lock_acquire+0xceb/0x5450
[  321.265269]  ? entry_SYSCALL_64_fastpath+0x1c/0xb1
[  321.265291]  ? kvmalloc_node+0x6b/0x80
[  321.265310]  ? kvmalloc_node+0x6b/0x80
[  321.265489]  ? eb_relocate_slow+0xbe0/0xbe0 [i915]
[  321.265520]  ? ___slab_alloc.constprop.28+0x2ab/0x3d0
[  321.265549]  ? debug_check_no_locks_freed+0x280/0x280
[  321.265591]  ? __might_fault+0xc6/0x1b0
[  321.265782]  i915_gem_execbuffer2+0x14a/0x3f0 [i915]
[  321.265815]  drm_ioctl+0x4ba/0xaa0
[  321.265986]  ? i915_gem_execbuffer+0xde0/0xde0 [i915]
[  321.266017]  ? drm_getunique+0x270/0x270
[  321.266068]  do_vfs_ioctl+0x17f/0xfa0
[  321.266091]  ? __fget+0x1ba/0x330
[  321.266112]  ? lock_acquire+0x390/0x390
[  321.266133]  ? ioctl_preallocate+0x1d0/0x1d0
[  321.266164]  ? __fget+0x1db/0x330
[  321.266194]  ? __fget_light+0x79/0x1f0
[  321.266219]  SyS_ioctl+0x3c/0x70
[  321.266247]  entry_SYSCALL_64_fastpath+0x1c/0xb1
[  321.266265] RIP: 0033:0x7fcede207357
[  321.266279] RSP: 002b:00007ffef0effe58 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[  321.266307] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007fcede207357
[  321.266321] RDX: 00007ffef0effef0 RSI: 0000000040406469 RDI: 0000000000000004
[  321.266335] RBP: ffffffff812097c6 R08: 0000000000000008 R09: 0000000000000000
[  321.266349] R10: 0000000000000008 R11: 0000000000000246 R12: ffff880116bcff98
[  321.266363] R13: ffffffff81cb7cb3 R14: ffff880116bcff70 R15: 0000000000000000
[  321.266385]  ? __this_cpu_preempt_check+0x13/0x20
[  321.266406]  ? trace_hardirqs_off_caller+0x1d6/0x2c0

[  321.266487] Allocated by task 2868:
[  321.266568]  save_stack_trace+0x16/0x20
[  321.266586]  kasan_kmalloc+0xee/0x180
[  321.266602]  kasan_slab_alloc+0x12/0x20
[  321.266620]  kmem_cache_alloc+0xc7/0x2e0
[  321.266795]  i915_vma_instance+0x28c/0x1540 [i915]
[  321.266964]  eb_lookup_vmas+0x5a7/0x2250 [i915]
[  321.267130]  i915_gem_do_execbuffer+0x69a/0x2a20 [i915]
[  321.267296]  i915_gem_execbuffer2+0x14a/0x3f0 [i915]
[  321.267315]  drm_ioctl+0x4ba/0xaa0
[  321.267333]  do_vfs_ioctl+0x17f/0xfa0
[  321.267350]  SyS_ioctl+0x3c/0x70
[  321.267369]  entry_SYSCALL_64_fastpath+0x1c/0xb1

[  321.267428] Freed by task 177:
[  321.267502]  save_stack_trace+0x16/0x20
[  321.267521]  kasan_slab_free+0xad/0x180
[  321.267539]  kmem_cache_free+0xc5/0x340
[  321.267710]  i915_vma_unbind+0x666/0x10a0 [i915]
[  321.267880]  i915_vma_close+0x23a/0x2f0 [i915]
[  321.268048]  __i915_gem_free_objects+0x17d/0xc70 [i915]
[  321.268215]  __i915_gem_free_work+0x49/0x70 [i915]
[  321.268234]  process_one_work+0x66f/0x1410
[  321.268252]  worker_thread+0xe1/0xe90
[  321.268269]  kthread+0x304/0x410
[  321.268285]  ret_from_fork+0x27/0x40

[  321.268346] The buggy address belongs to the object at ffff880100fc6640
                which belongs to the cache i915_vma of size 656
[  321.268550] The buggy address is located 408 bytes inside of
                656-byte region [ffff880100fc6640, ffff880100fc68d0)
[  321.268741] The buggy address belongs to the page:
[  321.268837] page:ffffea000403f000 count:1 mapcount:0 mapping:          (null) index:0xffff880100fc5980 compound_mapcount: 0
[  321.269045] flags: 0x8000000000008100(slab|head)
[  321.269147] raw: 8000000000008100 0000000000000000 ffff880100fc5980 00000001001e001d
[  321.269312] raw: ffffea0004038e20 ffff880116b46240 ffff88011646c640 0000000000000000
[  321.269484] page dumped because: kasan: bad access detected

[  321.269665] Memory state around the buggy address:
[  321.269778]  ffff880100fc6680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  321.269949]  ffff880100fc6700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  321.270115] >ffff880100fc6780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  321.270279]                                                     ^
[  321.270410]  ffff880100fc6800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  321.270576]  ffff880100fc6880: fb fb fb fb fb fb fb fb fb fb fc fc fc fc fc fc
[  321.270740] ==================================================================
[  321.270903] Disabling lock debugging due to kernel taint

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101511
Fixes: 7dd4f6729f92 ("drm/i915: Async GPU relocation processing")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index eb46dfa374a7..a23a1c3503c8 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1199,7 +1199,7 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
 	reservation_object_unlock(batch->resv);
 	i915_vma_unpin(batch);
 
-	i915_vma_move_to_active(vma, rq, true);
+	i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
 	reservation_object_lock(vma->resv, NULL);
 	reservation_object_add_excl_fence(vma->resv, &rq->fence);
 	reservation_object_unlock(vma->resv);
-- 
2.11.0

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

[PATCH 3/3] drm/i915: Assert the vma's active tracking is clear before free

[Chris Wilson]

In looking at a use-after-free on Baytrail, it looks like the VMA's
activity tracking is suspect. Add some asserts to catch freeing the VMA
before we have decoupled all of its i915_gem_active trackers.

References: https://bugs.freedesktop.org/show_bug.cgi?id=101511
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
c: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_vma.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 1cfe137cdc32..958be0a95960 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -579,11 +579,17 @@ int __i915_vma_do_pin(struct i915_vma *vma,
 
 static void i915_vma_destroy(struct i915_vma *vma)
 {
+	int i;
+
 	GEM_BUG_ON(vma->node.allocated);
 	GEM_BUG_ON(i915_vma_is_active(vma));
 	GEM_BUG_ON(!i915_vma_is_closed(vma));
 	GEM_BUG_ON(vma->fence);
 
+	for (i = 0; i < ARRAY_SIZE(vma->last_read); i++)
+		GEM_BUG_ON(i915_gem_active_isset(&vma->last_read[i]));
+	GEM_BUG_ON(i915_gem_active_isset(&vma->last_fence));
+
 	list_del(&vma->vm_link);
 	if (!i915_vma_is_ggtt(vma))
 		i915_ppgtt_put(i915_vm_to_ppgtt(vma->vm));
@@ -680,9 +686,8 @@ int i915_vma_unbind(struct i915_vma *vma)
 		__i915_vma_unpin(vma);
 		if (ret)
 			return ret;
-
-		GEM_BUG_ON(i915_vma_is_active(vma));
 	}
+	GEM_BUG_ON(i915_vma_is_active(vma));
 
 	if (i915_vma_is_pinned(vma))
 		return -EBUSY;
-- 
2.11.0

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

✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Retire the VMA's fence tracker before unbinding

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Retire the VMA's fence tracker before unbinding
URL   : https://patchwork.freedesktop.org/series/26045/
State : failure

== Summary ==

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

Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> INCOMPLETE (fi-hsw-4770)
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100125
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> FAIL       (fi-snb-2600) fdo#100215
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-j1900) fdo#101516
Test prime_busy:
        Subgroup basic-wait-after-default:
                pass       -> DMESG-WARN (fi-skl-6700hq) fdo#101515 +2
Test prime_vgem:
        Subgroup basic-fence-flip:
                pass       -> FAIL       (fi-byt-j1900)

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101516 https://bugs.freedesktop.org/show_bug.cgi?id=101516
fdo#101515 https://bugs.freedesktop.org/show_bug.cgi?id=101515

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:446s
fi-bsw-n3050     total:278  pass:241  dwarn:1   dfail:0   fail:0   skip:36  time:536s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:504s
fi-byt-j1900     total:278  pass:251  dwarn:2   dfail:0   fail:1   skip:24  time:497s
fi-byt-n2820     total:278  pass:248  dwarn:2   dfail:0   fail:0   skip:28  time:478s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:598s
fi-hsw-4770      total:108  pass:99   dwarn:0   dfail:0   fail:0   skip:8  
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:410s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:418s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:490s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:469s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:467s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:569s
fi-kbl-r         total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time:581s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:465s
fi-skl-6700hq    total:278  pass:220  dwarn:3   dfail:0   fail:30  skip:24  time:344s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:464s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:479s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:432s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:539s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time:408s
fi-bdw-gvtdvm failed to collect. IGT log at Patchwork_4996/fi-bdw-gvtdvm/igt.log

3e0add868ae0aa06f3203e3862ea74ab94cab013 drm-tip: 2017y-06m-20d-11h-42m-47s UTC integration manifest
4969e81 drm/i915: Assert the vma's active tracking is clear before free
314a4e5 drm/i915: Pass the right flags to i915_vma_move_to_active()
7db76de drm/i915: Retire the VMA's fence tracker before unbinding

== Logs ==

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

Re: [PATCH 2/3] drm/i915: Pass the right flags to i915_vma_move_to_active()

[Tvrtko Ursulin]

On 20/06/2017 13:43, Chris Wilson wrote:
> i915_vma_move_to_active() takes the execobject flags and not a boolean!
> Instead of passing EXEC_OBJECT_WRITE we passed true [i.e.
> EXEC_OBJECT_NEEDS_FENCE] causing us to start tracking the
> vma->last_fence access and since we forgot to clear that on unbinding,
> we caused a use-after-free.
> 
> [  321.263854] BUG: KASAN: use-after-free in i915_gem_request_retire+0x1728/0x1740 [i915]
> [  321.264001] Read of size 8 at addr ffff880100fc67d8 by task gem_exec_reloc/2868
> 
> [  321.264181] CPU: 0 PID: 2868 Comm: gem_exec_reloc Not tainted 4.12.0-rc6-CI-Custom_2759+ #1
> [  321.264195] Hardware name: GIGABYTE GB-BXBT-1900/MZBAYAB-00, BIOS F6 02/17/2015
> [  321.264208] Call Trace:
> [  321.264234]  dump_stack+0x67/0x99
> [  321.264260]  print_address_description+0x77/0x290
> [  321.264437]  ? i915_gem_request_retire+0x1728/0x1740 [i915]
> [  321.264459]  kasan_report+0x269/0x350
> [  321.264487]  __asan_report_load8_noabort+0x14/0x20
> [  321.264660]  i915_gem_request_retire+0x1728/0x1740 [i915]
> [  321.264841]  ? intel_ring_context_pin+0x131/0x690 [i915]
> [  321.265021]  i915_gem_request_alloc+0x2c6/0x1220 [i915]
> [  321.265044]  ? _raw_spin_unlock_irqrestore+0x3d/0x60
> [  321.265226]  i915_gem_do_execbuffer+0xac0/0x2a20 [i915]
> [  321.265250]  ? __lock_acquire+0xceb/0x5450
> [  321.265269]  ? entry_SYSCALL_64_fastpath+0x1c/0xb1
> [  321.265291]  ? kvmalloc_node+0x6b/0x80
> [  321.265310]  ? kvmalloc_node+0x6b/0x80
> [  321.265489]  ? eb_relocate_slow+0xbe0/0xbe0 [i915]
> [  321.265520]  ? ___slab_alloc.constprop.28+0x2ab/0x3d0
> [  321.265549]  ? debug_check_no_locks_freed+0x280/0x280
> [  321.265591]  ? __might_fault+0xc6/0x1b0
> [  321.265782]  i915_gem_execbuffer2+0x14a/0x3f0 [i915]
> [  321.265815]  drm_ioctl+0x4ba/0xaa0
> [  321.265986]  ? i915_gem_execbuffer+0xde0/0xde0 [i915]
> [  321.266017]  ? drm_getunique+0x270/0x270
> [  321.266068]  do_vfs_ioctl+0x17f/0xfa0
> [  321.266091]  ? __fget+0x1ba/0x330
> [  321.266112]  ? lock_acquire+0x390/0x390
> [  321.266133]  ? ioctl_preallocate+0x1d0/0x1d0
> [  321.266164]  ? __fget+0x1db/0x330
> [  321.266194]  ? __fget_light+0x79/0x1f0
> [  321.266219]  SyS_ioctl+0x3c/0x70
> [  321.266247]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> [  321.266265] RIP: 0033:0x7fcede207357
> [  321.266279] RSP: 002b:00007ffef0effe58 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [  321.266307] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007fcede207357
> [  321.266321] RDX: 00007ffef0effef0 RSI: 0000000040406469 RDI: 0000000000000004
> [  321.266335] RBP: ffffffff812097c6 R08: 0000000000000008 R09: 0000000000000000
> [  321.266349] R10: 0000000000000008 R11: 0000000000000246 R12: ffff880116bcff98
> [  321.266363] R13: ffffffff81cb7cb3 R14: ffff880116bcff70 R15: 0000000000000000
> [  321.266385]  ? __this_cpu_preempt_check+0x13/0x20
> [  321.266406]  ? trace_hardirqs_off_caller+0x1d6/0x2c0
> 
> [  321.266487] Allocated by task 2868:
> [  321.266568]  save_stack_trace+0x16/0x20
> [  321.266586]  kasan_kmalloc+0xee/0x180
> [  321.266602]  kasan_slab_alloc+0x12/0x20
> [  321.266620]  kmem_cache_alloc+0xc7/0x2e0
> [  321.266795]  i915_vma_instance+0x28c/0x1540 [i915]
> [  321.266964]  eb_lookup_vmas+0x5a7/0x2250 [i915]
> [  321.267130]  i915_gem_do_execbuffer+0x69a/0x2a20 [i915]
> [  321.267296]  i915_gem_execbuffer2+0x14a/0x3f0 [i915]
> [  321.267315]  drm_ioctl+0x4ba/0xaa0
> [  321.267333]  do_vfs_ioctl+0x17f/0xfa0
> [  321.267350]  SyS_ioctl+0x3c/0x70
> [  321.267369]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> 
> [  321.267428] Freed by task 177:
> [  321.267502]  save_stack_trace+0x16/0x20
> [  321.267521]  kasan_slab_free+0xad/0x180
> [  321.267539]  kmem_cache_free+0xc5/0x340
> [  321.267710]  i915_vma_unbind+0x666/0x10a0 [i915]
> [  321.267880]  i915_vma_close+0x23a/0x2f0 [i915]
> [  321.268048]  __i915_gem_free_objects+0x17d/0xc70 [i915]
> [  321.268215]  __i915_gem_free_work+0x49/0x70 [i915]
> [  321.268234]  process_one_work+0x66f/0x1410
> [  321.268252]  worker_thread+0xe1/0xe90
> [  321.268269]  kthread+0x304/0x410
> [  321.268285]  ret_from_fork+0x27/0x40
> 
> [  321.268346] The buggy address belongs to the object at ffff880100fc6640
>                  which belongs to the cache i915_vma of size 656
> [  321.268550] The buggy address is located 408 bytes inside of
>                  656-byte region [ffff880100fc6640, ffff880100fc68d0)
> [  321.268741] The buggy address belongs to the page:
> [  321.268837] page:ffffea000403f000 count:1 mapcount:0 mapping:          (null) index:0xffff880100fc5980 compound_mapcount: 0
> [  321.269045] flags: 0x8000000000008100(slab|head)
> [  321.269147] raw: 8000000000008100 0000000000000000 ffff880100fc5980 00000001001e001d
> [  321.269312] raw: ffffea0004038e20 ffff880116b46240 ffff88011646c640 0000000000000000
> [  321.269484] page dumped because: kasan: bad access detected
> 
> [  321.269665] Memory state around the buggy address:
> [  321.269778]  ffff880100fc6680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [  321.269949]  ffff880100fc6700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [  321.270115] >ffff880100fc6780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [  321.270279]                                                     ^
> [  321.270410]  ffff880100fc6800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [  321.270576]  ffff880100fc6880: fb fb fb fb fb fb fb fb fb fb fc fc fc fc fc fc
> [  321.270740] ==================================================================
> [  321.270903] Disabling lock debugging due to kernel taint
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101511
> Fixes: 7dd4f6729f92 ("drm/i915: Async GPU relocation processing")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index eb46dfa374a7..a23a1c3503c8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1199,7 +1199,7 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
>   	reservation_object_unlock(batch->resv);
>   	i915_vma_unpin(batch);
>   
> -	i915_vma_move_to_active(vma, rq, true);
> +	i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
>   	reservation_object_lock(vma->resv, NULL);
>   	reservation_object_add_excl_fence(vma->resv, &rq->fence);
>   	reservation_object_unlock(vma->resv);
> 

This one is straightforward.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH 3/3] drm/i915: Assert the vma's active tracking is clear before free

[Tvrtko Ursulin]

On 20/06/2017 13:43, Chris Wilson wrote:
> In looking at a use-after-free on Baytrail, it looks like the VMA's
> activity tracking is suspect. Add some asserts to catch freeing the VMA
> before we have decoupled all of its i915_gem_active trackers.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=101511
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> c: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_vma.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 1cfe137cdc32..958be0a95960 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -579,11 +579,17 @@ int __i915_vma_do_pin(struct i915_vma *vma,
>   
>   static void i915_vma_destroy(struct i915_vma *vma)
>   {
> +	int i;
> +
>   	GEM_BUG_ON(vma->node.allocated);
>   	GEM_BUG_ON(i915_vma_is_active(vma));
>   	GEM_BUG_ON(!i915_vma_is_closed(vma));
>   	GEM_BUG_ON(vma->fence);
>   
> +	for (i = 0; i < ARRAY_SIZE(vma->last_read); i++)
> +		GEM_BUG_ON(i915_gem_active_isset(&vma->last_read[i]));
> +	GEM_BUG_ON(i915_gem_active_isset(&vma->last_fence));
> +
>   	list_del(&vma->vm_link);
>   	if (!i915_vma_is_ggtt(vma))
>   		i915_ppgtt_put(i915_vm_to_ppgtt(vma->vm));
> @@ -680,9 +686,8 @@ int i915_vma_unbind(struct i915_vma *vma)
>   		__i915_vma_unpin(vma);
>   		if (ret)
>   			return ret;
> -
> -		GEM_BUG_ON(i915_vma_is_active(vma));
>   	}
> +	GEM_BUG_ON(i915_vma_is_active(vma));
>   
>   	if (i915_vma_is_pinned(vma))
>   		return -EBUSY;
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [PATCH 1/3] drm/i915: Retire the VMA's fence tracker before unbinding

[Tvrtko Ursulin]

On 20/06/2017 13:43, Chris Wilson wrote:
> Since we may track unfenced access (GPU access to the vma that
> explicitly requires no fence), vma->last_fence may be set without any

Is this the gen < 4 code path? There is no actual fence in this case?

> attached fence (vma->fence) and so will not be flushed when we call
> i915_vma_put_fence(). Since we stopped doing a full retire of the
> activity trackers for unbind, we need to explicitly retire each tracker.
> 
> Fixes: b0decaf75bd9 ("drm/i915: Track active vma requests")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_vma.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 532c709febbd..1cfe137cdc32 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -672,6 +672,11 @@ int i915_vma_unbind(struct i915_vma *vma)
>   				break;
>   		}
>   
> +		if (!ret) {
> +			ret = i915_gem_active_retire(&vma->last_fence,
> +						     &vma->vm->i915->drm.struct_mutex);
> +		}
> +
>   		__i915_vma_unpin(vma);
>   		if (ret)
>   			return ret;
> 

Looks safe anyway, but I'd like to understand how exactly it happens and 
if it is still possible after patch 2/3.

Regards,

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

Re: [PATCH 1/3] drm/i915: Retire the VMA's fence tracker before unbinding

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-06-20 16:55:12)
> 
> On 20/06/2017 13:43, Chris Wilson wrote:
> > Since we may track unfenced access (GPU access to the vma that
> > explicitly requires no fence), vma->last_fence may be set without any
> 
> Is this the gen < 4 code path? There is no actual fence in this case?

Yes, this is for old hw that used a fence for GPU access, and in this
case indeed there is no fence and we are tracking the access for when
the fence is not wanted to ensure that don't install a fence before the
GPU finished accessing the region.

> > attached fence (vma->fence) and so will not be flushed when we call
> > i915_vma_put_fence(). Since we stopped doing a full retire of the
> > activity trackers for unbind, we need to explicitly retire each tracker.
> > 
> > Fixes: b0decaf75bd9 ("drm/i915: Track active vma requests")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_vma.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> > index 532c709febbd..1cfe137cdc32 100644
> > --- a/drivers/gpu/drm/i915/i915_vma.c
> > +++ b/drivers/gpu/drm/i915/i915_vma.c
> > @@ -672,6 +672,11 @@ int i915_vma_unbind(struct i915_vma *vma)
> >                               break;
> >               }
> >   
> > +             if (!ret) {
> > +                     ret = i915_gem_active_retire(&vma->last_fence,
> > +                                                  &vma->vm->i915->drm.struct_mutex);
> > +             }
> > +
> >               __i915_vma_unpin(vma);
> >               if (ret)
> >                       return ret;
> > 
> 
> Looks safe anyway, but I'd like to understand how exactly it happens and 
> if it is still possible after patch 2/3.

For gen2/3, we still use last_fence. For others it was accidental and
should be no more after patch 2. I considered add a GEM_BUG_ON to
i915_move_to_active but didn't have a convenient i915 pointer, and I
have grander plans :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Pass the right flags to i915_vma_move_to_active()

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-06-20 16:47:06)
> 
> On 20/06/2017 13:43, Chris Wilson wrote:
> > i915_vma_move_to_active() takes the execobject flags and not a boolean!
> > Instead of passing EXEC_OBJECT_WRITE we passed true [i.e.
> > EXEC_OBJECT_NEEDS_FENCE] causing us to start tracking the
> > vma->last_fence access and since we forgot to clear that on unbinding,
> > we caused a use-after-free.
> > 
> > [  321.263854] BUG: KASAN: use-after-free in i915_gem_request_retire+0x1728/0x1740 [i915]
> > [  321.264001] Read of size 8 at addr ffff880100fc67d8 by task gem_exec_reloc/2868
> > 
> > [  321.264181] CPU: 0 PID: 2868 Comm: gem_exec_reloc Not tainted 4.12.0-rc6-CI-Custom_2759+ #1
> > [  321.264195] Hardware name: GIGABYTE GB-BXBT-1900/MZBAYAB-00, BIOS F6 02/17/2015
> > [  321.264208] Call Trace:
> > [  321.264234]  dump_stack+0x67/0x99
> > [  321.264260]  print_address_description+0x77/0x290
> > [  321.264437]  ? i915_gem_request_retire+0x1728/0x1740 [i915]
> > [  321.264459]  kasan_report+0x269/0x350
> > [  321.264487]  __asan_report_load8_noabort+0x14/0x20
> > [  321.264660]  i915_gem_request_retire+0x1728/0x1740 [i915]
> > [  321.264841]  ? intel_ring_context_pin+0x131/0x690 [i915]
> > [  321.265021]  i915_gem_request_alloc+0x2c6/0x1220 [i915]
> > [  321.265044]  ? _raw_spin_unlock_irqrestore+0x3d/0x60
> > [  321.265226]  i915_gem_do_execbuffer+0xac0/0x2a20 [i915]
> > [  321.265250]  ? __lock_acquire+0xceb/0x5450
> > [  321.265269]  ? entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [  321.265291]  ? kvmalloc_node+0x6b/0x80
> > [  321.265310]  ? kvmalloc_node+0x6b/0x80
> > [  321.265489]  ? eb_relocate_slow+0xbe0/0xbe0 [i915]
> > [  321.265520]  ? ___slab_alloc.constprop.28+0x2ab/0x3d0
> > [  321.265549]  ? debug_check_no_locks_freed+0x280/0x280
> > [  321.265591]  ? __might_fault+0xc6/0x1b0
> > [  321.265782]  i915_gem_execbuffer2+0x14a/0x3f0 [i915]
> > [  321.265815]  drm_ioctl+0x4ba/0xaa0
> > [  321.265986]  ? i915_gem_execbuffer+0xde0/0xde0 [i915]
> > [  321.266017]  ? drm_getunique+0x270/0x270
> > [  321.266068]  do_vfs_ioctl+0x17f/0xfa0
> > [  321.266091]  ? __fget+0x1ba/0x330
> > [  321.266112]  ? lock_acquire+0x390/0x390
> > [  321.266133]  ? ioctl_preallocate+0x1d0/0x1d0
> > [  321.266164]  ? __fget+0x1db/0x330
> > [  321.266194]  ? __fget_light+0x79/0x1f0
> > [  321.266219]  SyS_ioctl+0x3c/0x70
> > [  321.266247]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > [  321.266265] RIP: 0033:0x7fcede207357
> > [  321.266279] RSP: 002b:00007ffef0effe58 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > [  321.266307] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007fcede207357
> > [  321.266321] RDX: 00007ffef0effef0 RSI: 0000000040406469 RDI: 0000000000000004
> > [  321.266335] RBP: ffffffff812097c6 R08: 0000000000000008 R09: 0000000000000000
> > [  321.266349] R10: 0000000000000008 R11: 0000000000000246 R12: ffff880116bcff98
> > [  321.266363] R13: ffffffff81cb7cb3 R14: ffff880116bcff70 R15: 0000000000000000
> > [  321.266385]  ? __this_cpu_preempt_check+0x13/0x20
> > [  321.266406]  ? trace_hardirqs_off_caller+0x1d6/0x2c0
> > 
> > [  321.266487] Allocated by task 2868:
> > [  321.266568]  save_stack_trace+0x16/0x20
> > [  321.266586]  kasan_kmalloc+0xee/0x180
> > [  321.266602]  kasan_slab_alloc+0x12/0x20
> > [  321.266620]  kmem_cache_alloc+0xc7/0x2e0
> > [  321.266795]  i915_vma_instance+0x28c/0x1540 [i915]
> > [  321.266964]  eb_lookup_vmas+0x5a7/0x2250 [i915]
> > [  321.267130]  i915_gem_do_execbuffer+0x69a/0x2a20 [i915]
> > [  321.267296]  i915_gem_execbuffer2+0x14a/0x3f0 [i915]
> > [  321.267315]  drm_ioctl+0x4ba/0xaa0
> > [  321.267333]  do_vfs_ioctl+0x17f/0xfa0
> > [  321.267350]  SyS_ioctl+0x3c/0x70
> > [  321.267369]  entry_SYSCALL_64_fastpath+0x1c/0xb1
> > 
> > [  321.267428] Freed by task 177:
> > [  321.267502]  save_stack_trace+0x16/0x20
> > [  321.267521]  kasan_slab_free+0xad/0x180
> > [  321.267539]  kmem_cache_free+0xc5/0x340
> > [  321.267710]  i915_vma_unbind+0x666/0x10a0 [i915]
> > [  321.267880]  i915_vma_close+0x23a/0x2f0 [i915]
> > [  321.268048]  __i915_gem_free_objects+0x17d/0xc70 [i915]
> > [  321.268215]  __i915_gem_free_work+0x49/0x70 [i915]
> > [  321.268234]  process_one_work+0x66f/0x1410
> > [  321.268252]  worker_thread+0xe1/0xe90
> > [  321.268269]  kthread+0x304/0x410
> > [  321.268285]  ret_from_fork+0x27/0x40
> > 
> > [  321.268346] The buggy address belongs to the object at ffff880100fc6640
> >                  which belongs to the cache i915_vma of size 656
> > [  321.268550] The buggy address is located 408 bytes inside of
> >                  656-byte region [ffff880100fc6640, ffff880100fc68d0)
> > [  321.268741] The buggy address belongs to the page:
> > [  321.268837] page:ffffea000403f000 count:1 mapcount:0 mapping:          (null) index:0xffff880100fc5980 compound_mapcount: 0
> > [  321.269045] flags: 0x8000000000008100(slab|head)
> > [  321.269147] raw: 8000000000008100 0000000000000000 ffff880100fc5980 00000001001e001d
> > [  321.269312] raw: ffffea0004038e20 ffff880116b46240 ffff88011646c640 0000000000000000
> > [  321.269484] page dumped because: kasan: bad access detected
> > 
> > [  321.269665] Memory state around the buggy address:
> > [  321.269778]  ffff880100fc6680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [  321.269949]  ffff880100fc6700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [  321.270115] >ffff880100fc6780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [  321.270279]                                                     ^
> > [  321.270410]  ffff880100fc6800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [  321.270576]  ffff880100fc6880: fb fb fb fb fb fb fb fb fb fb fc fc fc fc fc fc
> > [  321.270740] ==================================================================
> > [  321.270903] Disabling lock debugging due to kernel taint
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101511
> > Fixes: 7dd4f6729f92 ("drm/i915: Async GPU relocation processing")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index eb46dfa374a7..a23a1c3503c8 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -1199,7 +1199,7 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
> >       reservation_object_unlock(batch->resv);
> >       i915_vma_unpin(batch);
> >   
> > -     i915_vma_move_to_active(vma, rq, true);
> > +     i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
> >       reservation_object_lock(vma->resv, NULL);
> >       reservation_object_add_excl_fence(vma->resv, &rq->fence);
> >       reservation_object_unlock(vma->resv);
> > 
> 
> This one is straightforward.
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks for the review, pushed the fix for the brainfart. Will come back
to the older issue in the morning if I've managed to clarify the logic.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Retire the VMA's fence tracker before unbinding

[Tvrtko Ursulin]

On 20/06/2017 17:05, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-06-20 16:55:12)
>>
>> On 20/06/2017 13:43, Chris Wilson wrote:
>>> Since we may track unfenced access (GPU access to the vma that
>>> explicitly requires no fence), vma->last_fence may be set without any
>>
>> Is this the gen < 4 code path? There is no actual fence in this case?
> 
> Yes, this is for old hw that used a fence for GPU access, and in this
> case indeed there is no fence and we are tracking the access for when
> the fence is not wanted to ensure that don't install a fence before the
> GPU finished accessing the region.
> 
>>> attached fence (vma->fence) and so will not be flushed when we call
>>> i915_vma_put_fence(). Since we stopped doing a full retire of the
>>> activity trackers for unbind, we need to explicitly retire each tracker.
>>>
>>> Fixes: b0decaf75bd9 ("drm/i915: Track active vma requests")
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_vma.c | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
>>> index 532c709febbd..1cfe137cdc32 100644
>>> --- a/drivers/gpu/drm/i915/i915_vma.c
>>> +++ b/drivers/gpu/drm/i915/i915_vma.c
>>> @@ -672,6 +672,11 @@ int i915_vma_unbind(struct i915_vma *vma)
>>>                                break;
>>>                }
>>>    
>>> +             if (!ret) {
>>> +                     ret = i915_gem_active_retire(&vma->last_fence,
>>> +                                                  &vma->vm->i915->drm.struct_mutex);
>>> +             }
>>> +
>>>                __i915_vma_unpin(vma);
>>>                if (ret)
>>>                        return ret;
>>>
>>
>> Looks safe anyway, but I'd like to understand how exactly it happens and
>> if it is still possible after patch 2/3.
> 
> For gen2/3, we still use last_fence. For others it was accidental and
> should be no more after patch 2. I considered add a GEM_BUG_ON to
> i915_move_to_active but didn't have a convenient i915 pointer, and I
> have grander plans :)

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

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

Re: [PATCH 1/3] drm/i915: Retire the VMA's fence tracker before unbinding

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-06-21 10:17:49)
> 
> On 20/06/2017 17:05, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2017-06-20 16:55:12)
> >>
> >> On 20/06/2017 13:43, Chris Wilson wrote:
> >>> Since we may track unfenced access (GPU access to the vma that
> >>> explicitly requires no fence), vma->last_fence may be set without any
> >>
> >> Is this the gen < 4 code path? There is no actual fence in this case?
> > 
> > Yes, this is for old hw that used a fence for GPU access, and in this
> > case indeed there is no fence and we are tracking the access for when
> > the fence is not wanted to ensure that don't install a fence before the
> > GPU finished accessing the region.
> > 
> >>> attached fence (vma->fence) and so will not be flushed when we call
> >>> i915_vma_put_fence(). Since we stopped doing a full retire of the
> >>> activity trackers for unbind, we need to explicitly retire each tracker.
> >>>
> >>> Fixes: b0decaf75bd9 ("drm/i915: Track active vma requests")
> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> >>> ---
> >>>    drivers/gpu/drm/i915/i915_vma.c | 5 +++++
> >>>    1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> >>> index 532c709febbd..1cfe137cdc32 100644
> >>> --- a/drivers/gpu/drm/i915/i915_vma.c
> >>> +++ b/drivers/gpu/drm/i915/i915_vma.c
> >>> @@ -672,6 +672,11 @@ int i915_vma_unbind(struct i915_vma *vma)
> >>>                                break;
> >>>                }
> >>>    
> >>> +             if (!ret) {
> >>> +                     ret = i915_gem_active_retire(&vma->last_fence,
> >>> +                                                  &vma->vm->i915->drm.struct_mutex);
> >>> +             }
> >>> +
> >>>                __i915_vma_unpin(vma);
> >>>                if (ret)
> >>>                        return ret;
> >>>
> >>
> >> Looks safe anyway, but I'd like to understand how exactly it happens and
> >> if it is still possible after patch 2/3.
> > 
> > For gen2/3, we still use last_fence. For others it was accidental and
> > should be no more after patch 2. I considered add a GEM_BUG_ON to
> > i915_move_to_active but didn't have a convenient i915 pointer, and I
> > have grander plans :)
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Thanks for accepting my poor grammar :)

Pushed the remaining pair,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx