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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 19/01/16 16:23, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@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.

Sounds like in the atomic age :) plane->fb should not even exist then. 
If it cannot be trusted from within skl_update_plane, which is at he 
point state should be all ready for programming into the hardware, then 
I see no point to it. At least to respect the principle of least surprise.

Only replying since it might have been me who put that code in and git 
blame on intel_display.c takes ages.

Regards,

Tvrtko

> 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@lists.freedesktop.org
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Testcase: igt/kms_plane
> Reported-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   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);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx