[PATCH] drm/fb-helper: Use proper plane mask for fb cleanup

[PATCH] drm/fb-helper: Use proper plane mask for fb cleanup

[Matt Roper]

pan_display_atomic() calls drm_atomic_clean_old_fb() to sanitize the
legacy FB fields (plane->fb and plane->old_fb).  However it was building
the plane mask to pass to this function incorrectly (the bitwise OR was
using plane indices rather than plane masks).  The end result was that
sometimes the legacy pointers would become out of sync with the atomic
pointers.  If another operation tried to re-set the same FB onto the
plane, we might end up with the pointers back in sync, but improper
reference counts, which would eventually lead to system crashes when we
accessed a pointer to a prematurely-destroyed FB.

The cause here was a very subtle bug introduced in commit:

        commit 07d3bad6c1210bd21e85d084807ef4ee4ac43a78
        Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
        Date:   Wed Nov 11 11:29:11 2015 +0100

            drm/core: Fix old_fb handling in pan_display_atomic.

I found the crashes were most easily reproduced (on i915 at least) by
starting X and then VT switching to a VT that wasn't running a console
instance...the sequence of vt/fbcon entries that happen in that case
trigger a reference count mismatch and crash the system.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93313
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 69cbab5..1e103c4 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1251,7 +1251,7 @@ retry:
 			goto fail;
 
 		plane = mode_set->crtc->primary;
-		plane_mask |= drm_plane_index(plane);
+		plane_mask |= (1 << drm_plane_index(plane));
 		plane->old_fb = plane->fb;
 	}
 
-- 
2.1.4

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

✗ warning: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on 7cdc548e77f503593b83a1c5d58f4dcc862c17e2 drm-intel-nightly: 2015y-12m-18d-19h-26m-21s UTC integration manifest

Test gem_storedw_loop:
        Subgroup basic-render:
                pass       -> DMESG-WARN (skl-i7k-2)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (skl-i7k-2)
        Subgroup basic-flip-vs-modeset:
                dmesg-warn -> PASS       (skl-i5k-2)
                dmesg-warn -> PASS       (byt-nuc)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (byt-nuc)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-a:
                dmesg-warn -> PASS       (snb-x220t)
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup read-crc-pipe-a-frame-sequence:
                dmesg-warn -> PASS       (byt-nuc)
        Subgroup read-crc-pipe-c:
                pass       -> DMESG-WARN (skl-i5k-2)
                dmesg-warn -> PASS       (skl-i7k-2)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (snb-x220t)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (bdw-nuci7)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-warn -> PASS       (bdw-ultra)

bdw-nuci7        total:135  pass:124  dwarn:2   dfail:0   fail:0   skip:9  
bdw-ultra        total:132  pass:124  dwarn:2   dfail:0   fail:0   skip:6  
byt-nuc          total:135  pass:120  dwarn:2   dfail:0   fail:0   skip:13 
hsw-brixbox      total:135  pass:126  dwarn:2   dfail:0   fail:0   skip:7  
hsw-gt2          total:135  pass:130  dwarn:1   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:135  pass:100  dwarn:0   dfail:0   fail:0   skip:35 
ivb-t430s        total:135  pass:127  dwarn:2   dfail:0   fail:0   skip:6  
skl-i5k-2        total:135  pass:122  dwarn:5   dfail:0   fail:0   skip:8  
skl-i7k-2        total:135  pass:121  dwarn:6   dfail:0   fail:0   skip:8  
snb-dellxps      total:135  pass:121  dwarn:2   dfail:0   fail:0   skip:12 
snb-x220t        total:135  pass:121  dwarn:2   dfail:0   fail:1   skip:11 

Results at /archive/results/CI_IGT_test/Patchwork_729/

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

Re: [PATCH] drm/fb-helper: Use proper plane mask for fb cleanup

[Daniel Vetter]

On Fri, Dec 18, 2015 at 05:27:01PM -0800, Matt Roper wrote:
> pan_display_atomic() calls drm_atomic_clean_old_fb() to sanitize the
> legacy FB fields (plane->fb and plane->old_fb).  However it was building
> the plane mask to pass to this function incorrectly (the bitwise OR was
> using plane indices rather than plane masks).  The end result was that
> sometimes the legacy pointers would become out of sync with the atomic
> pointers.  If another operation tried to re-set the same FB onto the
> plane, we might end up with the pointers back in sync, but improper
> reference counts, which would eventually lead to system crashes when we
> accessed a pointer to a prematurely-destroyed FB.
> 
> The cause here was a very subtle bug introduced in commit:
> 
>         commit 07d3bad6c1210bd21e85d084807ef4ee4ac43a78
>         Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>         Date:   Wed Nov 11 11:29:11 2015 +0100
> 
>             drm/core: Fix old_fb handling in pan_display_atomic.
> 
> I found the crashes were most easily reproduced (on i915 at least) by
> starting X and then VT switching to a VT that wasn't running a console
> instance...the sequence of vt/fbcon entries that happen in that case
> trigger a reference count mismatch and crash the system.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93313
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Applied to drm-misc, thanks.
-Daniel

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 69cbab5..1e103c4 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1251,7 +1251,7 @@ retry:
>  			goto fail;
>  
>  		plane = mode_set->crtc->primary;
> -		plane_mask |= drm_plane_index(plane);
> +		plane_mask |= (1 << drm_plane_index(plane));
>  		plane->old_fb = plane->fb;
>  	}
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/fb-helper: Use proper plane mask for fb cleanup

[Maarten Lankhorst]

Op 19-12-15 om 02:27 schreef Matt Roper:
> pan_display_atomic() calls drm_atomic_clean_old_fb() to sanitize the
> legacy FB fields (plane->fb and plane->old_fb).  However it was building
> the plane mask to pass to this function incorrectly (the bitwise OR was
> using plane indices rather than plane masks).  The end result was that
> sometimes the legacy pointers would become out of sync with the atomic
> pointers.  If another operation tried to re-set the same FB onto the
> plane, we might end up with the pointers back in sync, but improper
> reference counts, which would eventually lead to system crashes when we
> accessed a pointer to a prematurely-destroyed FB.
>
> The cause here was a very subtle bug introduced in commit:
>
>         commit 07d3bad6c1210bd21e85d084807ef4ee4ac43a78
>         Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>         Date:   Wed Nov 11 11:29:11 2015 +0100
>
>             drm/core: Fix old_fb handling in pan_display_atomic.
>
> I found the crashes were most easily reproduced (on i915 at least) by
> starting X and then VT switching to a VT that wasn't running a console
> instance...the sequence of vt/fbcon entries that happen in that case
> trigger a reference count mismatch and crash the system.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93313
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 69cbab5..1e103c4 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1251,7 +1251,7 @@ retry:
>  			goto fail;
>  
>  		plane = mode_set->crtc->primary;
> -		plane_mask |= drm_plane_index(plane);
> +		plane_mask |= (1 << drm_plane_index(plane));
>  		plane->old_fb = plane->fb;
>  	}
>  

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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