1. 27 7月, 2019 1 次提交
    • S
      drm/mst: Fix sphinx warnings in drm_dp_msg_connector register functions · 63b87c31
      Sean Paul 提交于
      Fixes the following warnings:
      
      ../drivers/gpu/drm/drm_dp_mst_topology.c:1593: warning: Excess function parameter 'drm_connector' description in 'drm_dp_mst_connector_late_register'
      ../drivers/gpu/drm/drm_dp_mst_topology.c:1613: warning: Excess function parameter 'drm_connector' description in 'drm_dp_mst_connector_early_unregister'
      ../drivers/gpu/drm/drm_dp_mst_topology.c:1594: warning: Function parameter or member 'connector' not described in 'drm_dp_mst_connector_late_register'
      ../drivers/gpu/drm/drm_dp_mst_topology.c:1614: warning: Function parameter or member 'connector' not described in 'drm_dp_mst_connector_early_unregister'
      
      Fixes: 562836a2 ("drm/dp_mst: Enable registration of AUX devices for MST ports")
      Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
      Cc: Leo Li <sunpeng.li@amd.com>
      Cc: Lyude Paul <lyude@redhat.com>
      Cc: Harry Wentland <harry.wentland@amd.com>
      Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
      Cc: Maxime Ripard <maxime.ripard@bootlin.com>
      Cc: Sean Paul <sean@poorly.run>
      Cc: David Airlie <airlied@linux.ie>
      Cc: Daniel Vetter <daniel@ffwll.ch>
      Cc: dri-devel@lists.freedesktop.org
      Reviewed-by: NLyude Paul <lyude@redhat.com>
      Signed-off-by: NSean Paul <seanpaul@chromium.org>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190726142057.224121-1-sean@poorly.run
      63b87c31
  2. 26 7月, 2019 1 次提交
    • V
      drm/dp_mst: Enable registration of AUX devices for MST ports · 562836a2
      Ville Syrjälä 提交于
      All available downstream ports - physical and logical - are exposed for
      each MST device. They are listed in /dev/, following the same naming
      scheme as SST devices by appending an incremental ID.
      
      Although all downstream ports are exposed, only some will work as
      expected. Consider the following topology:
      
                     +---------+
                     |  ASIC   |
                     +---------+
                    Conn-0|
                          |
                     +----v----+
                +----| MST HUB |----+
                |    +---------+    |
                |                   |
                |Port-1       Port-2|
          +-----v-----+       +-----v-----+
          |  MST      |       |  SST      |
          |  Display  |       |  Display  |
          +-----------+       +-----------+
                |Port-1
                x
      
       MST Path  | MST Device
       ----------+----------------------------------
       sst:0     | MST Hub
       mst:0-1   | MST Display
       mst:0-1-1 | MST Display's disconnected DP out
       mst:0-1-8 | MST Display's internal sink
       mst:0-2   | SST Display
      
      On certain MST displays, the upstream physical port will ACK DPCD reads.
      However, reads on the local logical port to the internal sink will
      *NAK*. i.e. reading mst:0-1 ACKs, but mst:0-1-8 NAKs.
      
      There may also be duplicates. Some displays will return the same GUID
      when reading DPCD from both mst:0-1 and mst:0-1-8.
      
      There are some device-dependent behavior as well. The MST hub used
      during testing will actually *ACK* read requests on a disconnected
      physical port, whereas the MST displays will NAK.
      
      In light of these discrepancies, it's simpler to expose all downstream
      ports - both physical and logical - and let the user decide what to use.
      
      v3 changes:
      * Change WARN_ON_ONCE -> DRM_ERROR on dpcd read errors
      * Docstring and cosmetic fixes
      
      v2 changes:
      
      Moved remote aux device (un)registration to new mst connector late
      register and early unregister helpers. Drivers should call these from
      their own mst connector function hooks.
      
      This is to solve an issue during driver unload, where mst connector
      devices are unregistered before the remote aux devices are. In a setup
      where aux devices are created as children of connector devices, the aux
      device would be removed too early, and uncleanly. Doing so in
      early_unregister solves this issue, as that is called before connector
      unregistration.
      Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
      Signed-off-by: NLeo Li <sunpeng.li@amd.com>
      Reviewed-by: NLyude Paul <lyude@redhat.com>
      Signed-off-by: NHarry Wentland <harry.wentland@amd.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190723232808.28128-3-sunpeng.li@amd.com
      562836a2
  3. 30 5月, 2019 1 次提交
    • I
      drm/mst: Fix MST sideband up-reply failure handling · d8fd3722
      Imre Deak 提交于
      Fix the breakage resulting in the stacktrace below, due to tx queue
      being full when trying to send an up-reply. txmsg->seqno is -1 in this
      case leading to a corruption of the mstb object by
      
      	txmsg->dst->tx_slots[txmsg->seqno] = NULL;
      
      in process_single_up_tx_qlock().
      
      [  +0,005162] [drm:process_single_tx_qlock [drm_kms_helper]] set_hdr_from_dst_qlock: failed to find slot
      [  +0,000015] [drm:drm_dp_send_up_ack_reply.constprop.19 [drm_kms_helper]] failed to send msg in q -11
      [  +0,000939] BUG: kernel NULL pointer dereference, address: 00000000000005a0
      [  +0,006982] #PF: supervisor write access in kernel mode
      [  +0,005223] #PF: error_code(0x0002) - not-present page
      [  +0,005135] PGD 0 P4D 0
      [  +0,002581] Oops: 0002 [#1] PREEMPT SMP NOPTI
      [  +0,004359] CPU: 1 PID: 1200 Comm: kworker/u16:3 Tainted: G     U            5.2.0-rc1+ #410
      [  +0,008433] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP, BIOS ICLSFWR1.R00.3175.A00.1904261428 04/26/2019
      [  +0,013323] Workqueue: i915-dp i915_digport_work_func [i915]
      [  +0,005676] RIP: 0010:queue_work_on+0x19/0x70
      [  +0,004372] Code: ff ff ff 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 41 56 49 89 f6 41 55 41 89 fd 41 54 55 53 48 89 d3 9c 5d fa e8 e7 81 0c 00 <f0> 48 0f ba 2b 00 73 31 45 31 e4 f7 c5 00 02 00 00 74 13 e8 cf 7f
      [  +0,018750] RSP: 0018:ffffc900007dfc50 EFLAGS: 00010006
      [  +0,005222] RAX: 0000000000000046 RBX: 00000000000005a0 RCX: 0000000000000001
      [  +0,007133] RDX: 000000000001b608 RSI: 0000000000000000 RDI: ffffffff82121972
      [  +0,007129] RBP: 0000000000000202 R08: 0000000000000000 R09: 0000000000000001
      [  +0,007129] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88847bfa5096
      [  +0,007131] R13: 0000000000000010 R14: ffff88849c08f3f8 R15: 0000000000000000
      [  +0,007128] FS:  0000000000000000(0000) GS:ffff88849dc80000(0000) knlGS:0000000000000000
      [  +0,008083] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
      [  +0,005749] CR2: 00000000000005a0 CR3: 0000000005210006 CR4: 0000000000760ee0
      [  +0,007128] PKRU: 55555554
      [  +0,002722] Call Trace:
      [  +0,002458]  drm_dp_mst_handle_up_req+0x517/0x540 [drm_kms_helper]
      [  +0,006197]  ? drm_dp_mst_hpd_irq+0x5b/0x9c0 [drm_kms_helper]
      [  +0,005764]  drm_dp_mst_hpd_irq+0x5b/0x9c0 [drm_kms_helper]
      [  +0,005623]  ? intel_dp_hpd_pulse+0x205/0x370 [i915]
      [  +0,005018]  intel_dp_hpd_pulse+0x205/0x370 [i915]
      [  +0,004836]  i915_digport_work_func+0xbb/0x140 [i915]
      [  +0,005108]  process_one_work+0x245/0x610
      [  +0,004027]  worker_thread+0x37/0x380
      [  +0,003684]  ? process_one_work+0x610/0x610
      [  +0,004184]  kthread+0x119/0x130
      [  +0,003240]  ? kthread_park+0x80/0x80
      [  +0,003668]  ret_from_fork+0x24/0x50
      
      Cc: Lyude Paul <lyude@redhat.com>
      Cc: Dave Airlie <airlied@redhat.com>
      Signed-off-by: NImre Deak <imre.deak@intel.com>
      Reviewed-by: NLyude Paul <lyude@redhat.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190523212433.9058-1-imre.deak@intel.com
      d8fd3722
  4. 06 5月, 2019 1 次提交
  5. 15 3月, 2019 1 次提交
  6. 07 2月, 2019 1 次提交
  7. 06 2月, 2019 3 次提交
    • L
      drm/atomic: Add drm_atomic_state->duplicated · 022debad
      Lyude Paul 提交于
      Since
      
      commit 39b50c60 ("drm/atomic_helper: Stop modesets on unregistered
      connectors harder")
      
      We've been failing atomic checks if they try to enable new displays on
      unregistered connectors. This is fine except for the one situation that
      breaks atomic assumptions: suspend/resume. If a connector is
      unregistered before we attempt to restore the atomic state, something we
      end up failing the atomic check that happens when trying to restore the
      state during resume.
      
      Normally this would be OK: we try our best to make sure that the atomic
      state pre-suspend can be restored post-suspend, but failures at that
      point usually don't cause problems. That is of course, until we
      introduced the new atomic MST VCPI helpers:
      
      [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] [CRTC:65:pipe B] active changed
      [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] Updating routing for [CONNECTOR:123:DP-5]
      [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] Disabling [CONNECTOR:123:DP-5]
      [drm:drm_atomic_get_private_obj_state [drm]] Added new private object 0000000025844636 state 000000009fd2899a to 000000003a13d7b8
      WARNING: CPU: 6 PID: 1070 at drivers/gpu/drm/drm_dp_mst_topology.c:3153 drm_dp_atomic_release_vcpi_slots+0xb9/0x200 [drm_kms_helper]
      Modules linked in: fuse vfat fat snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic joydev iTCO_wdt i915(O) wmi_bmof intel_rapl btusb btrtl x86_pkg_temp_thermal btbcm btintel coretemp i2c_algo_bit drm_kms_helper(O) crc32_pclmul snd_hda_intel syscopyarea sysfillrect snd_hda_codec sysimgblt snd_hda_core bluetooth fb_sys_fops snd_pcm pcspkr drm(O) psmouse snd_timer mei_me ecdh_generic i2c_i801 mei i2c_core ucsi_acpi typec_ucsi typec wmi thinkpad_acpi ledtrig_audio snd soundcore tpm_tis rfkill tpm_tis_core video tpm acpi_pad pcc_cpufreq uas usb_storage crc32c_intel nvme serio_raw xhci_pci nvme_core xhci_hcd
      CPU: 6 PID: 1070 Comm: gnome-shell Tainted: G        W  O      5.0.0-rc2Lyude-Test+ #1
      Hardware name: LENOVO 20L8S2N800/20L8S2N800, BIOS N22ET35W (1.12 ) 04/09/2018
      RIP: 0010:drm_dp_atomic_release_vcpi_slots+0xb9/0x200 [drm_kms_helper]
      Code: 00 4c 39 6d f0 74 49 48 8d 7b 10 48 89 f9 48 c1 e9 03 42 80 3c 21 00 0f 85 d2 00 00 00 48 8b 6b 10 48 8d 5d f0 49 39 ee 75 c5 <0f> 0b 48 c7 c7 c0 78 b3 a0 48 89 c2 4c 89 ee e8 03 6c aa ff b8 ea
      RSP: 0018:ffff88841235f268 EFLAGS: 00010246
      RAX: ffff88841bf12ab0 RBX: ffff88841bf12aa8 RCX: 1ffff110837e2557
      RDX: dffffc0000000000 RSI: 0000000000000000 RDI: ffffed108246bde0
      RBP: ffff88841bf12ab8 R08: ffffed1083db3c93 R09: ffffed1083db3c92
      R10: ffffed1083db3c92 R11: ffff88841ed9e497 R12: ffff888419555d80
      R13: ffff8883bc499100 R14: ffff88841bf12ab8 R15: 0000000000000000
      FS:  00007f16fbd4cd00(0000) GS:ffff88841ed80000(0000) knlGS:0000000000000000
      CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
      CR2: 00007f1687c9f000 CR3: 00000003ba3cc003 CR4: 00000000003606e0
      DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
      DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
      Call Trace:
       drm_atomic_helper_check_modeset+0xf21/0x2f50 [drm_kms_helper]
       ? drm_atomic_helper_commit_modeset_enables+0xa90/0xa90 [drm_kms_helper]
       ? __printk_safe_exit+0x10/0x10
       ? save_stack+0x8c/0xb0
       ? vprintk_func+0x96/0x1bf
       ? __printk_safe_exit+0x10/0x10
       intel_atomic_check+0x234/0x4750 [i915]
       ? printk+0x9f/0xc5
       ? kmsg_dump_rewind_nolock+0xd9/0xd9
       ? _raw_spin_lock_irqsave+0xa4/0x140
       ? drm_atomic_check_only+0xb1/0x28b0 [drm]
       ? drm_dbg+0x186/0x1b0 [drm]
       ? drm_dev_dbg+0x200/0x200 [drm]
       ? intel_link_compute_m_n+0xb0/0xb0 [i915]
       ? drm_mode_put_tile_group+0x20/0x20 [drm]
       ? skl_plane_format_mod_supported+0x17f/0x1b0 [i915]
       ? drm_plane_check_pixel_format+0x14a/0x310 [drm]
       drm_atomic_check_only+0x13c4/0x28b0 [drm]
       ? drm_state_info+0x220/0x220 [drm]
       ? drm_atomic_helper_disable_plane+0x1d0/0x1d0 [drm_kms_helper]
       ? pick_single_encoder_for_connector+0xe0/0xe0 [drm_kms_helper]
       ? kasan_unpoison_shadow+0x35/0x40
       drm_atomic_commit+0x3b/0x100 [drm]
       drm_atomic_helper_set_config+0xd5/0x100 [drm_kms_helper]
       drm_mode_setcrtc+0x636/0x1660 [drm]
       ? vprintk_func+0x96/0x1bf
       ? drm_dev_dbg+0x200/0x200 [drm]
       ? drm_mode_getcrtc+0x790/0x790 [drm]
       ? printk+0x9f/0xc5
       ? mutex_unlock+0x1d/0x40
       ? drm_mode_addfb2+0x2e9/0x3a0 [drm]
       ? rcu_sync_dtor+0x2e0/0x2e0
       ? drm_dbg+0x186/0x1b0 [drm]
       ? set_page_dirty+0x271/0x4d0
       drm_ioctl_kernel+0x203/0x290 [drm]
       ? drm_mode_getcrtc+0x790/0x790 [drm]
       ? drm_setversion+0x7f0/0x7f0 [drm]
       ? __switch_to_asm+0x34/0x70
       ? __switch_to_asm+0x34/0x70
       drm_ioctl+0x445/0x950 [drm]
       ? drm_mode_getcrtc+0x790/0x790 [drm]
       ? drm_getunique+0x220/0x220 [drm]
       ? expand_files.part.10+0x920/0x920
       do_vfs_ioctl+0x1a1/0x13d0
       ? ioctl_preallocate+0x2b0/0x2b0
       ? __fget_light+0x2d6/0x390
       ? schedule+0xd7/0x2e0
       ? fget_raw+0x10/0x10
       ? apic_timer_interrupt+0xa/0x20
       ? apic_timer_interrupt+0xa/0x20
       ? rcu_cleanup_dead_rnp+0x2c0/0x2c0
       ksys_ioctl+0x60/0x90
       __x64_sys_ioctl+0x6f/0xb0
       do_syscall_64+0x136/0x440
       ? syscall_return_slowpath+0x2d0/0x2d0
       ? do_page_fault+0x89/0x330
       ? __do_page_fault+0x9c0/0x9c0
       ? prepare_exit_to_usermode+0x188/0x200
       ? perf_trace_sys_enter+0x1090/0x1090
       ? __x64_sys_sigaltstack+0x280/0x280
       ? __put_user_4+0x1c/0x30
       entry_SYSCALL_64_after_hwframe+0x44/0xa9
      RIP: 0033:0x7f16ff89a09b
      Code: 0f 1e fa 48 8b 05 ed bd 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d bd bd 0c 00 f7 d8 64 89 01 48
      RSP: 002b:00007fff001232b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
      RAX: ffffffffffffffda RBX: 00007fff001232f0 RCX: 00007f16ff89a09b
      RDX: 00007fff001232f0 RSI: 00000000c06864a2 RDI: 000000000000000b
      RBP: 00007fff001232f0 R08: 0000000000000000 R09: 000055a79d484460
      R10: 000055a79d44e770 R11: 0000000000000246 R12: 00000000c06864a2
      R13: 000000000000000b R14: 0000000000000000 R15: 000055a79d44e770
      WARNING: CPU: 6 PID: 1070 at drivers/gpu/drm/drm_dp_mst_topology.c:3153 drm_dp_atomic_release_vcpi_slots+0xb9/0x200 [drm_kms_helper]
      ---[ end trace d536c05c13c83be2 ]---
      [drm:drm_dp_atomic_release_vcpi_slots [drm_kms_helper]] *ERROR* no VCPI for [MST PORT:00000000f9e2b143] found in mst state 000000009fd2899a
      
      This appears to be happening because we destroy the VCPI allocations
      when disabling all connected displays while suspending, and those VCPI
      allocations don't get restored on resume due to failing to restore the
      atomic state.
      
      So, fix this by introducing the suspending option to
      drm_atomic_helper_duplicate_state() and use that to indicate in the
      atomic state that it's being used for suspending or resuming the system,
      and thus needs to be fixed up by the driver. We can then use the new
      state->duplicated hook to tell update_connector_routing() in
      drm_atomic_check_modeset() to allow for modesets on unregistered
      connectors, which allows us to restore atomic states that contain MST
      topologies that were removed after the state was duplicated and thus:
      mostly fixing suspend and resume. This just leaves some issues that were
      introduced with nouveau, that will be addressed next.
      
      Changes since v3:
      * Remove ->duplicated hunks that I left in the VCPI helpers by accident.
        These don't need to be here, that was the supposed to be the purpose
        of the last revision
      Changes since v2:
      * Remove the changes in this patch to the VCPI helpers, they aren't
        needed anymore
      Changes since v1:
      * Rename suspend_or_resume to duplicated
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Fixes: eceae147 ("drm/dp_mst: Start tracking per-port VCPI allocations")
      Cc: Daniel Vetter <daniel@ffwll.ch>
      Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190202002023.29665-4-lyude@redhat.com
      022debad
    • L
      drm/dp_mst: Remove port validation in drm_dp_atomic_find_vcpi_slots() · a3d15c4b
      Lyude Paul 提交于
      Since we now have an easy way of refcounting drm_dp_mst_port structs and
      safely accessing their contents, there isn't any good reason to keep
      validating ports here. It doesn't prevent us from performing modesets on
      branch devices that have been removed either, and we already disallow
      enabling new displays on unregistered connectors in
      update_connector_routing() in drm_atomic_check_modeset(). All it does is
      cause us to have to make weird special exceptions in our atomic
      modesetting code. So, get rid of it entirely.
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Fixes: eceae147 ("drm/dp_mst: Start tracking per-port VCPI allocations")
      Reviewed-by: NDaniel Vetter <daniel@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190202002023.29665-3-lyude@redhat.com
      a3d15c4b
    • L
      drm/dp_mst: Fix unbalanced malloc ref in drm_dp_mst_deallocate_vcpi() · 3a8844c2
      Lyude Paul 提交于
      In drm_dp_mst_deallocate_vcpi(), we currently unconditionally call
      drm_dp_mst_put_port_malloc() on the port that's passed to us, even if we
      never successfully allocated VCPI to it. This is contrary to what we do
      in drm_dp_mst_allocate_vcpi(), where we only call
      drm_dp_mst_get_port_malloc() on the passed port if we successfully
      allocated VCPI to it.
      
      As a result, if drm_dp_mst_allocate_vcpi() fails during a modeset and
      another successive modeset calls drm_dp_mst_deallocate_vcpi() we will
      end up dropping someone else's malloc reference to the port. Example:
      
      [  962.309260] ==================================================================
      [  962.309290] BUG: KASAN: use-after-free in drm_dp_mst_put_port_malloc+0x72/0x180 [drm_kms_helper]
      [  962.309296] Read of size 4 at addr ffff888416c30004 by task kworker/0:1H/500
      
      [  962.309308] CPU: 0 PID: 500 Comm: kworker/0:1H Tainted: G        W  O      5.0.0-rc2Lyude-Test+ #1
      [  962.309313] Hardware name: LENOVO 20L8S2N800/20L8S2N800, BIOS N22ET35W (1.12 ) 04/09/2018
      [  962.309428] Workqueue: events_highpri intel_atomic_cleanup_work [i915]
      [  962.309434] Call Trace:
      [  962.309452]  dump_stack+0xad/0x150
      [  962.309462]  ? dump_stack_print_info.cold.0+0x1b/0x1b
      [  962.309472]  ? kmsg_dump_rewind_nolock+0xd9/0xd9
      [  962.309504]  ? drm_dp_mst_put_port_malloc+0x72/0x180 [drm_kms_helper]
      [  962.309515]  print_address_description+0x6c/0x23c
      [  962.309542]  ? drm_dp_mst_put_port_malloc+0x72/0x180 [drm_kms_helper]
      [  962.309568]  ? drm_dp_mst_put_port_malloc+0x72/0x180 [drm_kms_helper]
      [  962.309577]  kasan_report.cold.3+0x1a/0x32
      [  962.309605]  ? drm_dp_mst_put_port_malloc+0x72/0x180 [drm_kms_helper]
      [  962.309631]  drm_dp_mst_put_port_malloc+0x72/0x180 [drm_kms_helper]
      [  962.309658]  ? drm_dp_mst_put_mstb_malloc+0x180/0x180 [drm_kms_helper]
      [  962.309687]  drm_dp_mst_destroy_state+0xcd/0x120 [drm_kms_helper]
      [  962.309745]  drm_atomic_state_default_clear+0x6ee/0xcc0 [drm]
      [  962.309864]  intel_atomic_state_clear+0xe/0x80 [i915]
      [  962.309928]  __drm_atomic_state_free+0x35/0xd0 [drm]
      [  962.310044]  intel_atomic_cleanup_work+0x56/0x70 [i915]
      [  962.310057]  process_one_work+0x884/0x1400
      [  962.310067]  ? drain_workqueue+0x5a0/0x5a0
      [  962.310075]  ? __schedule+0x87f/0x1e80
      [  962.310086]  ? __sched_text_start+0x8/0x8
      [  962.310095]  ? run_rebalance_domains+0x400/0x400
      [  962.310110]  ? deref_stack_reg+0xb4/0x120
      [  962.310117]  ? __read_once_size_nocheck.constprop.7+0x10/0x10
      [  962.310124]  ? worker_enter_idle+0x47f/0x6a0
      [  962.310134]  ? schedule+0xd7/0x2e0
      [  962.310141]  ? __schedule+0x1e80/0x1e80
      [  962.310148]  ? _raw_spin_lock_irq+0x9f/0x130
      [  962.310155]  ? _raw_write_unlock_irqrestore+0x110/0x110
      [  962.310164]  worker_thread+0x196/0x11e0
      [  962.310175]  ? set_load_weight+0x2e0/0x2e0
      [  962.310181]  ? __switch_to_asm+0x34/0x70
      [  962.310187]  ? __switch_to_asm+0x40/0x70
      [  962.310194]  ? process_one_work+0x1400/0x1400
      [  962.310199]  ? __switch_to_asm+0x40/0x70
      [  962.310205]  ? __switch_to_asm+0x34/0x70
      [  962.310211]  ? __switch_to_asm+0x34/0x70
      [  962.310216]  ? __switch_to_asm+0x40/0x70
      [  962.310221]  ? __switch_to_asm+0x34/0x70
      [  962.310226]  ? __switch_to_asm+0x40/0x70
      [  962.310231]  ? __switch_to_asm+0x34/0x70
      [  962.310236]  ? __switch_to_asm+0x40/0x70
      [  962.310242]  ? syscall_return_via_sysret+0xf/0x7f
      [  962.310248]  ? __switch_to_asm+0x34/0x70
      [  962.310253]  ? __switch_to_asm+0x40/0x70
      [  962.310258]  ? __switch_to_asm+0x34/0x70
      [  962.310263]  ? __switch_to_asm+0x40/0x70
      [  962.310268]  ? __switch_to_asm+0x34/0x70
      [  962.310273]  ? __switch_to_asm+0x40/0x70
      [  962.310281]  ? __schedule+0x87f/0x1e80
      [  962.310292]  ? __sched_text_start+0x8/0x8
      [  962.310300]  ? save_stack+0x8c/0xb0
      [  962.310308]  ? __kasan_kmalloc.constprop.6+0xc6/0xd0
      [  962.310313]  ? kthread+0x98/0x3a0
      [  962.310318]  ? ret_from_fork+0x35/0x40
      [  962.310334]  ? __wake_up_common+0x178/0x6f0
      [  962.310343]  ? _raw_spin_lock_irqsave+0xa4/0x140
      [  962.310349]  ? __lock_text_start+0x8/0x8
      [  962.310355]  ? _raw_write_lock_irqsave+0x70/0x130
      [  962.310360]  ? __lock_text_start+0x8/0x8
      [  962.310371]  ? process_one_work+0x1400/0x1400
      [  962.310376]  kthread+0x2e2/0x3a0
      [  962.310383]  ? kthread_create_on_node+0xc0/0xc0
      [  962.310389]  ret_from_fork+0x35/0x40
      
      [  962.310401] Allocated by task 1462:
      [  962.310410]  __kasan_kmalloc.constprop.6+0xc6/0xd0
      [  962.310437]  drm_dp_add_port+0xd60/0x1960 [drm_kms_helper]
      [  962.310464]  drm_dp_send_link_address+0x4b0/0x770 [drm_kms_helper]
      [  962.310491]  drm_dp_check_and_send_link_address+0x197/0x1f0 [drm_kms_helper]
      [  962.310515]  drm_dp_mst_link_probe_work+0x2b6/0x330 [drm_kms_helper]
      [  962.310522]  process_one_work+0x884/0x1400
      [  962.310529]  worker_thread+0x196/0x11e0
      [  962.310533]  kthread+0x2e2/0x3a0
      [  962.310538]  ret_from_fork+0x35/0x40
      
      [  962.310543] Freed by task 500:
      [  962.310550]  __kasan_slab_free+0x133/0x180
      [  962.310555]  kfree+0x92/0x1a0
      [  962.310581]  drm_dp_mst_put_port_malloc+0x14d/0x180 [drm_kms_helper]
      [  962.310693]  intel_connector_destroy+0xb2/0xe0 [i915]
      [  962.310747]  drm_mode_object_put.part.0+0x12b/0x1a0 [drm]
      [  962.310802]  drm_atomic_state_default_clear+0x1f2/0xcc0 [drm]
      [  962.310916]  intel_atomic_state_clear+0xe/0x80 [i915]
      [  962.310972]  __drm_atomic_state_free+0x35/0xd0 [drm]
      [  962.311083]  intel_atomic_cleanup_work+0x56/0x70 [i915]
      [  962.311092]  process_one_work+0x884/0x1400
      [  962.311098]  worker_thread+0x196/0x11e0
      [  962.311103]  kthread+0x2e2/0x3a0
      [  962.311108]  ret_from_fork+0x35/0x40
      
      [  962.311116] The buggy address belongs to the object at ffff888416c30000
                      which belongs to the cache kmalloc-2k of size 2048
      [  962.311122] The buggy address is located 4 bytes inside of
                      2048-byte region [ffff888416c30000, ffff888416c30800)
      [  962.311124] The buggy address belongs to the page:
      [  962.311132] page:ffffea00105b0c00 count:1 mapcount:0 mapping:ffff88841d003040 index:0x0 compound_mapcount: 0
      [  962.311142] flags: 0x8000000000010200(slab|head)
      [  962.311152] raw: 8000000000010200 dead000000000100 dead000000000200 ffff88841d003040
      [  962.311159] raw: 0000000000000000 00000000000f000f 00000001ffffffff 0000000000000000
      [  962.311162] page dumped because: kasan: bad access detected
      
      So, bail early if drm_dp_mst_deallocate_vcpi() is called on a port with
      no VCPI allocation. Additionally, clean up the surrounding kerneldoc
      while we're at it since the port is assumed to be kept around because
      the DRM driver is expected to hold a malloc reference to it, not just
      us.
      
      Changes since v1:
      * Doc changes - danvet
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Fixes: eceae147 ("drm/dp_mst: Start tracking per-port VCPI allocations")
      Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190202002023.29665-2-lyude@redhat.com
      3a8844c2
  8. 04 2月, 2019 1 次提交
  9. 31 1月, 2019 2 次提交
  10. 24 1月, 2019 1 次提交
  11. 11 1月, 2019 12 次提交
    • L
      drm/dp_mst: Check payload count in drm_dp_mst_atomic_check() · 5e187a01
      Lyude Paul 提交于
      It occurred to me that we never actually check this! So let's start
      doing that.
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Cc: David Airlie <airlied@redhat.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Harry Wentland <harry.wentland@amd.com>
      Cc: Juston Li <juston.li@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-20-lyude@redhat.com
      5e187a01
    • L
      drm/dp_mst: Start tracking per-port VCPI allocations · eceae147
      Lyude Paul 提交于
      There has been a TODO waiting for quite a long time in
      drm_dp_mst_topology.c:
      
      	/* We cannot rely on port->vcpi.num_slots to update
      	 * topology_state->avail_slots as the port may not exist if the parent
      	 * branch device was unplugged. This should be fixed by tracking
      	 * per-port slot allocation in drm_dp_mst_topology_state instead of
      	 * depending on the caller to tell us how many slots to release.
      	 */
      
      That's not the only reason we should fix this: forcing the driver to
      track the VCPI allocations throughout a state's atomic check is
      error prone, because it means that extra care has to be taken with the
      order that drm_dp_atomic_find_vcpi_slots() and
      drm_dp_atomic_release_vcpi_slots() are called in in order to ensure
      idempotency. Currently the only driver actually using these helpers,
      i915, doesn't even do this correctly: multiple ->best_encoder() checks
      with i915's current implementation would not be idempotent and would
      over-allocate VCPI slots, something I learned trying to implement
      fallback retraining in MST.
      
      So: simplify this whole mess, and teach drm_dp_atomic_find_vcpi_slots()
      and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations for
      each port. This allows us to ensure idempotency without having to rely
      on the driver as much. Additionally: the driver doesn't need to do any
      kind of VCPI slot tracking anymore if it doesn't need it for it's own
      internal state.
      
      Additionally; this adds a new drm_dp_mst_atomic_check() helper which
      must be used by atomic drivers to perform validity checks for the new
      VCPI allocations incurred by a state.
      
      Also: update the documentation and make it more obvious that these
      /must/ be called by /all/ atomic drivers supporting MST.
      
      Changes since v9:
      * Add some missing changes that were requested by danvet that I forgot
        about after I redid all of the kref stuff:
        * Remove unnecessary state changes in intel_dp_mst_atomic_check
        * Cleanup atomic check logic for VCPI allocations - all we need to check in
          compute_config is whether or not this state disables a CRTC, then free
          VCPI based off that
      
      Changes since v8:
       * Fix compile errors, whoops!
      
      Changes since v7:
       - Don't check for mixed stale/valid VCPI allocations, just rely on
       connector registration to stop such erroneous modesets
      
      Changes since v6:
       - Keep a kref to all of the ports we have allocations on. This required
         a good bit of changing to when we call drm_dp_find_vcpi_slots(),
         mainly that we need to ensure that we only redo VCPI allocations on
         actual mode or CRTC changes, not crtc_state->active changes.
         Additionally, we no longer take the registration of the DRM connector
         for each port into account because so long as we have a kref to the
         port in the new or previous atomic state, the connector will stay
         registered.
       - Use the small changes to drm_dp_put_port() to add even more error
         checking to make misusage of the helpers more obvious. I added this
         after having to chase down various use-after-free conditions that
         started popping up from the new helpers so no one else has to
         troubleshoot that.
       - Move some accidental DRM_DEBUG_KMS() calls to DRM_DEBUG_ATOMIC()
       - Update documentation again, note that find/release() should both not be
         called on the same port in a single atomic check phase (but multiple
         calls to one or the other is OK)
      
      Changes since v4:
       - Don't skip the atomic checks for VCPI allocations if no new VCPI
         allocations happen in a state. This makes the next change I'm about
         to list here a lot easier to implement.
       - Don't ignore VCPI allocations on destroyed ports, instead ensure that
         when ports are destroyed and still have VCPI allocations in the
         topology state, the only state changes allowed are releasing said
         ports' VCPI. This prevents a state with a mix of VCPI allocations
         from destroyed ports, and allocations from valid ports.
      
      Changes since v3:
       - Don't release VCPI allocations in the topology state immediately in
         drm_dp_atomic_release_vcpi_slots(), instead mark them as 0 and skip
         over them in drm_dp_mst_duplicate_state(). This makes it so
         drm_dp_atomic_release_vcpi_slots() is still idempotent while also
         throwing warnings if the driver messes up it's book keeping and tries
         to release VCPI slots on a port that doesn't have any pre-existing
         VCPI allocation - danvet
       - Change mst_state/state in some debugging messages to "mst state"
      
      Changes since v2:
       - Use kmemdup() for duplicating MST state - danvet
       - Move port validation out of duplicate state callback - danvet
       - Handle looping through MST topology states in
         drm_dp_mst_atomic_check() so the driver doesn't have to do it
       - Fix documentation in drm_dp_atomic_find_vcpi_slots()
       - Move the atomic check for each individual topology state into it's
         own function, reduces indenting
       - Don't consider "stale" MST ports when calculating the bandwidth
         requirements. This is needed because originally we relied on the
         state duplication functions to prune any stale ports from the new
         state, which would prevent us from incorrectly considering their
         bandwidth requirements alongside legitimate new payloads.
       - Add function references in drm_dp_atomic_release_vcpi_slots() - danvet
       - Annotate atomic VCPI and atomic check functions with __must_check
         - danvet
      
      Changes since v1:
       - Don't use the now-removed ->atomic_check() for private objects hook,
         just give drivers a function to call themselves
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Reviewed-by: NDaniel Vetter <daniel@ffwll.ch>
      Cc: David Airlie <airlied@redhat.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Harry Wentland <harry.wentland@amd.com>
      Cc: Juston Li <juston.li@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-19-lyude@redhat.com
      eceae147
    • L
      drm/dp_mst: Add some atomic state iterator macros · bea5c38f
      Lyude Paul 提交于
      Changes since v6:
       - Move EXPORT_SYMBOL() for drm_dp_mst_topology_state_funcs to this
         commit
       - Document __drm_dp_mst_state_iter_get() and note that it shouldn't be
         called directly
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Cc: David Airlie <airlied@redhat.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Harry Wentland <harry.wentland@amd.com>
      Cc: Juston Li <juston.li@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-18-lyude@redhat.com
      bea5c38f
    • L
      drm/dp_mst: Fix payload deallocation on hotplugs using malloc refs · cfe9f903
      Lyude Paul 提交于
      Up until now, freeing payloads on remote MST hubs that just had ports
      removed has almost never worked because we've been relying on port
      validation in order to stop us from accessing ports that have already
      been freed from memory, but ports which need their payloads released due
      to being removed will never be a valid part of the topology after
      they've been removed.
      
      Since we've introduced malloc refs, we can replace all of the validation
      logic in payload helpers which are used for deallocation with some
      well-placed malloc krefs. This ensures that regardless of whether or not
      the ports are still valid and in the topology, any port which has an
      allocated payload will remain allocated in memory until it's payloads
      have been removed - finally allowing us to actually release said
      payloads correctly.
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
      Reviewed-by: NHarry Wentland <harry.wentland@amd.com>
      Cc: David Airlie <airlied@redhat.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Juston Li <juston.li@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-10-lyude@redhat.com
      cfe9f903
    • L
      drm/dp_mst: Stop releasing VCPI when removing ports from topology · a68f9917
      Lyude Paul 提交于
      This has never actually worked, and isn't needed anyway: the driver's
      always going to try to deallocate VCPI when it tears down the display
      that the VCPI belongs to.
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Reviewed-by: NHarry Wentland <harry.wentland@amd.com>
      Reviewed-by: NDaniel Vetter <daniel@ffwll.ch>
      Cc: David Airlie <airlied@redhat.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Juston Li <juston.li@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-9-lyude@redhat.com
      a68f9917
    • L
      drm/dp_mst: Restart last_connected_port_and_mstb() if topology ref fails · 56d1c14e
      Lyude Paul 提交于
      While this isn't a complete fix, this will improve the reliability of
      drm_dp_get_last_connected_port_and_mstb() pretty significantly during
      hotplug events, since there's a chance that the in-memory topology tree
      may not be fully updated when drm_dp_get_last_connected_port_and_mstb()
      is called and thus might end up causing our search to fail on an mstb
      whose topology refcount has reached 0, but has not yet been removed from
      it's parent.
      
      Ideally, we should further fix this problem by ensuring that we deal
      with the potential for racing with a hotplug event, which would look
      like this:
      
      * drm_dp_payload_send_msg() retrieves the last living relative of mstb
        with drm_dp_get_last_connected_port_and_mstb()
      * drm_dp_payload_send_msg() starts building payload message
        At the same time, mstb gets unplugged from the topology and is no
        longer the actual last living relative of the original mstb
      * drm_dp_payload_send_msg() tries sending the payload message, hub times
        out
      * Hub timed out, we give up and run away-resulting in the payload being
        leaked
      
      This could be fixed by restarting the
      drm_dp_get_last_connected_port_and_mstb() search whenever we get a
      timeout, sending the payload to the new mstb, then repeating until
      either the entire topology is removed from the system or
      drm_dp_get_last_connected_port_and_mstb() fails. But since the above
      race condition is not terribly likely, we'll address that in a later
      patch series once we've improved the recovery handling for VCPI
      allocations in the rest of the DP MST helpers.
      
      Changes since v1:
      * Convert kerneldoc for drm_dp_get_last_connected_port_and_mstb to
        normal comment - danvet
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Reviewed-by: NHarry Wentland <harry.wentland@amd.com>
      Reviewed-by: NDaniel Vetter <daniel@ffwll.ch>
      Cc: David Airlie <airlied@redhat.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Juston Li <juston.li@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-8-lyude@redhat.com
      56d1c14e
    • L
      drm/dp_mst: Introduce new refcounting scheme for mstbs and ports · ebcc0e6b
      Lyude Paul 提交于
      The current way of handling refcounting in the DP MST helpers is really
      confusing and probably just plain wrong because it's been hacked up many
      times over the years without anyone actually going over the code and
      seeing if things could be simplified.
      
      To the best of my understanding, the current scheme works like this:
      drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When
      this refcount hits 0 for either of the two, they're removed from the
      topology state, but not immediately freed. Both ports and branch devices
      will reinitialize their kref once it's hit 0 before actually destroying
      themselves. The intended purpose behind this is so that we can avoid
      problems like not being able to free a remote payload that might still
      be active, due to us having removed all of the port/branch device
      structures in memory, as per:
      
      commit 91a25e46 ("drm/dp/mst: deallocate payload on port destruction")
      
      Which may have worked, but then it caused use-after-free errors. Being
      new to MST at the time, I tried fixing it;
      
      commit 263efde3 ("drm/dp/mst: Get validated port ref in drm_dp_update_payload_part1()")
      
      But, that was broken: both drm_dp_mst_port and drm_dp_mst_branch structs
      are validated in almost every DP MST helper function. Simply put, this
      means we go through the topology and try to see if the given
      drm_dp_mst_branch or drm_dp_mst_port is still attached to something
      before trying to use it in order to avoid dereferencing freed memory
      (something that has happened a LOT in the past with this library).
      Because of this it doesn't actually matter whether or not we keep keep
      the ports and branches around in memory as that's not enough, because
      any function that validates the branches and ports passed to it will
      still reject them anyway since they're no longer in the topology
      structure. So, use-after-free errors were fixed but payload deallocation
      was completely broken.
      
      Two years later, AMD informed me about this issue and I attempted to
      come up with a temporary fix, pending a long-overdue cleanup of this
      library:
      
      commit c54c7374 ("drm/dp_mst: Skip validating ports during destruction, just ref")
      
      But then that introduced use-after-free errors, so I quickly reverted
      it:
      
      commit 9765635b ("Revert "drm/dp_mst: Skip validating ports during destruction, just ref"")
      
      And in the process, learned that there is just no simple fix for this:
      the design is just broken. Unfortunately, the usage of these helpers are
      quite broken as well. Some drivers like i915 have been smart enough to
      avoid accessing any kind of information from MST port structures, but
      others like nouveau have assumed, understandably so, that
      drm_dp_mst_port structures are normal and can just be accessed at any
      time without worrying about use-after-free errors.
      
      After a lot of discussion, me and Daniel Vetter came up with a better
      idea to replace all of this.
      
      To summarize, since this is documented far more indepth in the
      documentation this patch introduces, we make it so that drm_dp_mst_port
      and drm_dp_mst_branch structures have two different classes of
      refcounts: topology_kref, and malloc_kref. topology_kref corresponds to
      the lifetime of the given drm_dp_mst_port or drm_dp_mst_branch in it's
      given topology. Once it hits zero, any associated connectors are removed
      and the branch or port can no longer be validated. malloc_kref
      corresponds to the lifetime of the memory allocation for the actual
      structure, and will always be non-zero so long as the topology_kref is
      non-zero. This gives us a way to allow callers to hold onto port and
      branch device structures past their topology lifetime, and dramatically
      simplifies the lifetimes of both structures. This also finally fixes the
      port deallocation problem, properly.
      
      Additionally: since this now means that we can keep ports and branch
      devices allocated in memory for however long we need, we no longer need
      a significant amount of the port validation that we currently do.
      
      Additionally, there is one last scenario that this fixes, which couldn't
      have been fixed properly beforehand:
      
      - CPU1 unrefs port from topology (refcount 1->0)
      - CPU2 refs port in topology(refcount 0->1)
      
      Since we now can guarantee memory safety for ports and branches
      as-needed, we also can make our main reference counting functions fix
      this problem by using kref_get_unless_zero() internally so that topology
      refcounts can only ever reach 0 once.
      
      Changes since v4:
      * Change the kernel-figure summary for dp-mst/topology-figure-1.dot a
        bit - danvet
      * Remove figure numbers - danvet
      
      Changes since v3:
      * Remove rebase detritus - danvet
      * Split out purely style changes into separate patches - hwentlan
      
      Changes since v2:
      * Fix commit message - checkpatch
      * s/)-1/) - 1/g - checkpatch
      
      Changes since v1:
      * Remove forward declarations - danvet
      * Move "Branch device and port refcounting" section from documentation
        into kernel-doc comments - danvet
      * Export internal topology lifetime functions into their own section in
        the kernel-docs - danvet
      * s/@/&/g for struct references in kernel-docs - danvet
      * Drop the "when they are no longer being used" bits from the kernel
        docs - danvet
      * Modify diagrams to show how the DRM driver interacts with the topology
        and payloads - danvet
      * Make suggested documentation changes for
        drm_dp_mst_topology_get_mstb() and drm_dp_mst_topology_get_port() -
        danvet
      * Better explain the relationship between malloc refs and topology krefs
        in the documentation for drm_dp_mst_topology_get_port() and
        drm_dp_mst_topology_get_mstb() - danvet
      * Fix "See also" in drm_dp_mst_topology_get_mstb() - danvet
      * Rename drm_dp_mst_topology_get_(port|mstb)() ->
        drm_dp_mst_topology_try_get_(port|mstb)() and
        drm_dp_mst_topology_ref_(port|mstb)() ->
        drm_dp_mst_topology_get_(port|mstb)() - danvet
      * s/should/must in docs - danvet
      * WARN_ON(refcount == 0) in topology_get_(mstb|port) - danvet
      * Move kdocs for mstb/port structs inline - danvet
      * Split drm_dp_get_last_connected_port_and_mstb() changes into their own
        commit - danvet
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Reviewed-by: NHarry Wentland <harry.wentland@amd.com>
      Reviewed-by: NDaniel Vetter <daniel@ffwll.ch>
      Cc: David Airlie <airlied@redhat.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Juston Li <juston.li@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-7-lyude@redhat.com
      ebcc0e6b
    • L
      drm/dp_mst: Rename drm_dp_mst_get_validated_(port|mstb)_ref and friends · d0757afd
      Lyude Paul 提交于
      s/drm_dp_get_validated_port_ref/drm_dp_mst_topology_get_port_validated/
      s/drm_dp_put_port/drm_dp_mst_topology_put_port/
      s/drm_dp_get_validated_mstb_ref/drm_dp_mst_topology_get_mstb_validated/
      s/drm_dp_put_mst_branch_device/drm_dp_mst_topology_put_mstb/
      
      This is a much more consistent naming scheme, and will make even more
      sense once we redesign how the current refcounting scheme here works.
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Reviewed-by: NDaniel Vetter <daniel@ffwll.ch>
      Reviewed-by: NHarry Wentland <harry.wentland@amd.com>
      Cc: David Airlie <airlied@redhat.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Juston Li <juston.li@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-6-lyude@redhat.com
      d0757afd
    • L
      drm/dp_mst: Fix some formatting in drm_dp_mst_deallocate_vcpi() · 4afb8a26
      Lyude Paul 提交于
      Split some stuff across multiple lines
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Reviewed-by: NHarry Wentland <harry.wentland@amd.com>
      Reviewed-by: NDaniel Vetter <daniel@ffwll.ch>
      Cc: David Airlie <airlied@redhat.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Juston Li <juston.li@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-5-lyude@redhat.com
      4afb8a26
    • L
      drm/dp_mst: Fix some formatting in drm_dp_mst_allocate_vcpi() · e0ac7113
      Lyude Paul 提交于
      Fix some indenting, split some stuff across multiple lines.
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Reviewed-by: NHarry Wentland <harry.wentland@amd.com>
      Reviewed-by: NDaniel Vetter <daniel@ffwll.ch>
      Cc: David Airlie <airlied@redhat.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Juston Li <juston.li@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-4-lyude@redhat.com
      e0ac7113
    • L
      drm/dp_mst: Fix some formatting in drm_dp_payload_send_msg() · de6d6818
      Lyude Paul 提交于
      Split some stuff across multiple lines, remove some unnecessary braces
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Reviewed-by: NHarry Wentland <harry.wentland@amd.com>
      Reviewed-by: NDaniel Vetter <daniel@ffwll.ch>
      Cc: David Airlie <airlied@redhat.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Juston Li <juston.li@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-3-lyude@redhat.com
      de6d6818
    • L
      drm/dp_mst: Fix some formatting in drm_dp_add_port() · 3d76df63
      Lyude Paul 提交于
      Reindent some stuff, and split some stuff across multiple lines so we
      aren't going over the text width limit.
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Reviewed-by: NHarry Wentland <harry.wentland@amd.com>
      Reviewed-by: NDaniel Vetter <daniel@ffwll.ch>
      Cc: David Airlie <airlied@redhat.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Juston Li <juston.li@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-2-lyude@redhat.com
      3d76df63
  12. 15 12月, 2018 2 次提交
  13. 14 12月, 2018 1 次提交
  14. 11 12月, 2018 3 次提交
  15. 10 12月, 2018 1 次提交
  16. 29 11月, 2018 1 次提交
    • L
      Revert "drm/dp_mst: Skip validating ports during destruction, just ref" · 9765635b
      Lyude Paul 提交于
      This reverts commit:
      
      c54c7374 ("drm/dp_mst: Skip validating ports during destruction, just ref")
      
      ugh.
      
      In drm_dp_destroy_connector_work(), we have a pretty good chance of
      freeing the actual struct drm_dp_mst_port. However, after destroying
      things we send a hotplug through (*mgr->cbs->hotplug)(mgr) which is
      where the problems start.
      
      For i915, this calls all the way down to the fbcon probing helpers,
      which start trying to access the port in a modeset.
      
      [   45.062001] ==================================================================
      [   45.062112] BUG: KASAN: use-after-free in ex_handler_refcount+0x146/0x180
      [   45.062196] Write of size 4 at addr ffff8882b4b70968 by task kworker/3:1/53
      
      [   45.062325] CPU: 3 PID: 53 Comm: kworker/3:1 Kdump: loaded Tainted: G           O      4.20.0-rc4Lyude-Test+ #3
      [   45.062442] Hardware name: LENOVO 20BWS1KY00/20BWS1KY00, BIOS JBET71WW (1.35 ) 09/14/2018
      [   45.062554] Workqueue: events drm_dp_destroy_connector_work [drm_kms_helper]
      [   45.062641] Call Trace:
      [   45.062685]  dump_stack+0xbd/0x15a
      [   45.062735]  ? dump_stack_print_info.cold.0+0x1b/0x1b
      [   45.062801]  ? printk+0x9f/0xc5
      [   45.062847]  ? kmsg_dump_rewind_nolock+0xe4/0xe4
      [   45.062909]  ? ex_handler_refcount+0x146/0x180
      [   45.062970]  print_address_description+0x71/0x239
      [   45.063036]  ? ex_handler_refcount+0x146/0x180
      [   45.063095]  kasan_report.cold.5+0x242/0x30b
      [   45.063155]  __asan_report_store4_noabort+0x1c/0x20
      [   45.063313]  ex_handler_refcount+0x146/0x180
      [   45.063371]  ? ex_handler_clear_fs+0xb0/0xb0
      [   45.063428]  fixup_exception+0x98/0xd7
      [   45.063484]  ? raw_notifier_call_chain+0x20/0x20
      [   45.063548]  do_trap+0x6d/0x210
      [   45.063605]  ? _GLOBAL__sub_I_65535_1_drm_dp_aux_unregister_devnode+0x2f/0x1c6 [drm_kms_helper]
      [   45.063732]  do_error_trap+0xc0/0x170
      [   45.063802]  ? _GLOBAL__sub_I_65535_1_drm_dp_aux_unregister_devnode+0x2f/0x1c6 [drm_kms_helper]
      [   45.063929]  do_invalid_op+0x3b/0x50
      [   45.063997]  ? _GLOBAL__sub_I_65535_1_drm_dp_aux_unregister_devnode+0x2f/0x1c6 [drm_kms_helper]
      [   45.064103]  invalid_op+0x14/0x20
      [   45.064162] RIP: 0010:_GLOBAL__sub_I_65535_1_drm_dp_aux_unregister_devnode+0x2f/0x1c6 [drm_kms_helper]
      [   45.064274] Code: 00 48 c7 c7 80 fe 53 a0 48 89 e5 e8 5b 6f 26 e1 5d c3 48 8d 0e 0f 0b 48 8d 0b 0f 0b 48 8d 0f 0f 0b 48 8d 0f 0f 0b 49 8d 4d 00 <0f> 0b 49 8d 0e 0f 0b 48 8d 08 0f 0b 49 8d 4d 00 0f 0b 48 8d 0b 0f
      [   45.064569] RSP: 0018:ffff8882b789ee10 EFLAGS: 00010282
      [   45.064637] RAX: ffff8882af47ae70 RBX: ffff8882af47aa60 RCX: ffff8882b4b70968
      [   45.064723] RDX: ffff8882af47ae70 RSI: 0000000000000008 RDI: ffff8882b788bdb8
      [   45.064808] RBP: ffff8882b789ee28 R08: ffffed1056f13db4 R09: ffffed1056f13db3
      [   45.064894] R10: ffffed1056f13db3 R11: ffff8882b789ed9f R12: ffff8882af47ad28
      [   45.064980] R13: ffff8882b4b70968 R14: ffff8882acd86728 R15: ffff8882b4b75dc8
      [   45.065084]  drm_dp_mst_reset_vcpi_slots+0x12/0x80 [drm_kms_helper]
      [   45.065225]  intel_mst_disable_dp+0xda/0x180 [i915]
      [   45.065361]  intel_encoders_disable.isra.107+0x197/0x310 [i915]
      [   45.065498]  haswell_crtc_disable+0xbe/0x400 [i915]
      [   45.065622]  ? i9xx_disable_plane+0x1c0/0x3e0 [i915]
      [   45.065750]  intel_atomic_commit_tail+0x74e/0x3e60 [i915]
      [   45.065884]  ? intel_pre_plane_update+0xbc0/0xbc0 [i915]
      [   45.065968]  ? drm_atomic_helper_swap_state+0x88b/0x1d90 [drm_kms_helper]
      [   45.066054]  ? kasan_check_write+0x14/0x20
      [   45.066165]  ? i915_gem_track_fb+0x13a/0x330 [i915]
      [   45.066277]  ? i915_sw_fence_complete+0xe9/0x140 [i915]
      [   45.066406]  ? __i915_sw_fence_complete+0xc50/0xc50 [i915]
      [   45.066540]  intel_atomic_commit+0x72e/0xef0 [i915]
      [   45.066635]  ? drm_dev_dbg+0x200/0x200 [drm]
      [   45.066764]  ? intel_atomic_commit_tail+0x3e60/0x3e60 [i915]
      [   45.066898]  ? intel_atomic_commit_tail+0x3e60/0x3e60 [i915]
      [   45.067001]  drm_atomic_commit+0xc4/0xf0 [drm]
      [   45.067074]  restore_fbdev_mode_atomic+0x562/0x780 [drm_kms_helper]
      [   45.067166]  ? drm_fb_helper_debug_leave+0x690/0x690 [drm_kms_helper]
      [   45.067249]  ? kasan_check_read+0x11/0x20
      [   45.067324]  restore_fbdev_mode+0x127/0x4b0 [drm_kms_helper]
      [   45.067364]  ? kasan_check_read+0x11/0x20
      [   45.067406]  drm_fb_helper_restore_fbdev_mode_unlocked+0x164/0x200 [drm_kms_helper]
      [   45.067462]  ? drm_fb_helper_hotplug_event+0x30/0x30 [drm_kms_helper]
      [   45.067508]  ? kasan_check_write+0x14/0x20
      [   45.070360]  ? mutex_unlock+0x22/0x40
      [   45.073748]  drm_fb_helper_set_par+0xb2/0xf0 [drm_kms_helper]
      [   45.075846]  drm_fb_helper_hotplug_event.part.33+0x1cd/0x290 [drm_kms_helper]
      [   45.078088]  drm_fb_helper_hotplug_event+0x1c/0x30 [drm_kms_helper]
      [   45.082614]  intel_fbdev_output_poll_changed+0x9f/0x140 [i915]
      [   45.087069]  drm_kms_helper_hotplug_event+0x67/0x90 [drm_kms_helper]
      [   45.089319]  intel_dp_mst_hotplug+0x37/0x50 [i915]
      [   45.091496]  drm_dp_destroy_connector_work+0x510/0x6f0 [drm_kms_helper]
      [   45.093675]  ? drm_dp_update_payload_part1+0x1220/0x1220 [drm_kms_helper]
      [   45.095851]  ? kasan_check_write+0x14/0x20
      [   45.098473]  ? kasan_check_read+0x11/0x20
      [   45.101155]  ? strscpy+0x17c/0x530
      [   45.103808]  ? __switch_to_asm+0x34/0x70
      [   45.106456]  ? syscall_return_via_sysret+0xf/0x7f
      [   45.109711]  ? read_word_at_a_time+0x20/0x20
      [   45.113138]  ? __switch_to_asm+0x40/0x70
      [   45.116529]  ? __switch_to_asm+0x34/0x70
      [   45.119891]  ? __switch_to_asm+0x40/0x70
      [   45.123224]  ? __switch_to_asm+0x34/0x70
      [   45.126540]  ? __switch_to_asm+0x34/0x70
      [   45.129824]  process_one_work+0x88d/0x15d0
      [   45.133172]  ? pool_mayday_timeout+0x850/0x850
      [   45.136459]  ? pci_mmcfg_check_reserved+0x110/0x128
      [   45.139739]  ? wake_q_add+0xb0/0xb0
      [   45.143010]  ? check_preempt_wakeup+0x652/0x1050
      [   45.146304]  ? worker_enter_idle+0x29e/0x740
      [   45.149589]  ? __schedule+0x1ec0/0x1ec0
      [   45.152937]  ? kasan_check_read+0x11/0x20
      [   45.156179]  ? _raw_spin_lock_irq+0xa3/0x130
      [   45.159382]  ? _raw_read_unlock_irqrestore+0x30/0x30
      [   45.162542]  ? kasan_check_write+0x14/0x20
      [   45.165657]  worker_thread+0x1a5/0x1470
      [   45.168725]  ? set_load_weight+0x2e0/0x2e0
      [   45.171755]  ? process_one_work+0x15d0/0x15d0
      [   45.174806]  ? __switch_to_asm+0x34/0x70
      [   45.177645]  ? __switch_to_asm+0x40/0x70
      [   45.180323]  ? __switch_to_asm+0x34/0x70
      [   45.182936]  ? __switch_to_asm+0x40/0x70
      [   45.185539]  ? __switch_to_asm+0x34/0x70
      [   45.188100]  ? __switch_to_asm+0x40/0x70
      [   45.190628]  ? __schedule+0x7d4/0x1ec0
      [   45.193143]  ? save_stack+0xa9/0xd0
      [   45.195632]  ? kasan_check_write+0x10/0x20
      [   45.198162]  ? kasan_kmalloc+0xc4/0xe0
      [   45.200609]  ? kmem_cache_alloc_trace+0xdd/0x190
      [   45.203046]  ? kthread+0x9f/0x3b0
      [   45.205470]  ? ret_from_fork+0x35/0x40
      [   45.207876]  ? unwind_next_frame+0x43/0x50
      [   45.210273]  ? __save_stack_trace+0x82/0x100
      [   45.212658]  ? deactivate_slab.isra.67+0x3d4/0x580
      [   45.215026]  ? default_wake_function+0x35/0x50
      [   45.217399]  ? kasan_check_read+0x11/0x20
      [   45.219825]  ? _raw_spin_lock_irqsave+0xae/0x140
      [   45.222174]  ? __lock_text_start+0x8/0x8
      [   45.224521]  ? replenish_dl_entity.cold.62+0x4f/0x4f
      [   45.226868]  ? __kthread_parkme+0x87/0xf0
      [   45.229200]  kthread+0x2f7/0x3b0
      [   45.231557]  ? process_one_work+0x15d0/0x15d0
      [   45.233923]  ? kthread_park+0x120/0x120
      [   45.236249]  ret_from_fork+0x35/0x40
      
      [   45.240875] Allocated by task 242:
      [   45.243136]  save_stack+0x43/0xd0
      [   45.245385]  kasan_kmalloc+0xc4/0xe0
      [   45.247597]  kmem_cache_alloc_trace+0xdd/0x190
      [   45.249793]  drm_dp_add_port+0x1e0/0x2170 [drm_kms_helper]
      [   45.252000]  drm_dp_send_link_address+0x4a7/0x740 [drm_kms_helper]
      [   45.254389]  drm_dp_check_and_send_link_address+0x1a7/0x210 [drm_kms_helper]
      [   45.256803]  drm_dp_mst_link_probe_work+0x6f/0xb0 [drm_kms_helper]
      [   45.259200]  process_one_work+0x88d/0x15d0
      [   45.261597]  worker_thread+0x1a5/0x1470
      [   45.264038]  kthread+0x2f7/0x3b0
      [   45.266371]  ret_from_fork+0x35/0x40
      
      [   45.270937] Freed by task 53:
      [   45.273170]  save_stack+0x43/0xd0
      [   45.275382]  __kasan_slab_free+0x139/0x190
      [   45.277604]  kasan_slab_free+0xe/0x10
      [   45.279826]  kfree+0x99/0x1b0
      [   45.282044]  drm_dp_free_mst_port+0x4a/0x60 [drm_kms_helper]
      [   45.284330]  drm_dp_destroy_connector_work+0x43e/0x6f0 [drm_kms_helper]
      [   45.286660]  process_one_work+0x88d/0x15d0
      [   45.288934]  worker_thread+0x1a5/0x1470
      [   45.291231]  kthread+0x2f7/0x3b0
      [   45.293547]  ret_from_fork+0x35/0x40
      
      [   45.298206] The buggy address belongs to the object at ffff8882b4b70968
                      which belongs to the cache kmalloc-2k of size 2048
      [   45.303047] The buggy address is located 0 bytes inside of
                      2048-byte region [ffff8882b4b70968, ffff8882b4b71168)
      [   45.308010] The buggy address belongs to the page:
      [   45.310477] page:ffffea000ad2dc00 count:1 mapcount:0 mapping:ffff8882c080cf40 index:0x0 compound_mapcount: 0
      [   45.313051] flags: 0x8000000000010200(slab|head)
      [   45.315635] raw: 8000000000010200 ffffea000aac2808 ffffea000abe8608 ffff8882c080cf40
      [   45.318300] raw: 0000000000000000 00000000000d000d 00000001ffffffff 0000000000000000
      [   45.320966] page dumped because: kasan: bad access detected
      
      [   45.326312] Memory state around the buggy address:
      [   45.329085]  ffff8882b4b70800: fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
      [   45.331845]  ffff8882b4b70880: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
      [   45.334584] >ffff8882b4b70900: fc fc fc fc fc fc fc fc fc fc fc fc fc fb fb fb
      [   45.337302]                                                           ^
      [   45.340061]  ffff8882b4b70980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
      [   45.342910]  ffff8882b4b70a00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
      [   45.345748] ==================================================================
      
      So, this definitely isn't a fix that we want. This being said; there's
      no real easy fix for this problem because of some of the catch-22's of
      the MST helpers current design. For starters; we always need to validate
      a port with drm_dp_get_validated_port_ref(), but validation relies on
      the lifetime of the port in the actual topology. So once the port is
      gone, it can't be validated again.
      
      If we were to try to make the payload helpers not use port validation,
      then we'd cause another problem: if the port isn't validated, it could
      be freed and we'd just start causing more KASAN issues. There are
      already hacks that attempt to workaround this in
      drm_dp_mst_destroy_connector_work() by re-initializing the kref so that
      it can be used again and it's memory can be freed once the VCPI helpers
      finish removing the port's respective payloads. But none of these really
      do anything helpful since the port still can't be validated since it's
      gone from the topology. Also, that workaround is immensely confusing to
      read through.
      
      What really needs to be done in order to fix this is to teach DRM how to
      track the lifetime of the structs for MST ports and branch devices
      separately from their lifetime in the actual topology. Simply put; this
      means having two different krefs-one that removes the port/branch device
      from the topology, and one that finally calls kfree(). This would let us
      simplify things, since we'd now be able to keep ports around without
      having to keep them in the topology at the same time, which is exactly
      what we need in order to teach our VCPI helpers to only validate ports
      when it's actually necessary without running the risk of trying to use
      unallocated memory.
      
      Such a fix is on it's way, but for now let's play it safe and just
      revert this. If this bug has been around for well over a year, we can
      wait a little while to get an actual proper fix here.
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Fixes: c54c7374 ("drm/dp_mst: Skip validating ports during destruction, just ref")
      Cc: Daniel Vetter <daniel@ffwll.ch>
      Cc: Sean Paul <sean@poorly.run>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Harry Wentland <Harry.Wentland@amd.com>
      Cc: stable@vger.kernel.org # v4.6+
      Acked-by: NSean Paul <sean@poorly.run>
      Link: https://patchwork.freedesktop.org/patch/msgid/20181128210005.24434-1-lyude@redhat.com
      9765635b
  17. 27 11月, 2018 1 次提交
    • L
      drm/dp_mst: Skip validating ports during destruction, just ref · c54c7374
      Lyude Paul 提交于
      Jerry Zuo pointed out a rather obscure hotplugging issue that it seems I
      accidentally introduced into DRM two years ago.
      
      Pretend we have a topology like this:
      
      |- DP-1: mst_primary
         |- DP-4: active display
         |- DP-5: disconnected
         |- DP-6: active hub
            |- DP-7: active display
            |- DP-8: disconnected
            |- DP-9: disconnected
      
      If we unplug DP-6, the topology starting at DP-7 will be destroyed but
      it's payloads will live on in DP-1's VCPI allocations and thus require
      removal. However, this removal currently fails because
      drm_dp_update_payload_part1() will (rightly so) try to validate the port
      before accessing it, fail then abort. If we keep going, eventually we
      run the MST hub out of bandwidth and all new allocations will start to
      fail (or in my case; all new displays just start flickering a ton).
      
      We could just teach drm_dp_update_payload_part1() not to drop the port
      ref in this case, but then we also need to teach
      drm_dp_destroy_payload_step1() to do the same thing, then hope no one
      ever adds anything to the that requires a validated port reference in
      drm_dp_destroy_connector_work(). Kind of sketchy.
      
      So let's go with a more clever solution: any port that
      drm_dp_destroy_connector_work() interacts with is guaranteed to still
      exist in memory until we say so. While said port might not be valid we
      don't really care: that's the whole reason we're destroying it in the
      first place! So, teach drm_dp_get_validated_port_ref() to use the all
      mighty current_work() function to avoid attempting to validate ports
      from the context of mgr->destroy_connector_work. I can't see any
      situation where this wouldn't be safe, and this avoids having to play
      whack-a-mole in the future of trying to work around port validation.
      Signed-off-by: NLyude Paul <lyude@redhat.com>
      Fixes: 263efde3 ("drm/dp/mst: Get validated port ref in drm_dp_update_payload_part1()")
      Reported-by: NJerry Zuo <Jerry.Zuo@amd.com>
      Cc: Jerry Zuo <Jerry.Zuo@amd.com>
      Cc: Harry Wentland <Harry.Wentland@amd.com>
      Cc: <stable@vger.kernel.org> # v4.6+
      Reviewed-by: NDave Airlie <airlied@redhat.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20181113224613.28809-1-lyude@redhat.comSigned-off-by: NSean Paul <seanpaul@chromium.org>
      c54c7374
  18. 10 11月, 2018 1 次提交
  19. 25 10月, 2018 1 次提交
  20. 31 8月, 2018 1 次提交
  21. 14 7月, 2018 1 次提交
  22. 28 3月, 2018 1 次提交
  23. 19 2月, 2018 1 次提交