[Intel-gfx] [PATCH] drm/i915/display: Defer initial modeset until after GGTT is initialised

[Chris Wilson <chris@chris-wilson.co.uk>]

Prior to sanitizing the GGTT, the only operations around in
intel_display_init_nogem() are those to reserve the preallocated (and
active) regions in the GGTT leftover from the BIOS. Trying to allocate a
GGTT vma (such as intel_pin_and_fence_fb_obj during the initial modeset)
may then conflict with other preallocated regions that have not yet been
protected.

Move the initial modesetting from the end of init_nogem to the beginning
of init so that any vma pinning (either framebuffers or DSB, for example),
is after the GGTT is ready to handle it.

This will prevent the DSB object from being destroyed too early:

[   53.448973] ==================================================================
[   53.449241] BUG: KASAN: use-after-free in i915_init_ggtt+0x324/0x9e0 [i915]
[   53.449309] Read of size 8 at addr ffff88811b1e8070 by task systemd-udevd/345

[   53.449399] CPU: 1 PID: 345 Comm: systemd-udevd Tainted: G        W         5.10.0-rc5+ #12
[   53.449409] Call Trace:
[   53.449418]  dump_stack+0x9a/0xcc
[   53.449558]  ? i915_init_ggtt+0x324/0x9e0 [i915]
[   53.449565]  print_address_description.constprop.0+0x3e/0x60
[   53.449577]  ? _raw_spin_lock_irqsave+0x4e/0x50
[   53.449718]  ? i915_init_ggtt+0x324/0x9e0 [i915]
[   53.449849]  ? i915_init_ggtt+0x324/0x9e0 [i915]
[   53.449857]  kasan_report.cold+0x1f/0x37
[   53.449993]  ? i915_init_ggtt+0x324/0x9e0 [i915]
[   53.450130]  i915_init_ggtt+0x324/0x9e0 [i915]
[   53.450273]  ? i915_ggtt_suspend+0x1f0/0x1f0 [i915]
[   53.450281]  ? static_obj+0x69/0x80
[   53.450289]  ? lockdep_init_map_waits+0xa9/0x310
[   53.450431]  ? intel_wopcm_init+0x96/0x3d0 [i915]
[   53.450581]  ? i915_gem_init+0x75/0x2d0 [i915]
[   53.450720]  i915_gem_init+0x75/0x2d0 [i915]
[   53.450852]  i915_driver_probe+0x8c2/0x1210 [i915]
[   53.450993]  ? i915_pm_prepare+0x630/0x630 [i915]
[   53.451006]  ? check_chain_key+0x1e7/0x2e0
[   53.451025]  ? __pm_runtime_resume+0x58/0xb0
[   53.451157]  i915_pci_probe+0xa6/0x2b0 [i915]
[   53.451285]  ? i915_pci_remove+0x40/0x40 [i915]
[   53.451295]  ? lockdep_hardirqs_on_prepare+0x124/0x230
[   53.451302]  ? _raw_spin_unlock_irqrestore+0x42/0x50
[   53.451309]  ? lockdep_hardirqs_on+0xbf/0x130
[   53.451315]  ? preempt_count_sub+0xf/0xb0
[   53.451321]  ? _raw_spin_unlock_irqrestore+0x2f/0x50
[   53.451335]  pci_device_probe+0xf9/0x190
[   53.451350]  really_probe+0x17f/0x5b0
[   53.451365]  driver_probe_device+0x13a/0x1c0
[   53.451376]  device_driver_attach+0x82/0x90
[   53.451386]  ? device_driver_attach+0x90/0x90
[   53.451391]  __driver_attach+0xab/0x190
[   53.451401]  ? device_driver_attach+0x90/0x90
[   53.451407]  bus_for_each_dev+0xe4/0x140
[   53.451414]  ? subsys_dev_iter_exit+0x10/0x10
[   53.451423]  ? __list_add_valid+0x2b/0xa0
[   53.451440]  bus_add_driver+0x227/0x2e0
[   53.451454]  driver_register+0xd3/0x150
[   53.451585]  i915_init+0x92/0xac [i915]
[   53.451592]  ? 0xffffffffa0a20000
[   53.451598]  do_one_initcall+0xb6/0x3b0
[   53.451606]  ? trace_event_raw_event_initcall_finish+0x150/0x150
[   53.451614]  ? __kasan_kmalloc.constprop.0+0xc2/0xd0
[   53.451627]  ? kmem_cache_alloc_trace+0x4a4/0x8e0
[   53.451634]  ? kasan_unpoison_shadow+0x33/0x40
[   53.451649]  do_init_module+0xf8/0x350
[   53.451662]  load_module+0x43de/0x47f0
[   53.451716]  ? module_frob_arch_sections+0x20/0x20
[   53.451731]  ? rw_verify_area+0x5f/0x130
[   53.451780]  ? __do_sys_finit_module+0x10d/0x1a0
[   53.451785]  __do_sys_finit_module+0x10d/0x1a0
[   53.451792]  ? __ia32_sys_init_module+0x40/0x40
[   53.451800]  ? seccomp_do_user_notification.isra.0+0x5c0/0x5c0
[   53.451829]  ? rcu_read_lock_bh_held+0xb0/0xb0
[   53.451835]  ? mark_held_locks+0x24/0x90
[   53.451856]  do_syscall_64+0x33/0x80
[   53.451863]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   53.451868] RIP: 0033:0x7fde09b4470d
[   53.451875] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 53 f7 0c 00 f7 d8 64 89 01 48
[   53.451880] RSP: 002b:00007ffd6abc1718 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[   53.451890] RAX: ffffffffffffffda RBX: 000056444e528150 RCX: 00007fde09b4470d
[   53.451895] RDX: 0000000000000000 RSI: 00007fde09a21ded RDI: 000000000000000f
[   53.451899] RBP: 0000000000020000 R08: 0000000000000000 R09: 0000000000000000
[   53.451904] R10: 000000000000000f R11: 0000000000000246 R12: 00007fde09a21ded
[   53.451909] R13: 0000000000000000 R14: 000056444e329200 R15: 000056444e528150

[   53.451957] Allocated by task 345:
[   53.451995]  kasan_save_stack+0x1b/0x40
[   53.452001]  __kasan_kmalloc.constprop.0+0xc2/0xd0
[   53.452006]  kmem_cache_alloc+0x1cd/0x8d0
[   53.452146]  i915_vma_instance+0x126/0xb70 [i915]
[   53.452304]  i915_gem_object_ggtt_pin_ww+0x222/0x3f0 [i915]
[   53.452446]  intel_dsb_prepare+0x14f/0x230 [i915]
[   53.452588]  intel_atomic_commit+0x183/0x690 [i915]
[   53.452730]  intel_initial_commit+0x2bc/0x2f0 [i915]
[   53.452871]  intel_modeset_init_nogem+0xa02/0x2af0 [i915]
[   53.452995]  i915_driver_probe+0x8af/0x1210 [i915]
[   53.453120]  i915_pci_probe+0xa6/0x2b0 [i915]
[   53.453125]  pci_device_probe+0xf9/0x190
[   53.453131]  really_probe+0x17f/0x5b0
[   53.453136]  driver_probe_device+0x13a/0x1c0
[   53.453142]  device_driver_attach+0x82/0x90
[   53.453148]  __driver_attach+0xab/0x190
[   53.453153]  bus_for_each_dev+0xe4/0x140
[   53.453158]  bus_add_driver+0x227/0x2e0
[   53.453164]  driver_register+0xd3/0x150
[   53.453286]  i915_init+0x92/0xac [i915]
[   53.453292]  do_one_initcall+0xb6/0x3b0
[   53.453297]  do_init_module+0xf8/0x350
[   53.453302]  load_module+0x43de/0x47f0
[   53.453307]  __do_sys_finit_module+0x10d/0x1a0
[   53.453312]  do_syscall_64+0x33/0x80
[   53.453318]  entry_SYSCALL_64_after_hwframe+0x44/0xa9

[   53.453345] Freed by task 82:
[   53.453379]  kasan_save_stack+0x1b/0x40
[   53.453384]  kasan_set_track+0x1c/0x30
[   53.453389]  kasan_set_free_info+0x1b/0x30
[   53.453394]  __kasan_slab_free+0x112/0x160
[   53.453399]  kmem_cache_free+0xb2/0x3f0
[   53.453536]  i915_gem_flush_free_objects+0x31a/0x3b0 [i915]
[   53.453542]  process_one_work+0x519/0x9f0
[   53.453547]  worker_thread+0x75/0x5c0
[   53.453552]  kthread+0x1da/0x230
[   53.453557]  ret_from_fork+0x22/0x30

[   53.453584] The buggy address belongs to the object at ffff88811b1e8040
                which belongs to the cache i915_vma of size 968
[   53.453692] The buggy address is located 48 bytes inside of
                968-byte region [ffff88811b1e8040, ffff88811b1e8408)
[   53.453792] The buggy address belongs to the page:
[   53.453842] page:00000000b35f7048 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff88811b1ef940 pfn:0x11b1e8
[   53.453847] head:00000000b35f7048 order:3 compound_mapcount:0 compound_pincount:0
[   53.453853] flags: 0x8000000000010200(slab|head)
[   53.453860] raw: 8000000000010200 ffff888115596248 ffff888115596248 ffff8881155b6340
[   53.453866] raw: ffff88811b1ef940 0000000000170001 00000001ffffffff 0000000000000000
[   53.453870] page dumped because: kasan: bad access detected

[   53.453895] Memory state around the buggy address:
[   53.453944]  ffff88811b1e7f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[   53.454011]  ffff88811b1e7f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[   53.454079] >ffff88811b1e8000: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb
[   53.454146]                                                              ^
[   53.454211]  ffff88811b1e8080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[   53.454279]  ffff88811b1e8100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[   53.454347] ==================================================================
[   53.454414] Disabling lock debugging due to kernel taint
[   53.454434] general protection fault, probably for non-canonical address 0xdead0000000000d0: 0000 [#1] PREEMPT SMP KASAN PTI
[   53.454446] CPU: 1 PID: 345 Comm: systemd-udevd Tainted: G    B   W         5.10.0-rc5+ #12
[   53.454592] RIP: 0010:i915_init_ggtt+0x26f/0x9e0 [i915]
[   53.454602] Code: 89 8d 48 ff ff ff 4c 8d 60 d0 49 39 c7 0f 84 37 02 00 00 4c 89 b5 40 ff ff ff 4d 8d bc 24 90 00 00 00 4c 89 ff e8 c1 97 f8 e0 <49> 83 bc 24 90 00 00 00 00 0f 84 0f 02 00 00 49 8d 7c 24 08 e8 a8
[   53.454618] RSP: 0018:ffff88812247f430 EFLAGS: 00010286
[   53.454625] RAX: 0000000000000000 RBX: ffff888136440000 RCX: ffffffffa03fb78f
[   53.454633] RDX: 0000000000000000 RSI: 0000000000000008 RDI: dead000000000160
[   53.454641] RBP: ffff88812247f500 R08: ffffffff8113589f R09: 0000000000000000
[   53.454648] R10: ffffffff83063843 R11: fffffbfff060c708 R12: dead0000000000d0
[   53.454656] R13: ffff888136449ba0 R14: 0000000000002000 R15: dead000000000160
[   53.454664] FS:  00007fde095c4880(0000) GS:ffff88840c880000(0000) knlGS:0000000000000000
[   53.454672] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   53.454679] CR2: 00007fef132b4f28 CR3: 000000012245c002 CR4: 00000000003706e0
[   53.454686] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   53.454693] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   53.454700] Call Trace:
[   53.454833]  ? i915_ggtt_suspend+0x1f0/0x1f0 [i915]

Fixes: afeda4f3b1c8 ("drm/i915/dsb: Pre allocate and late cleanup of cmd buffer")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 595183f7b60f..ba26545392bc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -18729,16 +18729,6 @@ int intel_modeset_init_nogem(struct drm_i915_private *i915)
 	if (!HAS_GMCH(i915))
 		sanitize_watermarks(i915);
 
-	/*
-	 * Force all active planes to recompute their states. So that on
-	 * mode_setcrtc after probe, all the intel_plane_state variables
-	 * are already calculated and there is no assert_plane warnings
-	 * during bootup.
-	 */
-	ret = intel_initial_commit(dev);
-	if (ret)
-		drm_dbg_kms(&i915->drm, "Initial commit in probe failed.\n");
-
 	return 0;
 }
 
@@ -18750,6 +18740,16 @@ int intel_modeset_init(struct drm_i915_private *i915)
 	if (!HAS_DISPLAY(i915))
 		return 0;
 
+	/*
+	 * Force all active planes to recompute their states. So that on
+	 * mode_setcrtc after probe, all the intel_plane_state variables
+	 * are already calculated and there is no assert_plane warnings
+	 * during bootup.
+	 */
+	ret = intel_initial_commit(&i915->drm);
+	if (ret)
+		return ret;
+
 	intel_overlay_setup(i915);
 
 	ret = intel_fbdev_init(&i915->drm);
-- 
2.20.1

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