On Fri, Aug 16, 2019 at 4:25 PM Lyude Paul <lyude@xxxxxxxxxx> wrote: > > Looks like a regression got introduced into nv50_mstc_atomic_check() > that somehow didn't get found until now. If userspace changes > crtc_state->active to false but leaves the CRTC enabled, we end up > calling drm_dp_atomic_find_vcpi_slots() using the PBN calculated in > asyh->dp.pbn. However, if the display is inactive we end up calculating > a PBN of 0, which inadvertently causes us to have an allocation of 0. > >From there, if userspace then disables the CRTC afterwards we end up > accidentally attempting to free the VCPI twice: > > WARNING: CPU: 0 PID: 1484 at drivers/gpu/drm/drm_dp_mst_topology.c:3336 > drm_dp_atomic_release_vcpi_slots+0x87/0xb0 [drm_kms_helper] > RIP: 0010:drm_dp_atomic_release_vcpi_slots+0x87/0xb0 [drm_kms_helper] > Call Trace: > drm_atomic_helper_check_modeset+0x3f3/0xa60 [drm_kms_helper] > ? drm_atomic_check_only+0x43/0x780 [drm] > drm_atomic_helper_check+0x15/0x90 [drm_kms_helper] > nv50_disp_atomic_check+0x83/0x1d0 [nouveau] > drm_atomic_check_only+0x54d/0x780 [drm] > ? drm_atomic_set_crtc_for_connector+0xec/0x100 [drm] > drm_atomic_commit+0x13/0x50 [drm] > drm_atomic_helper_set_config+0x81/0x90 [drm_kms_helper] > drm_mode_setcrtc+0x194/0x6a0 [drm] > ? vprintk_emit+0x16a/0x230 > ? drm_ioctl+0x163/0x390 [drm] > ? drm_mode_getcrtc+0x180/0x180 [drm] > drm_ioctl_kernel+0xaa/0xf0 [drm] > drm_ioctl+0x208/0x390 [drm] > ? drm_mode_getcrtc+0x180/0x180 [drm] > nouveau_drm_ioctl+0x63/0xb0 [nouveau] > do_vfs_ioctl+0x405/0x660 > ? recalc_sigpending+0x17/0x50 > ? _copy_from_user+0x37/0x60 > ksys_ioctl+0x5e/0x90 > ? exit_to_usermode_loop+0x92/0xe0 > __x64_sys_ioctl+0x16/0x20 > do_syscall_64+0x59/0x190 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > WARNING: CPU: 0 PID: 1484 at drivers/gpu/drm/drm_dp_mst_topology.c:3336 > drm_dp_atomic_release_vcpi_slots+0x87/0xb0 [drm_kms_helper] > ---[ end trace 4c395c0c51b1f88d ]--- > [drm:drm_dp_atomic_release_vcpi_slots [drm_kms_helper]] *ERROR* no VCPI for > [MST PORT:00000000e288eb7d] found in mst state 000000008e642070 > > So, fix this by doing what we probably should have done from the start: only > call drm_dp_atomic_find_vcpi_slots() when crtc_state->mode_changed is set, so > that VCPI allocations remain for as long as the CRTC is enabled. > > Signed-off-by: Lyude Paul <lyude@xxxxxxxxxx> > Fixes: 232c9eec417a ("drm/nouveau: Use atomic VCPI helpers for MST") > Cc: Lyude Paul <lyude@xxxxxxxxxx> > Cc: Ben Skeggs <bskeggs@xxxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > Cc: David Airlie <airlied@xxxxxxxxxx> > Cc: Jerry Zuo <Jerry.Zuo@xxxxxxx> > Cc: Harry Wentland <harry.wentland@xxxxxxx> > Cc: Juston Li <juston.li@xxxxxxxxx> > Cc: Karol Herbst <karolherbst@xxxxxxxxx> > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > Cc: Ilia Mirkin <imirkin@xxxxxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> # v5.1+ > Acked-by: Ben Skeggs <bskeggs@xxxxxxxxxx> > Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx> > Link: https://patchwork.freedesktop.org/patch/msgid/20190801220216.15323-1-lyude@xxxxxxxxxx > --- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) This patch was included in 5.2.7 stable upstream. Justin > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c > index 4b1650f51955..847b7866137d 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > @@ -775,7 +775,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, > drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, > connector->display_info.bpc * 3); > > - if (drm_atomic_crtc_needs_modeset(crtc_state)) { > + if (crtc_state->mode_changed) { > slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr, > mstc->port, > asyh->dp.pbn); > -- > 2.21.0 > _______________________________________________ > kernel mailing list -- kernel@xxxxxxxxxxxxxxxxxxxxxxx > To unsubscribe send an email to kernel-leave@xxxxxxxxxxxxxxxxxxxxxxx > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines > List Archives: https://lists.fedoraproject.org/archives/list/kernel@xxxxxxxxxxxxxxxxxxxxxxx _______________________________________________ kernel mailing list -- kernel@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to kernel-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/kernel@xxxxxxxxxxxxxxxxxxxxxxx