Re: [PATCH v12 4/4] drm/xe/ras: Use fault-inject to trigger punit error handler

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Jul 24, 2026 at 03:33:07PM +0530, Mallesh Koujalagi wrote:
> Use fault-inject framework to trigger punit_error_handler()
> for testing.
> 
> Usage:
>   echo 100 > .../inject_punit_error/probability
>   echo 1   > .../inject_punit_error/times
> 
> Signed-off-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_debugfs.c |  4 ++++
>  drivers/gpu/drm/xe/xe_debugfs.h |  2 ++
>  drivers/gpu/drm/xe/xe_ras.c     | 17 +++++++++++++++++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index 5a3877fcb0f0..5599d493feca 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -42,6 +42,7 @@
>  
>  DECLARE_FAULT_ATTR(gt_reset_failure);
>  DECLARE_FAULT_ATTR(inject_csc_hw_error);
> +DECLARE_FAULT_ATTR(inject_punit_error);
>  
>  static bool csc_hw_error_available(struct xe_device *xe)
>  {
> @@ -62,6 +63,8 @@ static struct {
>  	{ .name = "inject_csc_hw_error",
>  	  .attr = &inject_csc_hw_error,
>  	  .is_visible = csc_hw_error_available },
> +	{ .name = "inject_punit_error",
> +	  .attr = &inject_punit_error },
>  };
>  
>  /*
> @@ -76,6 +79,7 @@ bool xe_fault_##name(void)				\
>  
>  FAULT_ACTION(gt_reset, gt_reset_failure)
>  FAULT_ACTION(csc_hw_error, inject_csc_hw_error)
> +FAULT_ACTION(punit_error, inject_punit_error)
>  
>  static void xe_fault_inject_debugfs_register(struct xe_device *xe,
>  					     struct dentry *root)
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.h b/drivers/gpu/drm/xe/xe_debugfs.h
> index cd56f7442b99..dd57914dd4f2 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.h
> +++ b/drivers/gpu/drm/xe/xe_debugfs.h
> @@ -13,10 +13,12 @@ struct xe_device;
>  #ifdef CONFIG_DEBUG_FS
>  bool xe_fault_gt_reset(void);
>  bool xe_fault_csc_hw_error(void);
> +bool xe_fault_punit_error(void);
>  void xe_debugfs_register(struct xe_device *xe);
>  #else
>  static inline bool xe_fault_gt_reset(void) { return false; }
>  static inline bool xe_fault_csc_hw_error(void) { return false; }
> +static inline bool xe_fault_punit_error(void) { return false; }
>  static inline void xe_debugfs_register(struct xe_device *xe) { }
>  #endif
>  
> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index 92b4181026cb..da06c16cf7ed 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -3,6 +3,7 @@
>   * Copyright © 2026 Intel Corporation
>   */
>  
> +#include "xe_debugfs.h"
>  #include "xe_device.h"
>  #include "xe_drm_ras.h"
>  #include "xe_pm.h"
> @@ -391,6 +392,22 @@ enum xe_ras_recovery_action xe_ras_process_errors(struct xe_device *xe)
>  	size_t rlen;
>  	int ret;
>  
> +	/*
> +	 * Only allow the injected PUNIT error once the DRM device is registered.

That entirely defeats the purpose! :)

Sashiko identified this corner case so I asked you to ensure that this is covered
and to prove that you should provide a fault-inject with test case.

But then you are skipping exactly the corner case. That makes absolutely no sense.

> +	 * xe_ras_process_errors() also runs during probe (via xe_ras_init()), before
> +	 * drm_dev_register() calls device_add() on the DRM minor's kdev. At that
> +	 * point kdev->kobj.parent is not yet linked into the sysfs hierarchy, so
> +	 * kobject_get_path() returns "/card0" instead of the real sysfs path.
> +	 * drm_dev_wedged_event() would then emit a KOBJ_CHANGE uevent with a wrong
> +	 * DEVPATH that udev cannot resolve, silently dropping the event and leaving
> +	 * the cold-reset recovery broken.
> +	 */
> +	if (xe->drm.registered && xe_fault_punit_error()) {
> +		xe_err(xe, "[RAS]: PUNIT error injected\n");
> +		punit_error_handler(xe);
> +		return XE_RAS_RECOVERY_ACTION_DISCONNECT;
> +	}
> +
>  	if (!xe->info.has_sysctrl)
>  		return XE_RAS_RECOVERY_ACTION_RESET;
>  
> -- 
> 2.48.1
>