Re: [PATCH 1/2] drm/xe/xe_ras: Add Fabric error handling

[Raag Jadav <raag.jadav@intel.com>]

On Tue, Aug 25, 2026 at 07:41:11PM +0530, Riana Tauro wrote:
> SAF MHB errors are collected and classifed under Fabric error component

Is 'SAF MHB' expected to be something obvious? For mere mortals like
me it's definitely not. Could you please elaborate?

> by System controller. For SAF_MHB errors, if the cause is data
> payload parity error then log and return.
> 
> For all other errors and causes, request a Secondary Bus Reset (SBR)

Punctuation please!

> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_ras.c       | 20 ++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_ras_types.h |  4 ++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index d25d25f77531..586dc28a638a 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -394,6 +394,23 @@ static u8 handle_device_memory_errors(struct xe_device *xe, struct xe_ras_error_
>  	return XE_RAS_RECOVERY_ACTION_RECOVERED;
>  }
>  
> +static u8 handle_fabric_errors(struct xe_device *xe, struct xe_ras_error_array *arr)
> +{
> +	struct xe_ras_error_product *product = &arr->counter.product;
> +	struct xe_ras_ieh_error *info = (void *)arr->details;
> +
> +	if ((info->global_error_status & XE_RAS_FAB_IEH_SAF_MHB) &&
> +	    product->cause.cause == XE_RAS_FAB_CAUSE_PAYLOAD) {
> +		xe_log_comp_recoverable(xe, FABRIC, &arr->counter, sizeof(arr->counter),
> +					"SAF MHB error detected\n");
> +		return XE_RAS_RECOVERY_ACTION_RECOVERED;
> +	}
> +
> +	xe_log_comp_fatal(xe, FABRIC, &arr->counter, sizeof(arr->counter),
> +			  "Other errors detected\n");

'Other' errors isn't telling me anything, nor they are documented here.
If we don't know what they are can we use 'unknown' instead?

> +	return XE_RAS_RECOVERY_ACTION_RESET;
> +}
> +
>  void xe_ras_counter_threshold_crossed(struct xe_device *xe,
>  				      struct xe_sysctrl_event_response *response)
>  {
> @@ -555,6 +572,9 @@ enum xe_ras_recovery_action xe_ras_process_errors(struct xe_device *xe)
>  			case XE_RAS_COMP_DEVICE_MEMORY:
>  				action = handle_device_memory_errors(xe, arr);
>  				break;
> +			case XE_RAS_COMP_FABRIC:
> +				action = handle_fabric_errors(xe, arr);
> +				break;
>  			default:
>  				/* For any other component, reset */
>  				action = 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 99b2466e2062..73517fd0d415 100644
> --- a/drivers/gpu/drm/xe/xe_ras_types.h
> +++ b/drivers/gpu/drm/xe/xe_ras_types.h
> @@ -12,6 +12,10 @@
>  #define XE_RAS_NUM_ERROR_ARR			3
>  /* Error bits in IEH global error status register */
>  #define XE_RAS_SOC_IEH_PUNIT			BIT(1)
> +/* Bits 16-31 represent individual SAF MHB unit */
> +#define XE_RAS_FAB_IEH_SAF_MHB			GENMASK(31, 16)
> +/* Fabric Data payload parity errors */
> +#define XE_RAS_FAB_CAUSE_PAYLOAD		BIT(2)

Nit: Better to have a blank line between defs if they're unrelated,
otherwise they look like part of same register.

Raag

>  /* Device memory error categories */
>  #define XE_RAS_MEMORY_DB_ECC			BIT(1)
>  #define XE_RAS_MEMORY_POISON			BIT(2)
> -- 
> 2.47.1
>