1. 03 Oct, 2023 2 commits
    • Mathias Krause's avatar
      drm/i915: Clarify type evolution of uabi_node/uabi_engines · 9c303439
      Mathias Krause authored
      Chaining user engines happens in multiple passes during driver
      initialization, mutating its type along the way. It starts off with a
      simple lock-less linked list (struct llist_node/head) populated by
      intel_engine_add_user() which later gets sorted and converted to an
      intermediate regular list (struct list_head) just to be converted once
      more to its final rb-tree structure (struct rb_node/root) in
      intel_engines_driver_register().
      
      All of these types overlay the uabi_node/uabi_engines members which is
      unfortunate but safe if one takes care about using the rb-tree based
      structure only after the conversion has completed. However, mistakes
      happen and commit 1ec23ed7 ("drm/i915: Use uabi engines for the
      default engine map") violated that assumption, as the multiple type
      evolution was all to easy hidden behind casts papering over it.
      
      Make the type evolution of uabi_node/uabi_engines more visible by
      putting all members into an anonymous union and use the correctly typed
      member in its various users. This allows us to drop quite some ugly
      casts and, hopefully, make the evolution of the members better
      recognisable to avoid future mistakes.
      Signed-off-by: default avatarMathias Krause <minipli@grsecurity.net>
      Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
      Signed-off-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230928182019.10256-3-minipli@grsecurity.net
      9c303439
    • Mathias Krause's avatar
      drm/i915: Register engines early to avoid type confusion · 2b562f03
      Mathias Krause authored
      Commit 1ec23ed7 ("drm/i915: Use uabi engines for the default engine
      map") switched from using for_each_engine() to for_each_uabi_engine() to
      iterate over the user engines. While this seems to be a sensible change,
      it's only safe to do when the engines are actually chained using the
      rb-tree structure which is not the case during early driver
      initialization where it can be either a lock-less list or regular
      double-linked list.
      
      In fact, the modesetting initialization code may end up calling
      default_engines() through the fb helper code while the engines list
      is still llist_node-based:
      
        i915_driver_probe() ->
          intel_display_driver_probe() ->
            intel_fbdev_init() ->
              drm_fb_helper_init() ->
                drm_client_init() ->
                  drm_client_open() ->
                    drm_file_alloc() ->
                      i915_driver_open() ->
                        i915_gem_open() ->
                          i915_gem_context_open() ->
                            i915_gem_create_context() ->
                              default_engines()
      
      Using for_each_uabi_engine() in default_engines() is therefore wrong, as
      it would try to interpret the llist as rb-tree, making it find no engine
      at all, as the rb_left and rb_right members will still be NULL, as they
      haven't been initialized yet.
      
      To fix this type confusion register the engines earlier and at the same
      time reduce the amount of code that has to deal with the intermediate
      llist state.
      
      Reported-by: sanity checks in grsecurity
      Suggested-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
      Fixes: 1ec23ed7 ("drm/i915: Use uabi engines for the default engine map")
      Signed-off-by: default avatarMathias Krause <minipli@grsecurity.net>
      Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
      Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
      Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
      Signed-off-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230928182019.10256-2-minipli@grsecurity.net
      [tursulin: fixed commit tag typo]
      2b562f03
  2. 30 Sep, 2023 8 commits
  3. 29 Sep, 2023 4 commits
  4. 28 Sep, 2023 1 commit
  5. 27 Sep, 2023 1 commit
    • Tvrtko Ursulin's avatar
      drm/i915: Do not disable preemption for resets · 1e975e59
      Tvrtko Ursulin authored
      Commit ade8a0f5 ("drm/i915: Make all GPU resets atomic") added a
      preempt disable section over the hardware reset callback to prepare the
      driver for being able to reset from atomic contexts.
      
      In retrospect I can see that the work item at a time was about removing
      the struct mutex from the reset path. Code base also briefly entertained
      the idea of doing the reset under stop_machine in order to serialize
      userspace mmap and temporary glitch in the fence registers (see
      eb8d0f5a ("drm/i915: Remove GPU reset dependence on struct_mutex"),
      but that never materialized and was soon removed in 2caffbf1
      ("drm/i915: Revoke mmaps and prevent access to fence registers across
      reset") and replaced with a SRCU based solution.
      
      As such, as far as I can see, today we still have a requirement that
      resets must not sleep (invoked from submission tasklets), but no need to
      support invoking them from a truly atomic context.
      
      Given that the preemption section is problematic on RT kernels, since the
      uncore lock becomes a sleeping lock and so is invalid in such section,
      lets try and remove it. Potential downside is that our short waits on GPU
      to complete the reset may get extended if CPU scheduling interferes, but
      in practice that probably isn't a deal breaker.
      
      In terms of mechanics, since the preemption disabled block is being
      removed we just need to replace a few of the wait_for_atomic macros into
      busy looping versions which will work (and not complain) when called from
      non-atomic sections.
      
      v2:
       * Fix timeouts which are now in us. (Andi)
       * Update one comment as a drive by. (Andi)
      Signed-off-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
      Cc: Chris Wilson <chris.p.wilson@intel.com>
      Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
      Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
      Cc: Andi Shyti <andi.shyti@linux.intel.com>
      Reviewed-by: default avatarAndi Shyti <andi.shyti@linux.intel.com>
      Link: https://patchwork.freedesktop.org/patch/msgid/20230926100855.61722-1-tvrtko.ursulin@linux.intel.com
      1e975e59
  6. 26 Sep, 2023 2 commits
  7. 25 Sep, 2023 3 commits
  8. 21 Sep, 2023 3 commits
  9. 20 Sep, 2023 1 commit
  10. 19 Sep, 2023 3 commits
  11. 15 Sep, 2023 3 commits
  12. 14 Sep, 2023 1 commit
  13. 13 Sep, 2023 1 commit
  14. 08 Sep, 2023 1 commit
  15. 07 Sep, 2023 2 commits
  16. 31 Aug, 2023 1 commit
  17. 30 Aug, 2023 1 commit
  18. 29 Aug, 2023 1 commit
  19. 28 Aug, 2023 1 commit