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

[Ghimiray, Himal Prasad]

Hi Brian,

> -----Original Message-----
> From: Welty, Brian <brian.welty at intel.com>
> Sent: 19 October 2023 06:22
> To: Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>; intel-
> xe at lists.freedesktop.org
> Subject: Re: [Intel-xe] [PATCH v2 05/10] drm/xe: Notify userspace about GSC
> HW errors.
> 
> 
> 
> 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.

https://one-api.gitlab-pages.devtools.intel.com/level_zero/sysman/api.html#_CPPv441ZES_EVENT_TYPE_FLAG_DEVICE_RESET_REQUIRED is the consumer for this uevent.
The hardware error injection via IGT is not possible. It explicitly needs python SV and ITP.
> 
> >
> > 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?
Miss from my end. It should be 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?

The values provided are different and uevents are on different nodes. 
XE_RESET_FAILED_UEVENT is at pci node of xe device whereas XE_GSC_HW_HEALTH_UEVENT
Is at device node.

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