1. 06 2月, 2019 2 次提交
    • 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
  2. 04 2月, 2019 1 次提交
  3. 31 1月, 2019 2 次提交
  4. 24 1月, 2019 1 次提交
  5. 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
  6. 15 12月, 2018 2 次提交
  7. 14 12月, 2018 1 次提交
  8. 11 12月, 2018 3 次提交
  9. 10 12月, 2018 1 次提交
  10. 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
  11. 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
  12. 10 11月, 2018 1 次提交
  13. 25 10月, 2018 1 次提交
  14. 31 8月, 2018 1 次提交
  15. 14 7月, 2018 1 次提交
  16. 28 3月, 2018 1 次提交
  17. 19 2月, 2018 1 次提交
  18. 26 1月, 2018 1 次提交
  19. 11 9月, 2017 1 次提交
  20. 20 7月, 2017 3 次提交
  21. 14 7月, 2017 2 次提交