| Message ID | 20241115050733.806934-4-raag.jadav@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce DRM device wedged event | expand |
On 15/11/24 10:37, Raag Jadav wrote: > This was previously attempted as xe specific reset uevent but dropped > in commit 77a0d4d1cea2 ("drm/xe/uapi: Remove reset uevent for now") > as part of refactoring. > > Now that we have device wedged event provided by DRM core, make use > of it and support both driver rebind and bus-reset based recovery. > With this in place userspace will be notified of wedged device, on > the basis of which, userspace may take respective action to recover > the device. > > $ udevadm monitor --property --kernel > monitor will print the received events for: > KERNEL - the kernel uevent > > KERNEL[265.802982] change /devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/card0 (drm) > ACTION=change > DEVPATH=/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/card0 > SUBSYSTEM=drm > WEDGED=rebind,bus-reset > DEVNAME=/dev/dri/card0 > DEVTYPE=drm_minor > SEQNUM=5208 > MAJOR=226 > MINOR=0 LGTM. Reviewed-by: Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com> Thanks, Aravind. > > v2: Change authorship to Himal (Aravind) > Add uevent for all device wedged cases (Aravind) > v3: Generic re-implementation in DRM subsystem (Lucas) > v4: Change authorship to Raag (Aravind) > > Signed-off-by: Raag Jadav <raag.jadav@intel.com> > --- > drivers/gpu/drm/xe/xe_device.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c > index 0e2dd691bdae..5878b331e35c 100644 > --- a/drivers/gpu/drm/xe/xe_device.c > +++ b/drivers/gpu/drm/xe/xe_device.c > @@ -989,11 +989,12 @@ static void xe_device_wedged_fini(struct drm_device *drm, void *arg) > * xe_device_declare_wedged - Declare device wedged > * @xe: xe device instance > * > - * This is a final state that can only be cleared with a mudule > + * This is a final state that can only be cleared with a module > * re-probe (unbind + bind). > * In this state every IOCTL will be blocked so the GT cannot be used. > * In general it will be called upon any critical error such as gt reset > - * failure or guc loading failure. > + * failure or guc loading failure. Userspace will be notified of this state > + * by a DRM uevent. > * If xe.wedged module parameter is set to 2, this function will be called > * on every single execution timeout (a.k.a. GPU hang) right after devcoredump > * snapshot capture. In this mode, GT reset won't be attempted so the state of > @@ -1023,6 +1024,10 @@ void xe_device_declare_wedged(struct xe_device *xe) > "IOCTLs and executions are blocked. Only a rebind may clear the failure\n" > "Please file a _new_ bug report at https://gitlab.freedesktop.org/drm/xe/kernel/issues/new\n", > dev_name(xe->drm.dev)); > + > + /* Notify userspace of wedged device */ > + drm_dev_wedged_event(&xe->drm, > + DRM_WEDGE_RECOVERY_REBIND | DRM_WEDGE_RECOVERY_BUS_RESET); > } > > for_each_gt(gt, xe, id)
On 15-11-2024 10:37, Raag Jadav wrote: > This was previously attempted as xe specific reset uevent but dropped > in commit 77a0d4d1cea2 ("drm/xe/uapi: Remove reset uevent for now") > as part of refactoring. > > Now that we have device wedged event provided by DRM core, make use > of it and support both driver rebind and bus-reset based recovery. > With this in place userspace will be notified of wedged device, on > the basis of which, userspace may take respective action to recover > the device. > > $ udevadm monitor --property --kernel > monitor will print the received events for: > KERNEL - the kernel uevent > > KERNEL[265.802982] change /devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/card0 (drm) > ACTION=change > DEVPATH=/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/card0 > SUBSYSTEM=drm > WEDGED=rebind,bus-reset > DEVNAME=/dev/dri/card0 > DEVTYPE=drm_minor > SEQNUM=5208 > MAJOR=226 > MINOR=0 The patch in itself looks good. Do we have IGT tests in place to validate this uevent ? BR Himal > > v2: Change authorship to Himal (Aravind) > Add uevent for all device wedged cases (Aravind) > v3: Generic re-implementation in DRM subsystem (Lucas) > v4: Change authorship to Raag (Aravind) > > Signed-off-by: Raag Jadav <raag.jadav@intel.com> > --- > drivers/gpu/drm/xe/xe_device.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c > index 0e2dd691bdae..5878b331e35c 100644 > --- a/drivers/gpu/drm/xe/xe_device.c > +++ b/drivers/gpu/drm/xe/xe_device.c > @@ -989,11 +989,12 @@ static void xe_device_wedged_fini(struct drm_device *drm, void *arg) > * xe_device_declare_wedged - Declare device wedged > * @xe: xe device instance > * > - * This is a final state that can only be cleared with a mudule > + * This is a final state that can only be cleared with a module > * re-probe (unbind + bind). > * In this state every IOCTL will be blocked so the GT cannot be used. > * In general it will be called upon any critical error such as gt reset > - * failure or guc loading failure. > + * failure or guc loading failure. Userspace will be notified of this state > + * by a DRM uevent. > * If xe.wedged module parameter is set to 2, this function will be called > * on every single execution timeout (a.k.a. GPU hang) right after devcoredump > * snapshot capture. In this mode, GT reset won't be attempted so the state of > @@ -1023,6 +1024,10 @@ void xe_device_declare_wedged(struct xe_device *xe) > "IOCTLs and executions are blocked. Only a rebind may clear the failure\n" > "Please file a _new_ bug report at https://gitlab.freedesktop.org/drm/xe/kernel/issues/new\n", > dev_name(xe->drm.dev)); > + > + /* Notify userspace of wedged device */ > + drm_dev_wedged_event(&xe->drm, > + DRM_WEDGE_RECOVERY_REBIND | DRM_WEDGE_RECOVERY_BUS_RESET); > } > > for_each_gt(gt, xe, id)
On Tue, Nov 19, 2024 at 10:25:10AM +0530, Ghimiray, Himal Prasad wrote: > On 15-11-2024 10:37, Raag Jadav wrote: > > This was previously attempted as xe specific reset uevent but dropped > > in commit 77a0d4d1cea2 ("drm/xe/uapi: Remove reset uevent for now") > > as part of refactoring. > > > > Now that we have device wedged event provided by DRM core, make use > > of it and support both driver rebind and bus-reset based recovery. > > With this in place userspace will be notified of wedged device, on > > the basis of which, userspace may take respective action to recover > > the device. > > > > $ udevadm monitor --property --kernel > > monitor will print the received events for: > > KERNEL - the kernel uevent > > > > KERNEL[265.802982] change /devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/card0 (drm) > > ACTION=change > > DEVPATH=/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/card0 > > SUBSYSTEM=drm > > WEDGED=rebind,bus-reset > > DEVNAME=/dev/dri/card0 > > DEVTYPE=drm_minor > > SEQNUM=5208 > > MAJOR=226 > > MINOR=0 > > > The patch in itself looks good. Do we have IGT tests in place to validate > this uevent ? I unit tested it with documented example which seems to work. We can have an igt in place once we have acceptance from the community. Raag
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c index 0e2dd691bdae..5878b331e35c 100644 --- a/drivers/gpu/drm/xe/xe_device.c +++ b/drivers/gpu/drm/xe/xe_device.c @@ -989,11 +989,12 @@ static void xe_device_wedged_fini(struct drm_device *drm, void *arg) * xe_device_declare_wedged - Declare device wedged * @xe: xe device instance * - * This is a final state that can only be cleared with a mudule + * This is a final state that can only be cleared with a module * re-probe (unbind + bind). * In this state every IOCTL will be blocked so the GT cannot be used. * In general it will be called upon any critical error such as gt reset - * failure or guc loading failure. + * failure or guc loading failure. Userspace will be notified of this state + * by a DRM uevent. * If xe.wedged module parameter is set to 2, this function will be called * on every single execution timeout (a.k.a. GPU hang) right after devcoredump * snapshot capture. In this mode, GT reset won't be attempted so the state of @@ -1023,6 +1024,10 @@ void xe_device_declare_wedged(struct xe_device *xe) "IOCTLs and executions are blocked. Only a rebind may clear the failure\n" "Please file a _new_ bug report at https://gitlab.freedesktop.org/drm/xe/kernel/issues/new\n", dev_name(xe->drm.dev)); + + /* Notify userspace of wedged device */ + drm_dev_wedged_event(&xe->drm, + DRM_WEDGE_RECOVERY_REBIND | DRM_WEDGE_RECOVERY_BUS_RESET); } for_each_gt(gt, xe, id)
This was previously attempted as xe specific reset uevent but dropped in commit 77a0d4d1cea2 ("drm/xe/uapi: Remove reset uevent for now") as part of refactoring. Now that we have device wedged event provided by DRM core, make use of it and support both driver rebind and bus-reset based recovery. With this in place userspace will be notified of wedged device, on the basis of which, userspace may take respective action to recover the device. $ udevadm monitor --property --kernel monitor will print the received events for: KERNEL - the kernel uevent KERNEL[265.802982] change /devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/card0 (drm) ACTION=change DEVPATH=/devices/pci0000:00/0000:00:01.0/0000:01:00.0/0000:02:01.0/0000:03:00.0/drm/card0 SUBSYSTEM=drm WEDGED=rebind,bus-reset DEVNAME=/dev/dri/card0 DEVTYPE=drm_minor SEQNUM=5208 MAJOR=226 MINOR=0 v2: Change authorship to Himal (Aravind) Add uevent for all device wedged cases (Aravind) v3: Generic re-implementation in DRM subsystem (Lucas) v4: Change authorship to Raag (Aravind) Signed-off-by: Raag Jadav <raag.jadav@intel.com> --- drivers/gpu/drm/xe/xe_device.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)