Re: [PATCH v2 2/6] drm/xe/xe_ras: Add support to query page offline queue and list

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Spurious failure if firmware page offline queue or list grows during readout.
--

commit ecb51c4744892bd663ca44008b7aac2eca7280e9
Author: Riana Tauro <riana.tauro@intel.com>
Subject: drm/xe/xe_ras: Add support to query page offline queue and list

This commit adds support to query the page offline list and queue from the
firmware during module load. The command retrieves pages that are already
offlined or pending to be offlined by the firmware.

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
[ ... ]
> +static void get_queued_pages(struct xe_device *xe)
> +{
[ ... ]
> +	do {
> +		memset(&response, 0, sizeof(response));
> +
> +		ret = xe_sysctrl_send_command(&xe->sc, &command, &rlen);
[ ... ]
> +		count += response.pages_returned;
> +		if (!response.pages_returned)
> +			break;
> +
> +		if (!total_pages)
> +			total_pages = response.total_pages;
> +
> +		if (count > total_pages) {
> +			xe_log_err(xe, SYSCTRL, -EINVAL,

[Severity: Medium]
If a memory RAS error occurs during the window when xe_ras_init() is querying
the offline queue, causing the firmware to append to the queue while the
driver is iterating, can this cause a spurious failure?

Since total_pages is latched on the first response and strictly enforced
against the growing count over multiple responses, it looks like an actively
growing queue could cause count to exceed total_pages, aborting the
initialization of offlined pages and leaving the RAS offline state incomplete.

> +				   "Pages returned from queue exceed total pages %u, returned %u\n",
> +				   total_pages, count);
> +			return;
> +		}
> +	} while (response.additional_data);
> +}
> +
> +static void get_offlined_list(struct xe_device *xe)
> +{
[ ... ]
> +	do {
> +		memset(&response, 0, sizeof(response));
> +		request.index = count;
> +
> +		ret = xe_sysctrl_send_command(&xe->sc, &command, &rlen);
[ ... ]
> +		count += response.pages_returned;
> +		if (!response.pages_returned)
> +			break;
> +
> +		if (!total_pages)
> +			total_pages = response.total_pages;
> +
> +		if (count > total_pages) {
> +			xe_log_err(xe, SYSCTRL, -EINVAL,

[Severity: Medium]
Does the same vulnerability apply here in get_offlined_list() if new pages are
offlined concurrently by the firmware?

If the list grows while reading, enforcing the originally latched total_pages
against the total returned count could similarly result in a spurious error
and abort the list iteration.

> +				   "Pages returned from list exceed total pages %u, returned %u\n",
> +				   total_pages, count);
> +			return;
> +		}
> +	} while (response.additional_data);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907094706.1407436-8-riana.tauro@intel.com?part=2