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; }
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
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
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.
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Jani Nikula