Re: [PATCH v4 05/12] drm/xe/xe_ras: Add support to retrieve info queue data for CRI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] xe_ras: missing validation of operation_status in get_info_queue_data
--

commit 7080ef5206c9792b353853f321f38646d98e4924
Author: Badal Nilawar <badal.nilawar@intel.com>

drm/xe/xe_ras: Add support to retrieve info queue data for CRI

Retrieve the RAS info queue data, in multiple chunks, and assemble it into
a flat buffer. A follow-up patch will use this data to prepare the CPER
error info.

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index 5cfc9beff8c13..900319938fe54 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
[ ... ]
> +static int get_info_queue_data(struct xe_device *xe,
> +			       struct xe_ras_get_info_queue_request *req,
> +			       struct xe_ras_get_info_queue_response *out)
> +{
[ ... ]
> +	if (rlen != sizeof(response)) {
> +		xe_err(xe, "sysctrl: unexpected get info queue data response length %zu (expected %zu)\n",
> +		       rlen, sizeof(response));
> +		return -EIO;
> +	}
> +
> +	hdr = &response.queue_response.header;
> +	xe_dbg(xe, "[RAS]: info queue data: status=%u chunk_size=%u flags=0x%x\n",
> +	       response.operation_status, hdr->chunk_size, hdr->flags);
> +
> +	*out = response;
> +	return 0;
> +}

[Severity: Medium]
Does this code need to validate response.operation_status and return an error
if the firmware operation failed?

get_info_queue_data() executes the XE_SYSCTRL_CMD_GET_INFO_QUEUE mailbox
command to fetch chunks from the firmware. The firmware response includes an
operation_status field to indicate whether the request was successful or
rejected (e.g., due to an invalid session ID or out-of-bounds offset).

However, this only logs the status in a debug message and unconditionally
returns 0 (success). Because it fails to return an error (such as converting
the status via ras_status_to_errno()), the caller
xe_ras_drain_info_queue_raw() will proceed to validate and process the
payload. If the firmware did not populate the payload on error, it will
contain zeroes or garbage, resulting in a confusing "invalid info queue
chunk" warning or silent failure, rather than properly halting the fetch
with the correct error code.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009104602.1075405-14-badal.nilawar@intel.com?part=5