[PATCH v4] drm/xe: Add helper function to inject fault into ct_dead_capture()
John Harrison
john.c.harrison at intel.com
Wed May 21 21:31:29 UTC 2025
On 5/15/2025 3:19 AM, Satyanarayana K V P wrote:
> When injecting fault to xe_guc_ct_send_recv() & xe_guc_mmio_send_recv()
> functions, the CI test systems are going out of space and crashing. To
> avoid this issue, a new helper function is created and when fault is
> injected into this xe_is_injection_active() helper function,
> ct dead capture is avoided which suppresses ct dumps in the log.
>
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p at intel.com>
> Suggested-by: John Harrison <John.C.Harrison at Intel.com>
> ---
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Jani Nikula <jani.nikula at intel.com>
>
> V3 -> V4:
> - Updated the name of helper function and moved to xe_device.h file.
>
> V2 -> V3:
> - Added inline function to avoid compilation error in the absence of
> CONFIG_FUNCTION_ERROR_INJECTION.
>
> V1 -> V2:
> - Fixed review comments.
> ---
> drivers/gpu/drm/xe/xe_device.h | 18 ++++++++++++++++++
> drivers/gpu/drm/xe/xe_guc_ct.c | 6 ++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> index 0bc3bc8e6803..b937d585083b 100644
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -195,6 +195,24 @@ void xe_device_declare_wedged(struct xe_device *xe);
> struct xe_file *xe_file_get(struct xe_file *xef);
> void xe_file_put(struct xe_file *xef);
>
> +/**
> + * is_injection_active() - Helper function to inject fault.
Technically, this is not injecting the fault. It is a place holder to
have a fault injected in to so that it can report to other code that a
fault injection test is running.
> + *
> + * This is a helper function to inject fault into ct_dead_capture().
Given that this is now a generic function in a a generic location, it
should not really talk about specific instances of usage.
I would say something like:
+/**
+ * is_injection_active() - Helper function to test whether a fault
inject test is running.
+ *
+ * This is a helper function that the driver can use to detect whether
a fault injection
+ * test is running in order to suppress excessive debug output. By
default, the return
+ * value is fixed as zero but it can be modified by the fault injection
framework to
+ * return an error.
+ *
+ * Returns:
+ * 0 if fault injection is not
+ *-EINVAL if fault injection is active
Or whatever error was picked by the IGT side?
> + * As fault is injected using this function, need to make sure that
> + * the compiler does not optimize and make it as a inline function.
> + * To prevent compile optimization, "noinline" is added.
> + */
> +#ifdef CONFIG_FUNCTION_ERROR_INJECTION
> +static noinline int xe_is_injection_active(void)
> +{
> + return 0;
Is the indentation correct here? It is showing as double intended in the
email reader.
> +}
> +ALLOW_ERROR_INJECTION(xe_is_injection_active, ERRNO);
> +#else
> +static inline int xe_is_injection_active(void) { return 0; }
Why have the first version spread over three lines but this version
compressed to a single line? The code is identical except for 'noinline'
vs 'inline' isn't it?
You could even do it as:
+#ifdef CONFIG_FUNCTION_ERROR_INJECTION
+ALLOW_ERROR_INJECTION(xe_is_injection_active, ERRNO);
+static noinline
+#else
+static inline
+#endif
+int xe_is_injection_active(void)
+{
+ return 0;
+}
Although I guess the ALLOW_ macro needs to be after the function definition?
John.
> +#endif
> +
> /*
> * Occasionally it is seen that the G2H worker starts running after a delay of more than
> * a second even after being queued and activated by the Linux workqueue subsystem. This
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index ff6edf1b14f4..a6964d13f3a3 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -1963,6 +1963,12 @@ static void ct_dead_capture(struct xe_guc_ct *ct, struct guc_ctb *ctb, u32 reaso
>
> if (ctb)
> ctb->info.broken = true;
> + /*
> + * Huge dump is getting generated when injecting error for guc CT/MMIO
> + * functions. So, let us suppress the dump when fault is injected.
> + */
> + if (xe_is_injection_active())
> + return;
>
> /* Ignore further errors after the first dump until a reset */
> if (ct->dead.reported)
More information about the Intel-xe
mailing list