[Intel-gfx] [PATCH] drm/i915: Fix NULL plane->fb oops on SKL

Tue Jan 19 12:28:03 PST 2016

On Tue, Jan 19, 2016 at 06:23:17PM +0200, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> In this atomic age, we can't trust the plane->fb pointer anymore.
> It might get update too late. Instead we are supposed to use the
> plane_state->fb pointer instead. Let's do that in
> intel_plane_obj_offset() and avoid problems from dereferencing the
> potentially stale plane->fb pointer.
> 
> Paulo found this with 'kms_frontbuffer_tracking --show-hidden --run-subtest nop-1p-rte'
> but it can be reproduced with just plain old kms_setplane.
> 
> I was too lazy to bisect this, so not sure exactly when it broke. The
> most obvious candidate
> commit ce7f17285639 ("drm/i915: Fix i915_ggtt_view_equal to handle rotation correctly")
> was actually still fine, so it must have broken some time after that.
> 
> Here's the resulting fireworks:
> BUG: unable to handle kernel NULL pointer dereference at           (null)
> IP: [<ffffffffa02d2d9a>] intel_fill_fb_ggtt_view+0x1b/0x15a [i915]
> PGD 8a5f6067 PUD 8a5f5067 PMD 0
> Oops: 0000 [#1] PREEMPT SMP
> Modules linked in: i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm intel_gtt agpgart netconsole mousedev hid_generic psmouse usbhid atkbd libps2 coretemp hwmon efi_pstore intel_rapl iosf_mbi x86_pkg_temp_thermal efivars pcspkr e1000e sdhci_pci ptp pps_core sdhci i2c_i801 mmc_core i2c_hid hid i8042 serio evdev sch_fq_codel ip_tables x_tables ipv6 autofs4
> CPU: 1 PID: 260 Comm: kms_plane Not tainted 4.4.0-skl+ #171
> Hardware name: Intel Corporation Skylake Client platform/Skylake Y LPDDR3 RVP3, BIOS SKLSE2R1.R00.B104.B00.1511030553 11/03/2015
> task: ffff88008bde2d80 ti: ffff88008a6ec000 task.ti: ffff88008a6ec000
> RIP: 0010:[<ffffffffa02d2d9a>]  [<ffffffffa02d2d9a>] intel_fill_fb_ggtt_view+0x1b/0x15a [i915]
> RSP: 0018:ffff88008a6efa10  EFLAGS: 00010086
> RAX: 0000000000000001 RBX: ffff8801674f4240 RCX: 0000000000000014
> RDX: ffff88008a7440c0 RSI: 0000000000000000 RDI: ffff88008a6efa40
> RBP: ffff88008a6efa30 R08: ffff88008bde3598 R09: 0000000000000001
> R10: ffff88008b782000 R11: 0000000000000000 R12: 0000000000000000
> R13: ffff88008a7440c0 R14: 0000000000000000 R15: ffff88008a7449c0
> FS:  00007fa0c07a28c0(0000) GS:ffff88016ec40000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000000 CR3: 000000008a6ff000 CR4: 00000000003406e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Stack:
>  ffff8801674f4240 0000000000000000 ffff88008a7440c0 0000000000000000
>  ffff88008a6efaa0 ffffffffa02daf25 ffffffff814ec80e 0000000000070298
>  ffff8800850d0000 ffff88008a6efaa0 ffffffffa02c49c2 0000000000000002
> Call Trace:
>  [<ffffffffa02daf25>] intel_plane_obj_offset+0x2d/0xa9 [i915]
>  [<ffffffff814ec80e>] ? _raw_spin_unlock_irqrestore+0x4b/0x60
>  [<ffffffffa02c49c2>] ? gen9_write32+0x2e8/0x3b8 [i915]
>  [<ffffffffa02eecfc>] skl_update_plane+0x203/0x4c5 [i915]
>  [<ffffffffa02ca1ab>] intel_plane_atomic_update+0x53/0x6a [i915]
>  [<ffffffffa02494a4>] drm_atomic_helper_commit_planes_on_crtc+0x142/0x1d5 [drm_kms_helper]
>  [<ffffffffa02de44b>] intel_atomic_commit+0x1262/0x1350 [i915]
>  [<ffffffffa024a0ee>] ? __drm_atomic_helper_crtc_duplicate_state+0x2f/0x41 [drm_kms_helper]
>  [<ffffffffa01ef089>] ? drm_atomic_check_only+0x3e3/0x552 [drm]
>  [<ffffffffa01ef245>] drm_atomic_commit+0x4d/0x52 [drm]
>  [<ffffffffa024996b>] drm_atomic_helper_update_plane+0xcb/0x118 [drm_kms_helper]
>  [<ffffffffa01e42e8>] __setplane_internal+0x1c8/0x224 [drm]
>  [<ffffffffa01e477f>] drm_mode_setplane+0x14e/0x172 [drm]
>  [<ffffffffa01d8117>] drm_ioctl+0x265/0x3ad [drm]
>  [<ffffffffa01e4631>] ? drm_mode_cursor_common+0x158/0x158 [drm]
>  [<ffffffff810d00ab>] ? current_kernel_time64+0x5e/0x98
>  [<ffffffff810a76ea>] ? trace_hardirqs_on_caller+0x17a/0x196
>  [<ffffffff8119880f>] do_vfs_ioctl+0x42b/0x4ea
>  [<ffffffff811a2b72>] ? __fget_light+0x4d/0x71
>  [<ffffffff81198911>] SyS_ioctl+0x43/0x61
>  [<ffffffff814ed057>] entry_SYSCALL_64_fastpath+0x12/0x6f
> 
> Cc: drm-intel-fixes at lists.freedesktop.org
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Testcase: igt/kms_plane
> Reported-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Surprised this even works, since iirc we've changed all the core code to
post-updating plane->fb. So it's probably some shuffling in
intel_display.c that broke things. Or maybe just utter reliance on
fastboot working ;-)

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a851cb70479e..5bb960826cd1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2940,7 +2940,7 @@ u32 intel_plane_obj_offset(struct intel_plane *intel_plane,
>  	struct i915_vma *vma;
>  	u64 offset;
>  
> -	intel_fill_fb_ggtt_view(&view, intel_plane->base.fb,
> +	intel_fill_fb_ggtt_view(&view, intel_plane->base.state->fb,
>  				intel_plane->base.state);
>  
>  	vma = i915_gem_obj_to_ggtt_view(obj, &view);
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch