Re: [PATCH v3 1/6] drm/xe/xe_ras: Handle page offline requests for device memory ecc errors

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

On 01-10-2026 16:20, 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 1/6] drm/xe/xe_ras: Handle page offline requests for
>> device memory ecc errors
>>
>> Add basic support for sending page offline/remove requests to system
>> controller and use it for device memory ECC error handling.
>> Pages that belong to critical BOs cannot be handled by offlining and require a
>> SBR (Secondary Bus Reset).
>> Pages that are configured for log-only handling are not marked as bad by
>> firmware.
>>
>> For all other valid page addresses, the first occurrence of error indicates a
>> poison error and the page is offlined only by software.
>> Firmware avoids permanently marking the page as bad. The second
>> occurrence of an error indicates a Double-bit ECC error and the firmware
>> permanently marks the page as bad.
>>
>> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> ---
>> v2: use ret in sigid logging (Mallesh)
>>      remove additional log
>>      use xe_assert (Michal)
>>
>> v3: align address to PAGE_SIZE (sashiko, Himal)
>>      rename decline to remove
>>      add more descriptive logs (Himal)
>> ---
>>   drivers/gpu/drm/xe/xe_ras.c                   | 127 +++++++++++++++++-
>>   drivers/gpu/drm/xe/xe_ras_types.h             |  35 +++++
>>   drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h |   2 +
>>   3 files changed, 159 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c index
>> 7a85735c57d5..1225c561a872 100644
>> --- a/drivers/gpu/drm/xe/xe_ras.c
>> +++ b/drivers/gpu/drm/xe/xe_ras.c
>> @@ -3,6 +3,8 @@
>>    * Copyright © 2026 Intel Corporation
>>    */
>>
>> +#include "xe_assert.h"
>> +#include "xe_bo.h"
>>   #include "xe_configfs.h"
>>   #include "xe_debugfs.h"
>>   #include "xe_device.h"
>> @@ -16,6 +18,7 @@
>>   #include "xe_sysctrl_event_types.h"
>>   #include "xe_sysctrl_mailbox.h"
>>   #include "xe_sysctrl_mailbox_types.h"
>> +#include "xe_ttm_vram_mgr.h"
>>
>>   #define CORE_COMPUTE_UNCORR_TYPE	GENMASK(26, 25)
>>   /*
>> @@ -201,6 +204,119 @@ static inline const char *comp_to_str(u8
>> component)
>>   	return xe_ras_components[component];
>>   }
>>
>> +static int send_page_offline_cmd(struct xe_device *xe, u64 page_address,
>> +				 enum xe_ras_page_action action)
>> +{
>> +	struct xe_sysctrl_mailbox_command command = {0};
>> +	struct xe_ras_page_offline_request request = {0};
>> +	struct xe_ras_page_offline_response response = {0};
>> +	size_t rlen;
>> +	int ret;
>> +
>> +	if (!xe->info.has_sysctrl)
>> +		return 0;
>> +
>> +	xe_assert(xe, action < XE_RAS_PAGE_ACTION_MAX);
>> +
>> +	request.page_address = page_address;
>> +	request.action = action;
>> +
>> +	if (action == XE_RAS_PAGE_ACTION_OFFLINE)
>> +		xe_log_err(xe, DEVICE_MEMORY, 0, "Requesting firmware to
>> offline page 0x%llx\n",
>> +			   page_address);
>> +	else
>> +		xe_log_err(xe, DEVICE_MEMORY, 0, "Requesting firmware to
>> remove page 0x%llx from queue\n",
>> +			   page_address);
>> +
>> +	xe_sysctrl_create_command(&command, XE_SYSCTRL_GROUP_GFSP,
>> XE_SYSCTRL_CMD_PAGE_OFFLINE,
>> +				  &request, sizeof(request), &response,
>> sizeof(response));
>> +
>> +	ret = xe_sysctrl_send_command(&xe->sc, &command, &rlen);
>> +	if (ret) {
>> +		xe_log_err(xe, SYSCTRL, ret, "failed to send page offline
>> command\n");
>> +		return ret;
>> +	}
>> +
>> +	if (rlen != sizeof(response)) {
>> +		xe_log_err(xe, SYSCTRL, -EINVAL,
>> +			   "unexpected page offline response length %zu
>> (expected %zu)\n",
>> +			   rlen, sizeof(response));
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = ras_status_to_errno(response.status);
>> +	if (ret)
>> +		xe_log_err(xe, SYSCTRL, ret, "page offline command failed with
>> status %u\n",
>> +			   response.status);
>> +
>> +	return ret;
>> +}
>> +
>> +static int handle_page_offline(struct xe_device *xe, u64 page_address,
>> +bool send_cmd) {
>> +	enum xe_ras_page_action action;
>> +	u64 addr;
>> +	int ret = 0;
>> +
>> +	if (!IS_ALIGNED(page_address, XE_PAGE_SIZE)) {
> xe_ttm_vram_handle_addr_fault() itself asserts against PAGE_SIZE, so it'd be more consistent to validate against PAGE_SIZE here too.
>
> Also, please have a look at Sashiko comments if applicable.

This is firmware response check. The response from firmware should be 4k 
aligned.
As suggested by Himal i have aligned the address to the nearest 
page_size below to be consistent with xe_ttm_vram_handle_addr_fault

For sashiko comments

[Severity: Medium]When xe_ttm_vram_handle_addr_fault() encounters an 
out-of-bounds address, does it also return -EOPNOTSUPP?
Will add a configfs check before calling xe_ttm_vram_handle_addr_fault. 
[Severity: High]Does returning 0 here mask the error code from 
xe_ttm_vram_handle_addr_fault()? For -ENOMEM, i can add a ret. But for 
the rest of the errors, triggering sbr or wedging is not required, 
Thanks Riana

>
> Tejas
>
>> +		xe_log_err(xe, SYSCTRL, -EINVAL, "Unaligned physical page
>> address: 0x%llx\n",
>> +			   page_address);
>> +		return -EINVAL;
>> +	}
>> +
>> +	addr = ALIGN_DOWN(page_address, PAGE_SIZE);
>> +
>> +	ret = xe_ttm_vram_handle_addr_fault(xe, addr);
>> +
>> +	/*
>> +	 * Handle return code from address fault handling function:
>> +	 *  0: Page softofflined, remove from firmware queue
>> +	 * -EIO: Address belongs to a critical BO/stolen area that cannot be
>> offlined
>> +	 * -EOPNOTSUPP: Address is valid and can be offlined but user policy is
>> not to offline
>> +	 * -EEXIST: Address is soft offlined but yet to be offlined by firmware
>> for second
>> +	 * occurrence
>> +	 */
> -ENOMEM - allocation failure; next action is reset. --> we should reset on this error.
>
>> +
>> +	switch (ret) {
>> +	case 0:
>> +		action = XE_RAS_PAGE_ACTION_REMOVE;
>> +		xe_log_err(xe, DEVICE_MEMORY, ret,
>> +			   "Poison detected at physical address 0x%llx, page
>> soft-offlined\n",
>> +			   page_address);
>> +		break;
>> +	/* User policy set to decline page offlining */
>> +	case -EOPNOTSUPP:
>> +		action = XE_RAS_PAGE_ACTION_REMOVE;
>> +		xe_log_err(xe, DEVICE_MEMORY, ret,
>> +			   "Poison detected at physical address 0x%llx, user
>> policy set to decline soft-offlining\n",
>> +			   page_address);
>> +		break;
>> +	case -EIO:
>> +		xe_log_err(xe, DEVICE_MEMORY, ret,
>> +			   "Poison detected at physical address 0x%llx, page
>> belongs to critical BO and cannot be soft-offlined\n",
>> +			   page_address);
>> +		return ret;
>> +	case -EEXIST:
>> +		action = XE_RAS_PAGE_ACTION_OFFLINE;
>> +		xe_log_err(xe, DEVICE_MEMORY, ret,
>> +			   "Double-bit ECC error detected at physical address
>> 0x%llx, page soft-offlined\n",
>> +			   page_address);
>> +		break;
>> +	default:
>> +		xe_log_err(xe, DEVICE_MEMORY, ret, "Failed to handle
>> address fault at physical address 0x%llx\n",
>> +			   page_address);
>> +		return 0;
>> +	}
>> +
>> +	if (send_cmd) {
>> +		ret = send_page_offline_cmd(xe, page_address, action);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static bool ras_counter_is_valid(struct xe_device *xe, struct xe_ras_error_class
>> *counter)  {
>>   	u8 severity = counter->common.severity; @@ -368,11 +484,12 @@
>> static u8 handle_soc_internal_errors(struct xe_device *xe, struct
>> xe_ras_error_a  static u8 handle_device_memory_errors(struct xe_device *xe,
>> struct xe_ras_error_array *arr)  {
>>   	struct xe_ras_memory_error *info = (void *)arr->details;
>> +	int ret;
>>
>>   	/*
>>   	 * For memory errors, the recovery action depends on the error
>> category
>>   	 *
>> -	 * TODO: Double-bit ECC errors: Page offlining
>> +	 * Double-bit ECC errors: Page offlining
>>   	 * Poison and data parity errors: Log only
>>   	 * For any other memory errors, request a reset as recovery
>> mechanism
>>   	 */
>> @@ -384,10 +501,10 @@ static u8 handle_device_memory_errors(struct
>> xe_device *xe, struct xe_ras_error_
>>   		xe_info(xe, "[RAS]: Data parity error detected\n");
>>   		break;
>>   	case XE_RAS_MEMORY_DB_ECC:
>> -		xe_info(xe, "[RAS]: Double-bit ECC error detected at sw
>> address 0x%llx\n",
>> -			info->sw_address);
>> -		/* TODO: Add page offlining for Double-bit ECC error */
>> -		fallthrough;
>> +		ret = handle_page_offline(xe, info->sw_address, true);
>> +		if (ret)
>> +			return XE_RAS_RECOVERY_ACTION_RESET;
>> +		break;
>>   	default:
>>   		return XE_RAS_RECOVERY_ACTION_RESET;
>>   	}
>> diff --git a/drivers/gpu/drm/xe/xe_ras_types.h
>> b/drivers/gpu/drm/xe/xe_ras_types.h
>> index fe6f3658a2a4..f119489bcdf2 100644
>> --- a/drivers/gpu/drm/xe/xe_ras_types.h
>> +++ b/drivers/gpu/drm/xe/xe_ras_types.h
>> @@ -17,6 +17,19 @@
>>   #define XE_RAS_MEMORY_POISON			BIT(2)
>>   #define XE_RAS_MEMORY_DATA_PARITY		BIT(5)
>>
>> +/**
>> + * enum xe_ras_page_action - Page offline actions for page offline
>> +request
>> + *
>> + * @XE_RAS_PAGE_ACTION_OFFLINE: Instruct firmware to offline the page
>> + * @XE_RAS_PAGE_ACTION_REMOVE: Instruct firmware to remove the page
>> +from queue
>> + * @XE_RAS_PAGE_ACTION_MAX: Max value
>> + */
>> +enum xe_ras_page_action {
>> +	XE_RAS_PAGE_ACTION_OFFLINE,
>> +	XE_RAS_PAGE_ACTION_REMOVE,
>> +	XE_RAS_PAGE_ACTION_MAX
>> +};
>> +
>>   /**
>>    * enum xe_ras_recovery_action - RAS recovery actions
>>    *
>> @@ -295,6 +308,28 @@ struct xe_ras_memory_error {
>>   	u32 reserved2[10];
>>   } __packed;
>>
>> +/**
>> + * struct xe_ras_page_offline_request - Request for page offline
>> +command  */ struct xe_ras_page_offline_request {
>> +	/** @page_address: Page address (4KB aligned) */
>> +	u64 page_address;
>> +	/** @action: Action to be performed, see &enum xe_ras_page_action
>> */
>> +	u32 action;
>> +	/** @reserved: Reserved for future use */
>> +	u32 reserved;
>> +} __packed;
>> +
>> +/**
>> + * struct xe_ras_page_offline_response - Response from page offline
>> +command  */ struct xe_ras_page_offline_response {
>> +	/** @status: Status of the page offline request */
>> +	u32 status;
>> +	/** @reserved: Reserved for future use */
>> +	u32 reserved;
>> +} __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 c236e5377f30..a01576bf2e73 100644
>> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
>> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
>> @@ -30,6 +30,7 @@ enum xe_sysctrl_group {
>>    * @XE_SYSCTRL_CMD_GET_THRESHOLD: Retrieve error threshold
>>    * @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_HEALTH: Retrieve gpu health
>>    * @XE_SYSCTRL_CMD_SET_HEALTH: Set gpu health
>>    */
>> @@ -40,6 +41,7 @@ enum xe_sysctrl_gfsp_cmd {
>>   	XE_SYSCTRL_CMD_GET_THRESHOLD		= 0x05,
>>   	XE_SYSCTRL_CMD_SET_THRESHOLD		= 0x06,
>>   	XE_SYSCTRL_CMD_GET_PENDING_EVENT	= 0x07,
>> +	XE_SYSCTRL_CMD_PAGE_OFFLINE		= 0x08,
>>   	XE_SYSCTRL_CMD_GET_HEALTH		= 0x0B,
>>   	XE_SYSCTRL_CMD_SET_HEALTH		= 0x0C,
>>   };
>> --
>> 2.47.1