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

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

On 07-09-2026 15:17, Riana Tauro wrote:
> Add basic support for sending page offline/decline 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)
> ---
>   drivers/gpu/drm/xe/xe_ras.c                   | 118 +++++++++++++++++-
>   drivers/gpu/drm/xe/xe_ras_types.h             |  35 ++++++
>   drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h |   2 +
>   3 files changed, 150 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index 7a85735c57d5..94ffd0852938 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,110 @@ 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;
> +
> +	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;
> +	}
> +
> +	return ret;
> +}
> +
> +static int handle_page_offline(struct xe_device *xe, u64 page_address, bool send_cmd)
> +{
> +	enum xe_ras_page_action action;
> +	int ret = 0;
> +
> +	if (!IS_ALIGNED(page_address, XE_PAGE_SIZE)) {
> +		xe_log_err(xe, SYSCTRL, -EINVAL, "Unaligned physical page address: 0x%llx\n",
> +			   page_address);
> +		return -EINVAL;

IS SBR right behaviour here, if FW sends unaligned addr ?

> +	}


FW address format should still be validated as 4K aligned, but the 
address handed to the current VRAM allocator path needs to be normalized 
to PAGE_SIZE, because XE offlining is allocator-granularity 
based(PAGE_SIZE, which can be different from 4K on some archs), not 
4K-granularity based.


> +
> +	ret = xe_ttm_vram_handle_addr_fault(xe, page_address);
> +
> +	/*
> +	 * Handle return code from address fault handling function:
> +	 *  0: Page soft offlined, decline to firmware
> +	 * -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
> +	 */
> +
> +	switch (ret) {
> +	case 0:
> +		action = XE_RAS_PAGE_ACTION_DECLINE;

Nit: The action name XE_RAS_PAGE_ACTION_DECLINE sounds inappropriate 
here, the action requested to FW is to remove page from queue not 
necessarily decline the offlining, the next error on same addr will show 
as double bit ecc and we do offlining.
HOW about:
XE_RAS_PAGE_ACTION_REMOVE_FROM_QUEUE


Nit: How about Loging:
FW->KMD(BEHAVIOR), DRIVER HANDLING, KMD->FW(ACTION REQUEST)
case 0:
"Poison detected at physical address, page software offlined, requested 
page removal from queue"
case -EOPNOTSUPP:
"Poison detected at physical address, User policy set to decline page 
offlining, requested page removal from queue"



> +		xe_log_err(xe, DEVICE_MEMORY, ret,
> +			   "Poison detected at physical address 0x%llx, page software offlined\n",
> +			   page_address);
> +		break;
> +	/* User policy set to decline page offlining */
> +	case -EOPNOTSUPP:
> +		action = XE_RAS_PAGE_ACTION_DECLINE;
> +		xe_log_err(xe, DEVICE_MEMORY, ret,
> +			   "User policy set to decline page offlining for physical address 0x%llx\n",
> +			   page_address);
> +		break;
> +	case -EIO:
> +		xe_log_err(xe, DEVICE_MEMORY, ret,
> +			   "Physical page address belongs to critical BO: 0x%llx\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 already software offlined\n",
> +			   page_address);
> +		break;
> +	default:
> +		xe_log_err(xe, DEVICE_MEMORY, ret, "Failed to handle address fault 0x%llx\n",
> +			   page_address);
> +		return 0;

Logically looks ok to return 0 here.
> +	}
> +
> +	if (send_cmd) {
> +		ret = send_page_offline_cmd(xe, page_address, action);

Same question as above, does failure in send_page_offline_cmd should 
lead to SBR ?

> +		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 +475,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 +492,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..20c74593ce05 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_DECLINE: 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_DECLINE,
> +	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 66e7cbcc3f91..590dd408399c 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox_types.h
> @@ -28,6 +28,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/decline a page
>    * @XE_SYSCTRL_CMD_GET_HEALTH: Retrieve gpu health
>    * @XE_SYSCTRL_CMD_SET_HEALTH: Set gpu health
>    */
> @@ -38,6 +39,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,
>   };