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

["Tauro, Riana" <riana.tauro@intel.com>]

On 01-10-2026 16:54, Upadhyay, Tejas wrote:
>
>> -----Original Message-----
>> From: Tauro, Riana <riana.tauro@intel.com>
>> Sent: 28 September 2026 11:49
>> To: intel-xe@lists.freedesktop.org
>> Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
>> <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
>> aravind.iddamsetty@linux.intel.com; Nilawar, Badal
>> <badal.nilawar@intel.com>; Jadav, Raag <raag.jadav@intel.com>;
>> Koppuravuri, Ravi Kishore <ravi.kishore.koppuravuri@intel.com>; Koujalagi,
>> Mallesh <mallesh.koujalagi@intel.com>; Upadhyay, Tejas
>> <tejas.upadhyay@intel.com>; Ghimiray, Himal Prasad
>> <himal.prasad.ghimiray@intel.com>
>> Subject: [PATCH v3 2/6] drm/xe/xe_ras: Add support to query page offline
>> queue and list
>>
>> Add support to query page offline list and queue from firmware during
>> module load. The page offline list command retrieves pages that are already
>> offlined by the firmware. The page offline queue command retrieves the pages
>> pending to be offlined by the firmware.
>>
>> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> ---
>> v2: rebase
>>      store total pages once per response (Sashiko)
>>
>> v3: common function for offline and queue (Himal)
>> ---
>>   drivers/gpu/drm/xe/xe_ras.c                   | 74 +++++++++++++++++++
>>   drivers/gpu/drm/xe/xe_ras_types.h             | 35 +++++++++
>>   drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h |  4 +
>>   3 files changed, 113 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c index
>> 1225c561a872..752754f09314 100644
>> --- a/drivers/gpu/drm/xe/xe_ras.c
>> +++ b/drivers/gpu/drm/xe/xe_ras.c
>> @@ -335,6 +335,77 @@ static bool ras_counter_is_valid(struct xe_device
>> *xe, struct xe_ras_error_class
>>   	return true;
>>   }
>>
>> +static void get_offline_pages(struct xe_device *xe, u32 cmd, void *req, size_t
>> req_size,
>> +			      void *resp, size_t resp_size,
>> +			      struct xe_ras_offline_common *common, bool
>> offline) {
>> +	struct xe_sysctrl_mailbox_command command = {0};
>> +	struct xe_ras_offline_list_request *list_req;
>> +	u32 total_pages = 0, count = 0;
>> +	ssize_t rlen;
> size_t rlen;
Sure will fix

>
>> +	int ret, i;
>> +
>> +	list_req = req ? req : NULL;
> equivalent to list_req = req

I had initially added all conversions. missed this while removing . yeah 
will fix it

>
>> +
>> +	xe_sysctrl_create_command(&command, XE_SYSCTRL_GROUP_GFSP,
>> cmd, req, req_size, resp,
>> +				  resp_size);
>> +
>> +	do {
>> +		memset(resp, 0, resp_size);
>> +
>> +		if (list_req)
>> +			list_req->index = count;
>> +
>> +		ret = xe_sysctrl_send_command(&xe->sc, &command, &rlen);
>> +		if (ret) {
>> +			xe_log_err(xe, SYSCTRL, ret, "failed to get page offline
>> data, cmd=%#x\n",
>> +				   cmd);
>> +			return;
>> +		}
>> +
>> +		if (rlen != resp_size) {
>> +			xe_log_err(xe, SYSCTRL, -EINVAL,
>> +				   "unexpected page offline response length
>> %zu (expected %zu), cmd=%#x\n",
>> +				   rlen, resp_size, cmd);
>> +			return;
>> +		}
>> +
>> +		for (i = 0; i < common->pages_returned && i <
>> XE_RAS_NUM_PAGES; i++)
>> +			handle_page_offline(xe, common->page_addresses[i],
>> offline);
>> +
>> +		count += common->pages_returned;
>> +		if (!common->pages_returned)
>> +			break;
>> +
>> +		if (!total_pages)
>> +			total_pages = common->total_pages;
>> +
>> +		if (count > total_pages) {
>> +			xe_log_err(xe, SYSCTRL, -EINVAL,
>> +				   "Pages returned exceed total pages %u,
>> returned %u, cmd=%#x\n",
>> +				   total_pages, count, cmd);
>> +			return;
>> +		}
>> +	} while (common->additional_data);
>> +}
>> +
>> +static void get_queued_pages(struct xe_device *xe) {
>> +	struct xe_ras_offline_common response = {0};
>> +
>> +	get_offline_pages(xe, XE_SYSCTRL_CMD_GET_OFFLINE_QUEUE, NULL,
>> 0, &response,
>> +			  sizeof(response), &response, true); }
>> +
>> +static void get_offlined_list(struct xe_device *xe) {
>> +	struct xe_ras_offline_list_response response = {0};
>> +	struct xe_ras_offline_list_request request = {0};
>> +
>> +	get_offline_pages(xe, XE_SYSCTRL_CMD_GET_OFFLINE_LIST,
>> &request, sizeof(request),
>> +			  &response, sizeof(response), &response.common,
>> false); }
> Please consider following if it looks ok. To make it less confusing and naming it what it actually does,
>
> /*
>   * Fetch the next batch of page addresses for @cmd from firmware and process
>   * each one locally via handle_page_offline(). @notify_fw controls whether
>   * firmware is told back (XE_SYSCTRL_CMD_PAGE_OFFLINE) once a page has been
>   * handled - see the two callers below for why that differs per source.
>   */
> static void xe_ras_process_offline_pages(struct xe_device *xe, u32 cmd, void *req,
>                       size_t req_size, void *resp, size_t resp_size,
>                       struct xe_ras_offline_common *common, bool notify_fw)
process_offline_pages is indeed better than get_offline_pages. Will 
avoid confusion
Will change it. File prefix is used in xe driver for non-static functions.

> {
>      ...
>      for (i = 0; i < common->pages_returned && i < XE_RAS_NUM_PAGES; i++)
>          handle_page_offline(xe, common->page_addresses[i], notify_fw);
>      ...
> }
>
> /*
>   * Firmware's pending queue: addresses it hasn't finished offlining yet.
>   * Drain it and ack each page back so firmware can dequeue it.
>   */
> static void xe_ras_drain_offline_queue(struct xe_device *xe)
> {
>      struct xe_ras_offline_common response = {0};
>
>      xe_ras_process_offline_pages(xe, XE_SYSCTRL_CMD_GET_OFFLINE_QUEUE, NULL, 0,
>                       &response, sizeof(response), &response, true);
> }
>
> /*
>   * Firmware's persisted (flash) list of already-offlined pages. Just replay
>   * them into local VRAM tracking on driver load; firmware already has them.
>   */
> static void xe_ras_restore_offlined_pages(struct xe_device *xe)

Drain does make sense for queue but restore doesn't for list. We are not 
restoring the pages, they are still offlined.
Wouldn't it be better to just retain the  command names. Adding 
description is better . will add that in new rev.

Thanks
Riana


> {
>      struct xe_ras_offline_list_response response = {0};
>      struct xe_ras_offline_list_request request = {0};
>
>      xe_ras_process_offline_pages(xe, XE_SYSCTRL_CMD_GET_OFFLINE_LIST, &request,
>                       sizeof(request), &response, sizeof(response),
>                       &response.common, false);
> }
>
> Tejas
>> +
>>   static struct pci_dev *find_usp_dev(struct pci_dev *pdev)  {
>>   	struct pci_dev *vsp;
>> @@ -1049,6 +1120,9 @@ void xe_ras_init(struct xe_device *xe)
>>   	if (IS_ENABLED(CONFIG_PCIEAER))
>>   		ras_usp_aer_init(xe);
>>
>> +	get_queued_pages(xe);
>> +	get_offlined_list(xe);
>> +
>>   	ret = devm_device_add_group(xe->drm.dev, &gpu_health_group);
>>   	if (ret)
>>   		xe_err(xe, "Failed to create GPU health sysfs, err=%d\n", ret);
>> diff --git a/drivers/gpu/drm/xe/xe_ras_types.h
>> b/drivers/gpu/drm/xe/xe_ras_types.h
>> index f119489bcdf2..021ffbd6d4e2 100644
>> --- a/drivers/gpu/drm/xe/xe_ras_types.h
>> +++ b/drivers/gpu/drm/xe/xe_ras_types.h
>> @@ -10,6 +10,7 @@
>>
>>   #define XE_RAS_NUM_COUNTERS			16
>>   #define XE_RAS_NUM_ERROR_ARR			3
>> +#define XE_RAS_NUM_PAGES			25
>>   /* Error bits in IEH global error status register */
>>   #define XE_RAS_SOC_IEH_PUNIT			BIT(1)
>>   /* Device memory error categories */
>> @@ -330,6 +331,40 @@ struct xe_ras_page_offline_response {
>>   	u32 reserved;
>>   } __packed;
>>
>> +/**
>> + * struct xe_ras_offline_common - Common structure for offline list and
>> +queue  */ struct xe_ras_offline_common {
>> +	/** @total_pages: Total number of queued pages */
>> +	u32 total_pages;
>> +	/** @pages_returned: Number of pages returned in this response */
>> +	u32 pages_returned;
>> +	/** @page_addresses: Array of page addresses (4KB aligned) */
>> +	u64 page_addresses[XE_RAS_NUM_PAGES];
>> +	/** @additional_data: Indicates if more data is available */
>> +	u8 additional_data;
>> +	/** @reserved: Reserved for future use */
>> +	u8 reserved[3];
>> +} __packed;
>> +
>> +/**
>> + * struct xe_ras_offline_list_request - Request for get offline list
>> +command  */ struct xe_ras_offline_list_request {
>> +	/** @index: Zero-based index into the offline page list */
>> +	u32 index;
>> +} __packed;
>> +
>> +/**
>> + * struct xe_ras_offline_list_response - Response from get offline list
>> +command  */ struct xe_ras_offline_list_response {
>> +	/** @max_entries: Total no of pages that can be stored in flash */
>> +	u32 max_entries;
>> +	/** @common: Common offline page information */
>> +	struct xe_ras_offline_common common;
>> +} __packed;
>> +
>>   /**
>>    * struct xe_ras_get_health_request - Request structure for obtaining gpu
>> health
>>    */
>> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
>> b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
>> index a01576bf2e73..3a71ed446949 100644
>> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
>> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
>> @@ -31,6 +31,8 @@ enum xe_sysctrl_group {
>>    * @XE_SYSCTRL_CMD_SET_THRESHOLD: Set error threshold
>>    * @XE_SYSCTRL_CMD_GET_PENDING_EVENT: Retrieve pending event
>>    * @XE_SYSCTRL_CMD_PAGE_OFFLINE: Instruct firmware to offline/remove a
>> page
>> + * @XE_SYSCTRL_CMD_GET_OFFLINE_LIST: Retrieve list of all offlined
>> + pages from flash
>> + * @XE_SYSCTRL_CMD_GET_OFFLINE_QUEUE: Retrieve list of offlined
>> queued
>> + pages from firmware
>>    * @XE_SYSCTRL_CMD_GET_HEALTH: Retrieve gpu health
>>    * @XE_SYSCTRL_CMD_SET_HEALTH: Set gpu health
>>    */
>> @@ -42,6 +44,8 @@ enum xe_sysctrl_gfsp_cmd {
>>   	XE_SYSCTRL_CMD_SET_THRESHOLD		= 0x06,
>>   	XE_SYSCTRL_CMD_GET_PENDING_EVENT	= 0x07,
>>   	XE_SYSCTRL_CMD_PAGE_OFFLINE		= 0x08,
>> +	XE_SYSCTRL_CMD_GET_OFFLINE_LIST		= 0x09,
>> +	XE_SYSCTRL_CMD_GET_OFFLINE_QUEUE	= 0x0A,
>>   	XE_SYSCTRL_CMD_GET_HEALTH		= 0x0B,
>>   	XE_SYSCTRL_CMD_SET_HEALTH		= 0x0C,
>>   };
>> --
>> 2.47.1