1. 25 Nov, 2019 20 commits
  2. 24 Nov, 2019 2 commits
  3. 22 Nov, 2019 1 commit
  4. 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
  5. 20 Nov, 2019 14 commits