[PATCH RESEND] drm/dp/mst: fix kernel oops when turning off secondary monitor

[PATCH RESEND] drm/dp/mst: fix kernel oops when turning off secondary monitor

[Jani Nikula]

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

100% reproducible issue found on SKL SkullCanyon NUC with two external
DP daisy-chained monitors in DP/MST mode. When turning off or changing
the input of the second monitor the machine stops with a kernel
oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.

This issue is traced to an inconsistent control flow in
drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at the
same time as 'req_payload.num_slots' is set to zero, but the pointer is
dereferenced even when req_payload.num_slot is zero.

The problematic dereference was introduced in commit dfda0df34
("drm/mst: rework payload table allocation to conform better") and may
impact all versions since v3.18

The fix suggested by Chris Wilson removes the kernel oops and was found to
work well after 10mn of monkey-testing with the second monitor power and
input buttons

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
Fixes: dfda0df34264 ("drm/mst: rework payload table allocation to conform better.")
Cc: Dave Airlie <airlied@redhat.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: <stable@vger.kernel.org> # v3.18+
Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 122a1b04bebc..f2cc375907d0 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
 				mgr->payloads[i].vcpi = req_payload.vcpi;
 			} else if (mgr->payloads[i].num_slots) {
 				mgr->payloads[i].num_slots = 0;
-				drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]);
+				drm_dp_destroy_payload_step1(mgr, port, mgr->payloads[i].vcpi, &mgr->payloads[i]);
 				req_payload.payload_state = mgr->payloads[i].payload_state;
 				mgr->payloads[i].start_slot = 0;
 			}
-- 
2.1.4

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

✓ Fi.CI.BAT: success for drm/dp/mst: fix kernel oops when turning off secondary monitor (rev3)

[Patchwork]

== Series Details ==

Series: drm/dp/mst: fix kernel oops when turning off secondary monitor (rev3)
URL   : https://patchwork.freedesktop.org/series/16337/
State : success

== Summary ==

Series 16337v3 drm/dp/mst: fix kernel oops when turning off secondary monitor
https://patchwork.freedesktop.org/api/1.0/series/16337/revisions/3/mbox/

Test gem_exec_fence:
        Subgroup await-hang-default:
                pass       -> INCOMPLETE (fi-hsw-4770) fdo#99726

fdo#99726 https://bugs.freedesktop.org/show_bug.cgi?id=99726

fi-bdw-5557u     total:252  pass:241  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:252  pass:213  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:252  pass:225  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:252  pass:221  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:48   pass:41   dwarn:0   dfail:0   fail:0   skip:6  
fi-hsw-4770r     total:252  pass:236  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:252  pass:202  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:252  pass:234  dwarn:0   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:252  pass:242  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:252  pass:235  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:252  pass:230  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:252  pass:242  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:252  pass:224  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:252  pass:223  dwarn:0   dfail:0   fail:0   skip:29 

7b80cb5504b3b3cebaea944d35f25e27415e89a6 drm-tip: 2017y-02m-14d-14h-36m-43s UTC integration manifest
1d27a82 drm/dp/mst: fix kernel oops when turning off secondary monitor

== Logs ==

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

Re: [Intel-gfx] [PATCH RESEND] drm/dp/mst: fix kernel oops when turning off secondary monitor

[Daniel Vetter]

On Tue, Feb 14, 2017 at 02:49:21PM +0200, Jani Nikula wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> 100% reproducible issue found on SKL SkullCanyon NUC with two external
> DP daisy-chained monitors in DP/MST mode. When turning off or changing
> the input of the second monitor the machine stops with a kernel
> oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
> 
> This issue is traced to an inconsistent control flow in
> drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at the
> same time as 'req_payload.num_slots' is set to zero, but the pointer is
> dereferenced even when req_payload.num_slot is zero.
> 
> The problematic dereference was introduced in commit dfda0df34
> ("drm/mst: rework payload table allocation to conform better") and may
> impact all versions since v3.18
> 
> The fix suggested by Chris Wilson removes the kernel oops and was found to
> work well after 10mn of monkey-testing with the second monitor power and
> input buttons
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
> Fixes: dfda0df34264 ("drm/mst: rework payload table allocation to conform better.")
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: <stable@vger.kernel.org> # v3.18+
> Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com>
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

You haz drm-misc commit rights, pls use them :-)

Since it doesn't have deps, probably simplest to smash into drm-misc-fixes
and then send a pull req to Dave right away. If you want, you can roll
-fixes forward to -rc8 while at it.
-Daniel

> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 122a1b04bebc..f2cc375907d0 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
>  				mgr->payloads[i].vcpi = req_payload.vcpi;
>  			} else if (mgr->payloads[i].num_slots) {
>  				mgr->payloads[i].num_slots = 0;
> -				drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]);
> +				drm_dp_destroy_payload_step1(mgr, port, mgr->payloads[i].vcpi, &mgr->payloads[i]);
>  				req_payload.payload_state = mgr->payloads[i].payload_state;
>  				mgr->payloads[i].start_slot = 0;
>  			}
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH RESEND] drm/dp/mst: fix kernel oops when turning off secondary monitor

[Jani Nikula]

On Tue, 14 Feb 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Feb 14, 2017 at 02:49:21PM +0200, Jani Nikula wrote:
>> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> 
>> 100% reproducible issue found on SKL SkullCanyon NUC with two external
>> DP daisy-chained monitors in DP/MST mode. When turning off or changing
>> the input of the second monitor the machine stops with a kernel
>> oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
>> 
>> This issue is traced to an inconsistent control flow in
>> drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at the
>> same time as 'req_payload.num_slots' is set to zero, but the pointer is
>> dereferenced even when req_payload.num_slot is zero.
>> 
>> The problematic dereference was introduced in commit dfda0df34
>> ("drm/mst: rework payload table allocation to conform better") and may
>> impact all versions since v3.18
>> 
>> The fix suggested by Chris Wilson removes the kernel oops and was found to
>> work well after 10mn of monkey-testing with the second monitor power and
>> input buttons
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
>> Fixes: dfda0df34264 ("drm/mst: rework payload table allocation to conform better.")
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Cc: Sean Paul <seanpaul@chromium.org>
>> Cc: <stable@vger.kernel.org> # v3.18+
>> Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com>
>> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> You haz drm-misc commit rights, pls use them :-)

Sure, I just wanted to have a fresh CI run on this.

> Since it doesn't have deps, probably simplest to smash into drm-misc-fixes
> and then send a pull req to Dave right away. If you want, you can roll
> -fixes forward to -rc8 while at it.

Will do.

BR,
Jani.

> -Daniel
>
>> ---
>>  drivers/gpu/drm/drm_dp_mst_topology.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
>> index 122a1b04bebc..f2cc375907d0 100644
>> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
>> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
>> @@ -1817,7 +1817,7 @@ int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr)
>>  				mgr->payloads[i].vcpi = req_payload.vcpi;
>>  			} else if (mgr->payloads[i].num_slots) {
>>  				mgr->payloads[i].num_slots = 0;
>> -				drm_dp_destroy_payload_step1(mgr, port, port->vcpi.vcpi, &mgr->payloads[i]);
>> +				drm_dp_destroy_payload_step1(mgr, port, mgr->payloads[i].vcpi, &mgr->payloads[i]);
>>  				req_payload.payload_state = mgr->payloads[i].payload_state;
>>  				mgr->payloads[i].start_slot = 0;
>>  			}
>> -- 
>> 2.1.4
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH RESEND] drm/dp/mst: fix kernel oops when turning off secondary monitor

[Jani Nikula]

On Wed, 15 Feb 2017, Jani Nikula <jani.nikula@intel.com> wrote:
> On Tue, 14 Feb 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Tue, Feb 14, 2017 at 02:49:21PM +0200, Jani Nikula wrote:
>>> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>> 
>>> 100% reproducible issue found on SKL SkullCanyon NUC with two external
>>> DP daisy-chained monitors in DP/MST mode. When turning off or changing
>>> the input of the second monitor the machine stops with a kernel
>>> oops. This issue happened with 4.8.8 as well as drm/drm-intel-nightly.
>>> 
>>> This issue is traced to an inconsistent control flow in
>>> drm_dp_update_payload_part1(): the 'port' pointer is set to NULL at the
>>> same time as 'req_payload.num_slots' is set to zero, but the pointer is
>>> dereferenced even when req_payload.num_slot is zero.
>>> 
>>> The problematic dereference was introduced in commit dfda0df34
>>> ("drm/mst: rework payload table allocation to conform better") and may
>>> impact all versions since v3.18
>>> 
>>> The fix suggested by Chris Wilson removes the kernel oops and was found to
>>> work well after 10mn of monkey-testing with the second monitor power and
>>> input buttons
>>> 
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98990
>>> Fixes: dfda0df34264 ("drm/mst: rework payload table allocation to conform better.")
>>> Cc: Dave Airlie <airlied@redhat.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com>
>>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>>> Cc: Sean Paul <seanpaul@chromium.org>
>>> Cc: <stable@vger.kernel.org> # v3.18+
>>> Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@linux.intel.com>
>>> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>
>> You haz drm-misc commit rights, pls use them :-)
>
> Sure, I just wanted to have a fresh CI run on this.
>
>> Since it doesn't have deps, probably simplest to smash into drm-misc-fixes
>> and then send a pull req to Dave right away. If you want, you can roll
>> -fixes forward to -rc8 while at it.
>
> Will do.

And pushed to drm-misc-fixes, thanks for the patch, review and testing.

BR,
Jani.



-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx