Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Raag Jadav <raag.jadav@intel.com>
To: Badal Nilawar <badal.nilawar@intel.com>
Cc: intel-xe@lists.freedesktop.org, anshuman.gupta@intel.com,
	rodrigo.vivi@intel.com, daniele.ceraolospurio@intel.com,
	riana.tauro@intel.com, mallesh.koujalagi@intel.com,
	aravind.iddamsetty@intel.com, himal.prasad.ghimiray@intel.com,
	arvind.yadav@intel.com, syed.abdul.muqthyar.ahmed@intel.com,
	nitin.r.gote@intel.com, pravalika.gurram@intel.com
Subject: Re: [PATCH v4 05/12] drm/xe/xe_ras: Add support to retrieve info queue data for CRI
Date: Wed, 14 Oct 2026 17:19:20 +0200	[thread overview]
Message-ID: <as-deM1jmVVLxCA4@black.igk.intel.com> (raw)
In-Reply-To: <20261009104602.1075405-19-badal.nilawar@intel.com>

On Fri, Oct 09, 2026 at 04:16:08PM +0530, Badal Nilawar wrote:

Nit: Although this is only available on selected platforms, the
implementation is platform agnostic. So I'd remove CRI from subject.

> 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.

Again, no follow-up. Just describe this patch.

> Assisted-by: Copilot:Claude-Opus-5.5
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> v4:
>  - Simplify info queue handling and validation (Raag)
>  - Clean up info queue naming and definitions (Raag)
>  - Validate each chunk against the requested offset
> ---
>  drivers/gpu/drm/xe/xe_ras.c                   | 117 +++++++++++++++++
>  drivers/gpu/drm/xe/xe_ras.h                   |   3 +
>  drivers/gpu/drm/xe/xe_ras_types.h             | 118 +++++++++++++++++-
>  drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h |   2 +
>  4 files changed, 238 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index 5cfc9beff8c1..900319938fe5 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -788,6 +788,123 @@ int xe_ras_set_threshold(struct xe_device *xe, u8 severity, u8 component, u32 th
>  	return 0;
>  }
>  
> +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)

'resp'.

> +{
> +	struct xe_ras_get_info_queue_response response = {0};

No need for local response, you already got it from caller.

> +	struct xe_sysctrl_mailbox_command command = {0};
> +	const struct xe_ras_info_queue_header *hdr;
> +	size_t rlen;
> +	int ret;
> +
> +	xe_sysctrl_create_command(&command, XE_SYSCTRL_GROUP_GFSP, XE_SYSCTRL_CMD_GET_INFO_QUEUE,
> +				  req, sizeof(*req), &response, sizeof(response));
> +
> +	ret = xe_sysctrl_send_command(&xe->sc, &command, &rlen);
> +	if (ret) {
> +		xe_err(xe, "sysctrl: failed to get info queue data %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (rlen != sizeof(response)) {
> +		xe_err(xe, "sysctrl: unexpected get info queue data response length %zu (expected %zu)\n",
> +		       rlen, sizeof(response));
> +		return -EIO;
> +	}

If we got a response status, we may want to look for failures here. You
can refer to clear_counter() and set_threshold() for examples.

> +	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);

Looking at the header definition, we may want to log more details.
At the bare minimum, we should have total_size, chunk_offset and
chunk_size. The rest are upto you.

> +	*out = response;
> +	return 0;
> +}
> +
> +static bool chunk_is_valid(const struct xe_ras_info_queue_header *hdr,
> +			   u32 expected_offset, u32 dest_size)
> +{
> +	u32 end;
> +
> +	return hdr->chunk_offset == expected_offset &&
> +	       hdr->chunk_size && hdr->chunk_size <= XE_RAS_INFO_QUEUE_MAX_CHUNK_SIZE &&
> +	       !check_add_overflow(hdr->chunk_offset, hdr->chunk_size, &end) &&
> +	       end <= XE_RAS_INFO_QUEUE_MAX_TOTAL_SIZE && end <= dest_size;

Curious, should I worry about termination byte?

> +}
> +
> +/**
> + * xe_ras_drain_info_queue_raw() - Drain the full RAS info queue into a flat buffer
> + * @xe: xe device
> + * @counter_resp: counter response carrying the first embedded chunk and the
> + *                counter identifier used as the source context for subsequent
> + *                GET_INFO_QUEUE fetches
> + * @dest_buf: destination buffer supplied by the caller
> + * @dest_size: size of @dest_buf in bytes; also caps the total amount of data
> + *             assembled from the info queue
> + *
> + * Copies the first chunk already embedded in @counter_resp, then loops
> + * issuing GET_INFO_QUEUE to fetch any remaining chunks until the queue
> + * signals no more data or a transport/bounds error is encountered.  On
> + * transport or bounds errors the function stops and returns whatever has
> + * been assembled so far.
> + *
> + * Returns: number of valid bytes written into @dest_buf.  Zero if @dest_buf is

Nit: 'Number' with capital N.

> + *          NULL or @dest_size is 0.
> + */
> +u32 xe_ras_drain_info_queue_raw(struct xe_device *xe,
> +				const struct xe_ras_get_counter_response *counter_resp,
> +				u8 *dest_buf, u32 dest_size)
> +{
> +	const struct xe_ras_info_queue_header *first_hdr = &counter_resp->first.header;
> +	struct xe_ras_get_info_queue_request req = {0};
> +	struct xe_ras_get_info_queue_response resp;

This one should be inside the while loop.

> +	u32 flags = first_hdr->flags;
> +	u32 offset = 0;
> +
> +	if (!dest_buf || !dest_size)
> +		return 0;
> +
> +	if (first_hdr->total_size > dest_size)
> +		xe_warn(xe, "[RAS]: info queue size %u exceeds %u, truncating\n",
> +			first_hdr->total_size, dest_size);

Why not return error here?

> +	/* Copy first chunk already embedded in the counter response */
> +	if (chunk_is_valid(first_hdr, offset, dest_size)) {

Ditto, why do we continue after invalid chunk?

> +		memcpy(dest_buf, counter_resp->first.data, first_hdr->chunk_size);
> +		offset = first_hdr->chunk_size;
> +	}
> +
> +	req.source_command = XE_SYSCTRL_CMD_GET_COUNTER;
> +	req.source_context = counter_resp->counter;
> +	req.queue_request.session_id = counter_resp->counter;

Why the duplication?

> +	/* Fetch any remaining chunks */
> +	while ((flags & XE_RAS_INFO_QUEUE_FLAG_MORE_DATA) && offset < dest_size) {
> +		const struct xe_ras_info_queue_header *hdr = &resp.queue_response.header;

memset(resp, 0)

> +		req.queue_request.offset = offset;
> +		req.queue_request.size = min(XE_RAS_INFO_QUEUE_MAX_CHUNK_SIZE, dest_size - offset);
> +
> +		if (get_info_queue_data(xe, &req, &resp)) {
> +			xe_err(xe, "[RAS]: info queue drain aborted: fetch at offset=%u failed\n",

... with "%d\n

> +			       offset);

, ret);

> +			return offset;
> +		}
> +
> +		if (!chunk_is_valid(hdr, offset, dest_size)) {
> +			xe_warn(xe, "[RAS]: invalid info queue chunk (offset=%u size=%u, expected offset=%u)\n",
> +				hdr->chunk_offset, hdr->chunk_size, offset);
> +			return offset;
> +		}
> +
> +		memcpy(dest_buf + offset, resp.queue_response.data, hdr->chunk_size);
> +		offset += hdr->chunk_size;
> +		flags = hdr->flags;
> +	}
> +
> +	return min(offset, first_hdr->total_size);
> +}
> +
>  static ssize_t gpu_health_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	struct xe_ras_get_health_response response = {0};
> diff --git a/drivers/gpu/drm/xe/xe_ras.h b/drivers/gpu/drm/xe/xe_ras.h
> index 84700ea63394..27ce6265ace5 100644
> --- a/drivers/gpu/drm/xe/xe_ras.h
> +++ b/drivers/gpu/drm/xe/xe_ras.h
> @@ -22,5 +22,8 @@ void xe_ras_init(struct xe_device *xe);
>  enum xe_ras_recovery_action xe_ras_process_errors(struct xe_device *xe);
>  int xe_ras_get_counter_response(struct xe_device *xe, const struct xe_ras_error_class *counter,
>  				struct xe_ras_get_counter_response *out);
> +u32 xe_ras_drain_info_queue_raw(struct xe_device *xe,
> +				const struct xe_ras_get_counter_response *counter_resp,
> +				u8 *dest_buf, u32 dest_size);
>  
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_ras_types.h b/drivers/gpu/drm/xe/xe_ras_types.h
> index fe6f3658a2a4..9901fd034e3f 100644
> --- a/drivers/gpu/drm/xe/xe_ras_types.h
> +++ b/drivers/gpu/drm/xe/xe_ras_types.h
> @@ -17,6 +17,10 @@
>  #define XE_RAS_MEMORY_POISON			BIT(2)
>  #define XE_RAS_MEMORY_DATA_PARITY		BIT(5)
>  
> +#define XE_RAS_INFO_QUEUE_MAX_CHUNK_SIZE	200
> +#define XE_RAS_INFO_QUEUE_MAX_TOTAL_SIZE	5120
> +#define XE_RAS_INFO_QUEUE_FLAG_MORE_DATA	0x02
> +
>  /**
>   * enum xe_ras_recovery_action - RAS recovery actions
>   *
> @@ -95,6 +99,110 @@ struct xe_ras_threshold_crossed {
>  	struct xe_ras_error_class counters[XE_RAS_NUM_COUNTERS];
>  } __packed;
>  
> +/**
> + * struct xe_ras_info_queue_header - Metadata for large info queue data transfers
> + *
> + * Provides chunk metadata for commands that support extended info queue
> + * functionality. Used when the total data exceeds a single mailbox response.
> + */
> +struct xe_ras_info_queue_header {
> +	/** @total_size: Total size of the complete info queue data in bytes */
> +	u32 total_size;
> +	/** @chunk_offset: Offset of this chunk within the total data in bytes */
> +	u32 chunk_offset;
> +	/** @chunk_size: Size of the data in this chunk in bytes */
> +	u32 chunk_size;
> +	/** @reserved0: Reserved for future use */
> +	u32 reserved0;
> +	/** @flags: Info queue control flags (XE_RAS_INFO_QUEUE_FLAG_*) */
> +	u32 flags:8;
> +	/** @reserved1: Reserved for future use */
> +	u32 reserved1:4;
> +	/** @num_headers: Number of detailed counter headers at start of the data */
> +	u32 num_headers:5;

Is this actually used here?

> +	/** @reserved2: Reserved for future use */
> +	u32 reserved2:15;
> +	/** @checksum: CRC32 checksum of this chunk data */
> +	u32 checksum;

Ditto.

> +} __packed;
> +
> +/**
> + * struct xe_ras_info_queue_data_request - Request for a specific chunk of info queue data
> + *
> + * Allows the driver to request continuation of large info queue transfers
> + * by specifying an offset and size within the full data set.
> + */
> +struct xe_ras_info_queue_data_request {
> +	/** @offset: Byte offset of the requested data chunk */
> +	u32 offset;
> +	/** @size: Maximum size of the requested chunk in bytes */
> +	u32 size;
> +	/** @session_id: Session ID to correlate multi-chunk transfers */
> +	struct xe_ras_error_class session_id;
> +	/** @reserved: Reserved for future use */
> +	u32 reserved;
> +} __packed;
> +
> +/**
> + * struct xe_ras_info_queue_data_response - Generic response for commands with info queues
> + *
> + * Standard response format for any command that returns an info queue
> + * payload. May be embedded in a command-specific response structure.
> + */
> +struct xe_ras_info_queue_data_response {
> +	/** @header: Info queue metadata for this chunk */
> +	struct xe_ras_info_queue_header header;
> +	/** @data: Info queue data for this chunk */
> +	u8 data[XE_RAS_INFO_QUEUE_MAX_CHUNK_SIZE];
> +} __packed;
> +
> +/**
> + * struct xe_ras_info_queue_dynamic_counter_hdr - Aggregate counter header entry
> + *
> + * When a session requests aggregate counter data, one header per matching
> + * dynamic counter class is prepended to the queue data. The @counter field
> + * is the error count of this class; at most XE_RAS_NUM_COUNTERS error log
> + * entries follow for it.
> + */
> +struct xe_ras_info_queue_dynamic_counter_hdr {
> +	/** @error_class: Error class associated with this counter group */
> +	struct xe_ras_error_class error_class;

'counter'

> +	/** @counter: Error count of this class */
> +	u32 counter;

'count'

> +} __packed;
> +
> +/**
> + * struct xe_ras_error_log - Single error log entry following dynamic counter headers
> + */
> +struct xe_ras_error_log {
> +	/** @timestamp: Timestamp when the error was recorded */
> +	u64 timestamp;
> +	/** @error_details: Error-specific details */
> +	u32 error_details[16];
> +} __packed;
> +
> +/**
> + * struct xe_ras_get_info_queue_request - Request for XE_SYSCTRL_CMD_GET_INFO_QUEUE
> + */
> +struct xe_ras_get_info_queue_request {
> +	/** @queue_request: Info queue request parameters */
> +	struct xe_ras_info_queue_data_request queue_request;
> +	/** @source_command: Original command that generated the info queue */
> +	u32 source_command;
> +	/** @source_context: Context from original command, if applicable */
> +	struct xe_ras_error_class source_context;
> +} __packed;
> +
> +/**
> + * struct xe_ras_get_info_queue_response - Response for XE_SYSCTRL_CMD_GET_INFO_QUEUE
> + */
> +struct xe_ras_get_info_queue_response {
> +	/** @operation_status: Status of the retrieval operation */
> +	u32 operation_status;

'status'

> +	/** @queue_response: Info queue data chunk */
> +	struct xe_ras_info_queue_data_response queue_response;
> +} __packed;
> +
>  /**
>   * struct xe_ras_get_counter_request - Request structure for get counter
>   */
> @@ -117,8 +225,14 @@ struct xe_ras_get_counter_response {
>  	u64 timestamp;
>  	/** @threshold: Threshold value for the counter */
>  	u32 threshold;
> -	/** @reserved: Reserved  */
> -	u32 reserved[57];
> +	/** @reserved: Reserved for future use */
> +	u32 reserved:9;
> +	/** @has_info_queue: Set if info queue is available */
> +	u32 has_info_queue:1;

Hmm...

> +	/** @reserved1: Reserved for future use */
> +	u32 reserved1:22;
> +	/** @first: First info queue chunk, if @has_info_queue is set */

... so where is this condition?

Raag

> +	struct xe_ras_info_queue_data_response first;
>  } __packed;
>  
>  /**
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
> index a0628eb47bff..299d12b94574 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
> @@ -34,6 +34,7 @@ enum xe_sysctrl_group {
>   * @XE_SYSCTRL_CMD_GET_PENDING_EVENT: Retrieve pending event
>   * @XE_SYSCTRL_CMD_GET_HEALTH: Retrieve gpu health
>   * @XE_SYSCTRL_CMD_SET_HEALTH: Set gpu health
> + * @XE_SYSCTRL_CMD_GET_INFO_QUEUE: Retrieve a chunk of info queue data
>   */
>  enum xe_sysctrl_gfsp_cmd {
>  	XE_SYSCTRL_CMD_GET_SOC_ERROR		= 0x01,
> @@ -44,6 +45,7 @@ enum xe_sysctrl_gfsp_cmd {
>  	XE_SYSCTRL_CMD_GET_PENDING_EVENT	= 0x07,
>  	XE_SYSCTRL_CMD_GET_HEALTH		= 0x0B,
>  	XE_SYSCTRL_CMD_SET_HEALTH		= 0x0C,
> +	XE_SYSCTRL_CMD_GET_INFO_QUEUE		= 0x0D,
>  };
>  
>  /**
> -- 
> 2.54.0
> 

  parent reply	other threads:[~2026-10-14 15:19 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-09 10:46 [PATCH v4 00/12] Add CPER logging support for CRI Badal Nilawar
2026-10-09 10:46 ` [PATCH v4 01/12] drm/xe/cper: Add CPER reporting entry point for xe_log Badal Nilawar
2026-10-09 10:43   ` sashiko-bot
2026-10-09 12:17     ` Nilawar, Badal
2026-10-12  6:18   ` Raag Jadav
2026-10-09 10:46 ` [PATCH v4 02/12] drm/xe/cper: Retrieve the error counter record for CPER reporting Badal Nilawar
2026-10-09 10:46   ` sashiko-bot
2026-10-09 12:27     ` Nilawar, Badal
2026-10-12  9:07   ` Raag Jadav
2026-10-09 10:46 ` [PATCH v4 03/12] drm/xe/cper: Initialize Intel-specific error section header Badal Nilawar
2026-10-14  7:27   ` Raag Jadav
2026-10-09 10:46 ` [PATCH v4 04/12] drm/xe/cper: Construct CPER record for hardware errors Badal Nilawar
2026-10-14 10:30   ` Raag Jadav
2026-10-09 10:46 ` [PATCH v4 05/12] drm/xe/xe_ras: Add support to retrieve info queue data for CRI Badal Nilawar
2026-10-09 10:40   ` sashiko-bot
2026-10-09 12:45     ` Nilawar, Badal
2026-10-14 15:19   ` Raag Jadav [this message]
2026-10-09 10:46 ` [PATCH v4 06/12] drm/xe/cper: Prepare Intel CPER error info records Badal Nilawar
2026-10-09 10:41   ` sashiko-bot
2026-10-09 13:07     ` Nilawar, Badal
2026-10-09 10:46 ` [PATCH v4 07/12] drm/xe/cper: Log CPER records for aggregate counter retrieval Badal Nilawar
2026-10-09 10:46 ` [PATCH v4 08/12] drm/xe/cper: Emit cper record to trace buf Badal Nilawar
2026-10-09 10:38   ` sashiko-bot
2026-10-09 10:46 ` [PATCH v4 09/12] drm/xe/xe_ras: Report device memory errors using SIGID Badal Nilawar
2026-10-14  9:05   ` Mallesh, Koujalagi
2026-10-09 10:46 ` [PATCH v4 10/12] drm/xe/xe_ras: Report core compute " Badal Nilawar
2026-10-14  9:28   ` Mallesh, Koujalagi
2026-10-14  9:30     ` Tauro, Riana
2026-10-09 10:46 ` [PATCH v4 11/12] drm/xe/xe_ras: Report soc internal " Badal Nilawar
2026-10-14  9:31   ` Mallesh, Koujalagi
2026-10-09 10:46 ` [PATCH v4 12/12] drm/xe/xe_ras: Report correctable " Badal Nilawar
2026-10-09 10:50   ` sashiko-bot
2026-10-09 10:54 ` ✗ CI.checkpatch: warning for Add CPER logging support for CRI (rev4) Patchwork
2026-10-09 10:56 ` ✓ CI.KUnit: success " Patchwork
2026-10-09 12:03 ` ✗ Xe.CI.BAT: failure " Patchwork
2026-10-09 15:13 ` ✓ Xe.CI.FULL: success " Patchwork
2026-10-12  6:14 ` [PATCH v4 00/12] Add CPER logging support for CRI Raag Jadav

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=as-deM1jmVVLxCA4@black.igk.intel.com \
    --to=raag.jadav@intel.com \
    --cc=anshuman.gupta@intel.com \
    --cc=aravind.iddamsetty@intel.com \
    --cc=arvind.yadav@intel.com \
    --cc=badal.nilawar@intel.com \
    --cc=daniele.ceraolospurio@intel.com \
    --cc=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=mallesh.koujalagi@intel.com \
    --cc=nitin.r.gote@intel.com \
    --cc=pravalika.gurram@intel.com \
    --cc=riana.tauro@intel.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=syed.abdul.muqthyar.ahmed@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox