Re: [PATCH v4 1/3] drm/xe: Add error_injectable define

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 8/27/2025 8:26 PM, Jonathan Cavitt wrote:
> Add a declaration for the function attribute "error_injectable", which
> appends noinline to functions tagged with ALLOW_ERROR_INJECTION only if
> CONFIG_FUNCTION_ERROR_INJECTION is enabled.

this really doesn't answer the question "why"

>  This function attribute was
> added to drivers/gpu/drm/xe/xe_macros.h, 

this can be clearly seen in diff

> and has been applied to
> all Xe functions with ALLOW_ERROR_INJECTION enabled.
> 
> v2:
> - Move error_injectable from error-injection.h to xe_macros.h
>   (Wajdeczko)

it would be better to just say (Michal)

> - Reformat function attribute application to minimize diff size (Brost)
> 
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Suggested-by: John Harrison <john.c.harrison@intel.com>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device.c                |  3 +++
>  drivers/gpu/drm/xe/xe_exec_queue.c            |  2 ++
>  drivers/gpu/drm/xe/xe_ggtt.c                  |  1 +
>  drivers/gpu/drm/xe/xe_guc.c                   |  1 +
>  drivers/gpu/drm/xe/xe_guc_ads.c               |  1 +
>  drivers/gpu/drm/xe/xe_guc_ct.c                | 20 +++++--------------
>  drivers/gpu/drm/xe/xe_guc_log.c               |  1 +
>  drivers/gpu/drm/xe/xe_guc_relay.c             |  1 +
>  drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c |  2 ++
>  drivers/gpu/drm/xe/xe_hw_engine_group.c       |  1 +
>  drivers/gpu/drm/xe/xe_macros.h                |  6 ++++++
>  drivers/gpu/drm/xe/xe_mmio.c                  |  1 +
>  drivers/gpu/drm/xe/xe_oa.c                    |  1 +
>  drivers/gpu/drm/xe/xe_pcode.c                 |  2 ++
>  drivers/gpu/drm/xe/xe_pm.c                    |  1 +
>  drivers/gpu/drm/xe/xe_pt.c                    |  3 +++
>  drivers/gpu/drm/xe/xe_sriov.c                 |  2 ++
>  drivers/gpu/drm/xe/xe_sync.c                  |  1 +
>  drivers/gpu/drm/xe/xe_tile.c                  |  1 +
>  drivers/gpu/drm/xe/xe_tuning.c                |  2 ++
>  drivers/gpu/drm/xe/xe_uc_fw.c                 |  1 +
>  drivers/gpu/drm/xe/xe_vm.c                    |  5 +++++
>  drivers/gpu/drm/xe/xe_wa.c                    |  2 ++
>  drivers/gpu/drm/xe/xe_wopcm.c                 |  1 +
>  24 files changed, 47 insertions(+), 15 deletions(-)
> 

...

> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index 3f4e6a46ff16..a7b7045fc732 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -213,6 +213,7 @@ static void primelockdep(struct xe_guc_ct *ct)
>  	fs_reclaim_release(GFP_KERNEL);
>  }
>  
> +error_injectable
>  int xe_guc_ct_init_noalloc(struct xe_guc_ct *ct)
>  {
>  	struct xe_device *xe = ct_to_xe(ct);
> @@ -252,6 +253,7 @@ int xe_guc_ct_init_noalloc(struct xe_guc_ct *ct)
>  }
>  ALLOW_ERROR_INJECTION(xe_guc_ct_init_noalloc, ERRNO); /* See xe_pci_probe() */
>  
> +error_injectable
>  int xe_guc_ct_init(struct xe_guc_ct *ct)
>  {
>  	struct xe_device *xe = ct_to_xe(ct);
> @@ -1149,6 +1151,7 @@ static int guc_ct_send_recv(struct xe_guc_ct *ct, const u32 *action, u32 len,
>   *         DATA0 from `HXG Response`_ if &response_buffer was NULL, or
>   *         a negative error code on failure.
>   */
> +error_injectable
>  int xe_guc_ct_send_recv(struct xe_guc_ct *ct, const u32 *action, u32 len,
>  			u32 *response_buffer)
>  {
> @@ -2069,22 +2072,9 @@ void xe_guc_ct_print(struct xe_guc_ct *ct, struct drm_printer *p, bool want_ctb)
>  
>  #if IS_ENABLED(CONFIG_DRM_XE_DEBUG)
>  
> -#ifdef CONFIG_FUNCTION_ERROR_INJECTION
> -/*
> - * This is a helper function which assists the driver in identifying if a fault
> - * injection test is currently active, allowing it to reduce unnecessary debug
> - * output. Typically, the function returns zero, but the fault injection
> - * framework can alter this to return an error. Since faults are injected

you can use some parts of below explanation in your commit msg

> - * through this function, it's important to ensure the compiler doesn't optimize
> - * it into an inline function. To avoid such optimization, the 'noinline'
> - * attribute is applied. 

....

> diff --git a/drivers/gpu/drm/xe/xe_macros.h b/drivers/gpu/drm/xe/xe_macros.h
> index 8a77c2423555..0d09a213eaba 100644
> --- a/drivers/gpu/drm/xe/xe_macros.h
> +++ b/drivers/gpu/drm/xe/xe_macros.h
> @@ -19,4 +19,10 @@
>  	cond__;								\
>  })
>  

and it wouldn't hurt if we also put some description here with
how/when to use below macro
> +#ifdef CONFIG_FUNCTION_ERROR_INJECTION
> +#define error_injectable noinline
> +#else
> +#define error_injectable
> +#endif
> +
>  #endif