Re: [PATCH v6 15/18] drm/xe/vsec: Support late bind fw information

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Delayed work polling for firmware readiness is not canceled during system suspend, risking hardware access on a powered-off device.
- [Medium] `wait_for_fw` sleeps uninterruptibly for up to 20 seconds while holding the `drm_dev_enter()` SRCU read lock, which can block system suspend and stall concurrent device unplug operations.
- [Low] The function `xe_is_oobmsm_fw_ready` is called multiple times but is undefined in the codebase.
--

commit 78def2bf4e30236b5f83a02d2b38ab43b6afb79d
Author: Michael J. Ruhl <michael.j.ruhl@intel.com>

drm/xe/vsec: Support late bind fw information

This commit updates the XE PMT probe to support late-binding firmware. It
introduces a polling mechanism using delayed work to wait for the CRI
firmware to be ready before initializing VSEC features.

> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
[ ... ]
> @@ -281,6 +291,48 @@ static int xe_guid_decode(u32 guid, int *index, u32 *offset)
>  	return -ENODEV;
>  }
>  
> +static void cri_late_bind_probe_work(struct work_struct *work)
> +{
> +	struct xe_device *xe = container_of(work, struct xe_device, pmt.work.work);
> +
> +	if (xe_is_oobmsm_fw_ready(xe)) {

[Severity: Low]
Is the xe_is_oobmsm_fw_ready() function defined anywhere? It appears this
function is called here but the definition seems to be missing from the
codebase. Will this cause a build failure with an unresolved symbol?

> +		cri_late_bind_probe(xe);
> +		xe_pm_runtime_put(xe);
> +		return;
> +	}
> +
> +	xe->pmt.retry_count++;
> +
> +	/* wait up to 20 seconds */
> +	if (xe->pmt.retry_count == VSEC_LATE_BIND_RETRY) {
> +		drm_warn(&xe->drm, "PMT probe: Late Binding failed to complete\n");
> +		xe_pm_runtime_put(xe);
> +		return;
> +	}
> +
> +	if (!schedule_delayed_work(&xe->pmt.work, msecs_to_jiffies(VSEC_LATE_BIND_DELAY_MSEC)))

[Severity: High]
Is there a risk that this delayed work could execute while the system is
suspended? The work is scheduled to poll for up to 20 seconds, but there
doesn't appear to be any integration with the driver's suspend/resume paths
to cancel this work.

If the system enters S3 sleep during this window, could the work wake up
and attempt to access MMIO (e.g., via xe_is_oobmsm_fw_ready()) on a
powered-off device, potentially causing PCI bus errors or a kernel panic?

> +		xe_pm_runtime_put(xe);
> +}
> +
> +static bool wait_for_fw(struct xe_device *xe)
> +{
> +	int retries = VSEC_LATE_BIND_RETRY;  /* wait up to 20 secs */
> +
> +	if (xe->info.platform != XE_CRESCENTISLAND)
> +		return true;
> +
> +	while (retries--) {
> +		if (xe_is_oobmsm_fw_ready(xe))
> +			return true;
> +
> +		msleep(VSEC_LATE_BIND_DELAY_MSEC);

[Severity: Medium]
Could this uninterruptible sleep cause issues with system suspend or device
unplug? This function can sleep for up to 20 seconds, and it is called from
xe_pmt_telem_read() which holds the drm_dev_enter() SRCU read lock.

Holding that lock for 20 seconds will block drm_dev_unplug() for the same
duration. Additionally, an uninterruptible sleep of this length can block
the system PM freezer, potentially causing suspend to time out and abort.

Since this can be reached by a userspace process reading PMT telemetry,
could this allow an unprivileged user to trigger a localized denial of
service?

> +	}
> +
> +	drm_warn(&xe->drm, "Late Binding failed to complete\n");
> +
> +	return false;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904182451.1164868-20-michael.j.ruhl@intel.com?part=15