[Intel-xe] [PATCH v2 05/10] drm/xe: Notify userspace about GSC HW errors.

[Welty, Brian]

On 10/17/2023 9:00 PM, Himal Prasad Ghimiray wrote:
> Send uevent incase of nonfatal errors reported by gsc.
> 
> v2
> - No need to provide tile info in uevent because error is
> accepted only in tile0. (Aravind)

This is user API.  Don't we need to demonstrate user-space consumer of 
this before merge?  Do we at least have IGT?  Also see question below.

> 
> Cc: Aravind Iddamsetty <aravind.iddamsetty at intel.com>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
> ---
>   drivers/gpu/drm/xe/xe_device_types.h |  3 +++
>   drivers/gpu/drm/xe/xe_hw_error.c     | 16 ++++++++++++++++
>   drivers/gpu/drm/xe/xe_hw_error.h     |  3 ++-
>   drivers/gpu/drm/xe/xe_irq.c          |  4 ++++
>   include/uapi/drm/xe_drm.h            |  8 ++++++++
>   5 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 2998ee517f0d..d2ee5549d20c 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -196,6 +196,9 @@ struct xe_tile {
>   	struct tile_hw_errors {
>   		struct xarray hw_error;
>   	} errors;
> +
> +	/** @gsc_hw_err_work: worker for uevent to report GSC HW errors */
> +	struct work_struct gsc_hw_err_work;
>   };
>   
>   /**
> diff --git a/drivers/gpu/drm/xe/xe_hw_error.c b/drivers/gpu/drm/xe/xe_hw_error.c
> index 1e94ee72a34f..9ac817c1dd03 100644
> --- a/drivers/gpu/drm/xe/xe_hw_error.c
> +++ b/drivers/gpu/drm/xe/xe_hw_error.c
> @@ -3,6 +3,8 @@
>    * Copyright © 2023 Intel Corporation
>    */
>   
> +#include <drm/xe_drm.h>
> +
>   #include "xe_hw_error.h"
>   
>   #include "regs/xe_regs.h"
> @@ -387,6 +389,19 @@ xe_gt_hw_error_handler(struct xe_gt *gt, const enum hardware_error hw_err)
>   		xe_gt_hw_error_log_vector_reg(gt, hw_err);
>   }
>   
> +void xe_gsc_hw_error_work(struct work_struct *work)
> +{
> +	struct xe_tile *tile = container_of(work, typeof(*tile), gsc_hw_err_work);
> +	char *csc_hw_error_event[3];
> +
> +	csc_hw_error_event[0] = XE_GSC_HW_HEALTH_UEVENT "=1";
> +	csc_hw_error_event[1] = "RESET_REQUIRED=1";
> +	csc_hw_error_event[2] = NULL;
> +
> +	kobject_uevent_env(&tile->xe->drm.primary->kdev->kobj, KOBJ_CHANGE,
> +			   csc_hw_error_event);
> +}
> +
...
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index d48d8e3c898c..06f6fce1531d 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -16,6 +16,14 @@ extern "C" {
>    * subject to backwards-compatibility constraints.
>    */
>   
> +/**
> + * DOC: uevent generated by xe on it's tile node.

What does 'tile node' mean?  If I'm not mistaken, isn't
tile->xe->drm.primary->kdev->kobj just the device node?

> + *
> + * XE_GSC_HW_HEALTH_UEVENT - Event is generated when GSC reports HW
> + * errors. The value supplied with the event is always "RESET_REQUIRED=1".
> + */
> +#define XE_GSC_HW_HEALTH_UEVENT "DEVICE_STATUS"

So both XE_RESET_FAILED_UEVENT and XE_GSC_HW_HEALTH_UEVENT are defined
as "DEVICE_STATUS".  Should be unique or isn't this going to be
confusing to the user reading these events?

-Brian


> +
>   /**
>    * DOC: uevent generated by xe on it's pci node.
>    *