[Intel-gfx] [bug report] drm/i915/mtl: Add hardware-level lock for steering

[Dan Carpenter]

Hello Matt Roper,

The patch 3100240bf846: "drm/i915/mtl: Add hardware-level lock for
steering" from Nov 28, 2022, leads to the following Smatch static
checker warning:

drivers/gpu/drm/i915/gt/intel_gt_mcr.c:379 intel_gt_mcr_lock() warn: sleeping in atomic context
CALL TREE:
intel_engine_reset() <- disables preempt
intel_gt_handle_error() <- disables preempt
live_hold_reset() <- disables preempt
reset_virtual_engine() <- disables preempt
-> __intel_engine_reset_bh()
   -> intel_engine_resume()
      -> intel_engine_apply_workarounds()
         -> wa_list_apply()
            -> intel_gt_mcr_lock()

drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c:769 intel_guc_ct_send() warn: sleeping in atomic context
CALL TREE:
guc_submission_tasklet() <- disables preempt
-> guc_dequeue_one_context()
guc_submit_request() <- disables preempt
-> guc_bypass_tasklet_submit()
   -> guc_add_request()
      -> __guc_add_request() <- disables preempt
         -> intel_guc_send_nb()
            -> intel_guc_ct_send()

drivers/gpu/drm/i915/gt/intel_gt_mcr.c
    366 void intel_gt_mcr_lock(struct intel_gt *gt, unsigned long *flags)
    367 {
    368         unsigned long __flags;
    369         int err = 0;
    370 
    371         lockdep_assert_not_held(&gt->uncore->lock);
    372 
    373         /*
    374          * Starting with MTL, we need to coordinate not only with other
    375          * driver threads, but also with hardware/firmware agents.  A dedicated
    376          * locking register is used.
    377          */
    378         if (GRAPHICS_VER_FULL(gt->i915) >= IP_VER(12, 70))
--> 379                 err = wait_for(intel_uncore_read_fw(gt->uncore,
    380                                                     MTL_STEER_SEMAPHORE) == 0x1, 100);

The wait_for() macro sleeps.  But we can't sleep if preempt is disabled.
The code in the caller looks likes:

drivers/gpu/drm/i915/gt/selftest_execlists.c
   626                  err = engine_lock_reset_tasklet(engine);
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
local_bh_disable() bumps the preempt_count.

   627                  if (err)
   628                          goto out;
   629  
   630                  engine->sched_engine->tasklet.callback(&engine->sched_engine->tasklet);
   631                  GEM_BUG_ON(execlists_active(&engine->execlists) != rq);
   632  
   633                  i915_request_get(rq);
   634                  execlists_hold(engine, rq);
   635                  GEM_BUG_ON(!i915_request_on_hold(rq));
   636  
   637                  __intel_engine_reset_bh(engine, NULL);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Sleeping.

   638                  GEM_BUG_ON(rq->fence.error != -EIO);
   639  
   640                  engine_unlock_reset_tasklet(engine);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
preempt enabled again.

   641  
   642                  /* Check that we do not resubmit the held request */

regards,
dan carpenter