All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.