Re: [PATCH v1 2/4] drm/xe/mcr: Sanitize steering semaphore on GT resume

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing MCR lock sanitization during driver load despite documented claims.
--

--- Patch [2]: [PATCH v1 2/4] drm/xe/mcr: Sanitize steering semaphore on GT resume ---
commit 5b9e1b6b52eef7a040056b851f2fb96429ee7d28
Author: Tales A. Mendonça <talesam@gmail.com>

drm/xe/mcr: Sanitize steering semaphore on GT resume

This commit addresses an issue where the MCR steering semaphore is left
held across a suspend cycle. It adds xe_gt_mcr_lock_sanitize() to forcibly
release the semaphore and calls it in xe_gt_resume() before the first MCR
access.

> diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.c b/drivers/gpu/drm/xe/xe_gt_mcr.c
> index e1e3c0688dc2e..3934d0de7170a 100644
> --- a/drivers/gpu/drm/xe/xe_gt_mcr.c
> +++ b/drivers/gpu/drm/xe/xe_gt_mcr.c
[ ... ]
> @@ -750,6 +750,28 @@ static void mcr_unlock(struct xe_gt *gt, unsigned int fw_ref) __releases(&gt->mc
>  	}
>  }
>  
> +/**
> + * xe_gt_mcr_lock_sanitize - Release the hardware MCR steering semaphore
> + * @gt: GT structure
> + *
> + * The MCR steering semaphore can be left in a held state after a suspend
> + * cycle, in which case the first MCR access after resume stalls until the
> + * semaphore wait times out.  Forcibly release the semaphore during driver
> + * load/resume; this is safe because no other agents acquire the semaphore

[Severity: Medium]
The comment and commit message both mention releasing the semaphore during
driver load, but the patch only adds the sanitization call to xe_gt_resume().

If the boot firmware leaves the MCR lock held, won't the first MCR access
during driver load still stall and log a timeout error?

For example, this could occur in early initialization paths like:
xe_device_probe()
  -> xe_gt_init_early()
    -> xe_pat_init()

Or in:
gt_init_with_gt_forcewake()
  -> xe_gt_enable_host_l2_vram()

Should a call to xe_gt_mcr_lock_sanitize() be added to the early driver
initialization paths to fully cover the load sequence?

> + * during the load/resume process, mirroring i915's
> + * intel_gt_mcr_lock_sanitize().
> + */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722004654.744249-1-talesam@gmail.com?part=2