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

Rodrigo Vivi <[email protected]>
Newsgroups org.freedesktop.lists.intel-xe,org.freedesktop.lists.dri-devel
Message-ID <[email protected]>
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 <[email protected]>
> ---
>  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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.