Skip to content

L
linux
Switch branch/tag
  1. 24 Nov, 2019 2 commits
  3. 22 Nov, 2019 1 commit
  5. 21 Nov, 2019 3 commits
    • Daniel Vetter's avatar
      drm/msm: Don't init ww_mutec acquire ctx before needed · 92ec0767
      Daniel Vetter authored 
      For locking semantics it really doesn't matter when we grab the
ticket. But for lockdep validation it does: the acquire ctx is a fake
lockdep. Since other drivers might want to do a full multi-lock dance
in their fault-handler, not just lock a single dma_resv. Therefore we
must init the acquire_ctx only after we've done all the copy_*_user or
anything else that might trigger a pagefault. For msm this means we
need to move it past submit_lookup_objects.

Aside: Why is msm still using struct_mutex, it seems to be using
dma_resv_lock for general buffer state protection?

v2:
- Add comment to explain why the ww ticket setup is separate (Rob)
- Fix up error handling, we need to make sure we don't call
  ww_acquire_fini without _init.
Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-and-tested-by: default avatarRob Clark <robdclark@gmail.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Link: https://patchwork.freedesktop.org/patch/msgid/20191120105607.3023-1-daniel.vetter@ffwll.ch
      92ec0767
    • Daniel Vetter's avatar
      dma-resv: Also prime acquire ctx for lockdep · fedf7a44
      Daniel Vetter authored 
      Semnatically it really doesn't matter where we grab the ticket. But
since the ticket is a fake lockdep lock, it matters for lockdep
validation purposes.

This means stuff like grabbing a ticket and then doing
copy_from/to_user isn't allowed anymore. This is a changed compared to
the current ttm fault handler, which doesn't bother with having a full
reservation. Since I'm looking into fixing the TODO entry in
ttm_mem_evict_wait_busy() I think that'll have to change sooner or
later anyway, better get started. A bit more context on why I'm
looking into this: For backwards compat with existing i915 gem code I
think we'll have to do full slowpath locking in the i915 equivalent of
the eviction code. And with dynamic dma-buf that will leak across
drivers, so another thing we need to standardize and make sure it's
done the same way everyway.

Unfortunately this means another full audit of all drivers:

- gem helpers: acquire_init is done right before taking locks, so no
  problem. Same for acquire_fini and unlocking, which means nothing
  that's not already covered by the dma_resv_lock rules will be caught
  with this extension here to the acquire_ctx.

- etnaviv: An absolute massive amount of code is run between the
  acquire_init and the first lock acquisition in submit_lock_objects.
  But nothing that would touch user memory and could cause a fault.
  Furthermore nothing that uses the ticket, so even if I missed
  something, it would be easy to fix by pushing the acquire_init right
  before the first use. Similar on the unlock/acquire_fini side.

- i915: Right now (and this will likely change a lot rsn) the acquire
  ctx and actual locks are right next to each another. No problem.

- msm has a problem: submit_create calls acquire_init, but then
  submit_lookup_objects() has a bunch of copy_from_user to do the
  object lookups. That's the only thing before submit_lock_objects
  call dma_resv_lock(). Despite all the copypasta to etnaviv, etnaviv
  does not have this issue since it copies all the userspace structs
  earlier. submit_cleanup does not have any such issues.

  With the prep patch to pull out the acquire_ctx and reorder it msm
  is going to be safe too.

- nouveau: acquire_init is right next to ttm_bo_reserve, so all good.
  Similar on the acquire_fini/ttm_bo_unreserve side.

- ttm execbuf utils: acquire context and locking are even in the same
  functions here (one function to reserve everything, the other to
  unreserve), so all good.

- vc4: Another case where acquire context and locking are handled in
  the same functions (one function to lock everything, the other to
  unlock).

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Christian König <christian.koenig@amd.com>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: Huang Rui <ray.huang@amd.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Russell King <linux+etnaviv@armlinux.org.uk>
Cc: Christian Gmeiner <christian.gmeiner@gmail.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Acked-by: default avatarChristian König <christian.koenig@amd.com>
Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191119210844.16947-3-daniel.vetter@ffwll.ch
      fedf7a44
    • Daniel Vetter's avatar
      drm/modeset: Prime modeset lock vs dma_resv · 2c51419e
      Daniel Vetter authored 
      It's kinda really hard to get this wrong on a driver with both display
and dma_resv locking. But who ever knows, so better to make sure that
really all drivers nest these the same way.

For actual lock semantics the acquire context nesting doesn't matter.
But to teach lockdep what's going on with ww_mutex the acquire ctx is
a fake lockdep lock, hence from a lockdep pov it does matter. That's
why I figured better to include it.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Christian König <christian.koenig@amd.com>
Acked-by: default avatarChristian König <christian.koenig@amd.com>
Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191119210844.16947-2-daniel.vetter@ffwll.ch
      2c51419e
  7. 20 Nov, 2019 16 commits
  9. 19 Nov, 2019 8 commits
  11. 18 Nov, 2019 2 commits
  13. 15 Nov, 2019 4 commits
  15. 14 Nov, 2019 4 commits
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7