Re: [Intel-xe] [PATCH v8 2/3] drm/xe: Notify Userspace when gt reset fails

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Jul 26, 2023 at 10:44:12PM +0530, Ghimiray, Himal Prasad wrote:
> Hi Rodrigo,
> 
> On 26-07-2023 20:09, Rodrigo Vivi wrote:
> > On Tue, Jul 25, 2023 at 09:21:14PM +0530, Himal Prasad Ghimiray wrote:
> > > Send uevent in case of gt reset failure. This intimation can be used by
> > > userspace monitoring tool to do the device level reset/reboot
> > > when GT reset fails. udevadm can be used to monitor the uevents.
> > > 
> > > v2:
> > > - Support only gt failure notification (Rodrigo)
> > > 
> > > v3
> > > - Rectify the comments in header file.
> > > 
> > > v4
> > > - Use pci kobj instead of drm kobj for notification.(Rodrigo)
> > > - Cleanup (Badal)
> > > 
> > > Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
> > > Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>
> > > Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> > Cc: Matt Roper Matt Roper <matthew.d.roper@intel.com>
> > 
> > > ---
> > >   drivers/gpu/drm/xe/xe_gt.c | 17 +++++++++++++++++
> > >   include/uapi/drm/xe_drm.h  |  8 ++++++++
> > >   2 files changed, 25 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > > index 3e32d38aeeea..f4766fb6bfdb 100644
> > > --- a/drivers/gpu/drm/xe/xe_gt.c
> > > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > > @@ -8,6 +8,7 @@
> > >   #include <linux/minmax.h>
> > >   #include <drm/drm_managed.h>
> > > +#include <drm/xe_drm.h>
> > >   #include "regs/xe_gt_regs.h"
> > >   #include "xe_bb.h"
> > > @@ -500,6 +501,19 @@ static int do_gt_restart(struct xe_gt *gt)
> > >   	return 0;
> > >   }
> > > +static void xe_uevent_gt_reset_failure(struct pci_dev *pdev, u8 id)
> > > +{
> > > +	char *reset_event[4];
> > > +
> > > +	reset_event[0] = XE_RESET_FAILED_UEVENT "=NEEDS_RESET";
> > > +	reset_event[1] = "RESET_FAILED=gt";
> > > +	reset_event[2] = kasprintf(GFP_KERNEL, "RESET_ID=%d", id);
> > should we also put which tile this is coming from?
> > Matt?
> > 
> > > +	reset_event[3] = NULL;
> > > +	kobject_uevent_env(&pdev->dev.kobj, KOBJ_CHANGE, reset_event);
> > Himal, could you please paste here an example of the output of this event
> > when monitoring it with the:
> > $ udevadm monitor
> > ?
> 
> Please find the output from udevadm  monitor below

this is really great. Thank you.
(more below)

> 
> 
> KERNEL[471.352287] change
> /devices/pci0000:89/0000:89:02.0/0000:8a:00.0/0000:8b:01.0/0000:8c:00.0
> (pci)
> ACTION=change
> DEVPATH=/devices/pci0000:89/0000:89:02.0/0000:8a:00.0/0000:8b:01.0/0000:8c:00.0

Since it is at the PCI level, we need to identify the tile.

Maybe:
TILE=%id?
reset_event[x] = kasprintf(GFP_KERNEL, "TILE_ID=%d", id);

> SUBSYSTEM=pci
> DEVICE_STATUS=NEEDS_RESET
> RESET_FAILED=gt

What could be the other RESET_FAILED options?
Could we get some code documentation along with this?

> RESET_ID=0

maybe s/RESET_ID/GT_ID ?
reset_event[x] = kasprintf(GFP_KERNEL, "GT_ID=%d", id);

> DRIVER=xe
> PCI_CLASS=38000
> PCI_ID=8086:0BD6
> PCI_SUBSYS_ID=8086:0000
> PCI_SLOT_NAME=0000:8c:00.0
> MODALIAS=pci:v00008086d00000BD6sv00008086sd00000000bc03sc80i00
> SEQNUM=8817
> 
> BR
> 
> Himal
> 
> 
> > 
> > > +
> > > +	kfree(reset_event[2]);
> > > +}
> > > +
> > >   static int gt_reset(struct xe_gt *gt)
> > >   {
> > >   	int err;
> > > @@ -550,6 +564,9 @@ static int gt_reset(struct xe_gt *gt)
> > >   	xe_device_mem_access_put(gt_to_xe(gt));
> > >   	xe_gt_err(gt, "reset failed (%pe)\n", ERR_PTR(err));
> > > +	/* Notify userspace about gt reset failure */
> > > +	xe_uevent_gt_reset_failure(to_pci_dev(gt_to_xe(gt)->drm.dev), gt->info.id);
> > > +
> > >   	return err;
> > >   }
> > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> > > index 347351a8f618..fdacee0a27c5 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.
> > >    */
> > > +/*
> > > + * Uevent generated by xe on it's pci node.
> > > + *
> > > + * XE_RESET_FAILED_UEVENT - Event is generated when attempt to reset engine
> > > + * fails. The value supplied with the event is always "NEEDS_RESET".
> > > + */
> > > +#define XE_RESET_FAILED_UEVENT "DEVICE_STATUS"
> > > +
> > >   /**
> > >    * struct xe_user_extension - Base class for defining a chain of extensions
> > >    *
> > > -- 
> > > 2.25.1
> > >