| Message ID | 20240917040235.197019-4-raag.jadav@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce DRM device wedged event | expand |
On 17-09-2024 09:32, 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 supported by DRM core, make use > of it. 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. As per earlier discussions, the UAPI was also supposed to provide the reason for wedging( which is supposedly used by L0). IS that requirement nomore in place ? BR Himal > > $ 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=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 | 17 +++++++++++++++-- > drivers/gpu/drm/xe/xe_device.h | 1 + > drivers/gpu/drm/xe/xe_pci.c | 2 ++ > 3 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c > index 4d3c794f134c..1b097643aacb 100644 > --- a/drivers/gpu/drm/xe/xe_device.c > +++ b/drivers/gpu/drm/xe/xe_device.c > @@ -780,6 +780,15 @@ int xe_device_probe(struct xe_device *xe) > return err; > } > > +void xe_setup_wedge_recovery(struct xe_device *xe) > +{ > + struct drm_device *dev = &xe->drm; > + > + /* Support both driver rebind and bus reset based recovery. */ > + set_bit(DRM_WEDGE_RECOVERY_REBIND, &dev->wedge_recovery); > + set_bit(DRM_WEDGE_RECOVERY_BUS_RESET, &dev->wedge_recovery); > +} > + > static void xe_device_remove_display(struct xe_device *xe) > { > xe_display_unregister(xe); > @@ -986,11 +995,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 > @@ -1020,6 +1030,9 @@ 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_BUS_RESET); > } > > for_each_gt(gt, xe, id) > diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h > index ca8d8ef6342b..77a2332b4b87 100644 > --- a/drivers/gpu/drm/xe/xe_device.h > +++ b/drivers/gpu/drm/xe/xe_device.h > @@ -174,6 +174,7 @@ static inline bool xe_device_wedged(struct xe_device *xe) > return atomic_read(&xe->wedged.flag); > } > > +void xe_setup_wedge_recovery(struct xe_device *xe); > void xe_device_declare_wedged(struct xe_device *xe); > > struct xe_file *xe_file_get(struct xe_file *xef); > diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c > index a1d08e20cd34..60a8a60f1d9f 100644 > --- a/drivers/gpu/drm/xe/xe_pci.c > +++ b/drivers/gpu/drm/xe/xe_pci.c > @@ -872,6 +872,8 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > if (err) > goto err_driver_cleanup; > > + xe_setup_wedge_recovery(xe); > + > drm_dbg(&xe->drm, "d3cold: capable=%s\n", > str_yes_no(xe->d3cold.capable)); >
On Tue, Sep 17, 2024 at 10:11:05AM +0530, Ghimiray, Himal Prasad wrote: > On 17-09-2024 09:32, 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 supported by DRM core, make use > > of it. 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. > > > As per earlier discussions, the UAPI was also supposed to provide the reason > for wedging( which is supposedly used by L0). IS that requirement nomore in > place ? Wondering how does that contribute to the usecase? Raag
On 17-09-2024 12:08, Raag Jadav wrote: > On Tue, Sep 17, 2024 at 10:11:05AM +0530, Ghimiray, Himal Prasad wrote: >> On 17-09-2024 09:32, 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 supported by DRM core, make use >>> of it. 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. >> >> >> As per earlier discussions, the UAPI was also supposed to provide the reason >> for wedging( which is supposedly used by L0). IS that requirement nomore in >> place ? > > Wondering how does that contribute to the usecase? ZES_EVENT_TYPE_FLAG_DEVICE_RESET_REQUIRED uses zesDeviceGetState "Get information about the state of the device - if a reset is required, reasons for the reset and if the device has been repaired. " https://spec.oneapi.io/level-zero/latest/sysman/api.html#zes__api_8h_1aec73230b938f08ad632d0b7817b66183 > > Raag
On 17/09/24 13:33, Ghimiray, Himal Prasad wrote: > > > On 17-09-2024 12:08, Raag Jadav wrote: >> On Tue, Sep 17, 2024 at 10:11:05AM +0530, Ghimiray, Himal Prasad wrote: >>> On 17-09-2024 09:32, 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 supported by DRM core, make use >>>> of it. 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. >>> >>> >>> As per earlier discussions, the UAPI was also supposed to provide the reason >>> for wedging( which is supposedly used by L0). IS that requirement nomore in >>> place ? >> >> Wondering how does that contribute to the usecase? > > > ZES_EVENT_TYPE_FLAG_DEVICE_RESET_REQUIRED uses zesDeviceGetState > > "Get information about the state of the device - if a reset is required, reasons for the reset and if the device has been repaired. " > > https://spec.oneapi.io/level-zero/latest/sysman/api.html#zes__api_8h_1aec73230b938f08ad632d0b7817b66183 L0 doesn't read this uevent to know the reason, this uevent is for L0 to know that reset is required https://spec.oneapi.io/level-zero/latest/sysman/api.html#_CPPv4N21zes_event_type_flag_t41ZES_EVENT_TYPE_FLAG_DEVICE_RESET_REQUIREDE. The reason is via a different API via https://spec.oneapi.io/level-zero/latest/sysman/api.html#zesdevicegetstate for which they can open any IOCTL which will fail with -ECANCELED when device is wedged and by that they can know the reason. Thanks, Aravind. > >> >> Raag
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c index 4d3c794f134c..1b097643aacb 100644 --- a/drivers/gpu/drm/xe/xe_device.c +++ b/drivers/gpu/drm/xe/xe_device.c @@ -780,6 +780,15 @@ int xe_device_probe(struct xe_device *xe) return err; } +void xe_setup_wedge_recovery(struct xe_device *xe) +{ + struct drm_device *dev = &xe->drm; + + /* Support both driver rebind and bus reset based recovery. */ + set_bit(DRM_WEDGE_RECOVERY_REBIND, &dev->wedge_recovery); + set_bit(DRM_WEDGE_RECOVERY_BUS_RESET, &dev->wedge_recovery); +} + static void xe_device_remove_display(struct xe_device *xe) { xe_display_unregister(xe); @@ -986,11 +995,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 @@ -1020,6 +1030,9 @@ 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_BUS_RESET); } for_each_gt(gt, xe, id) diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h index ca8d8ef6342b..77a2332b4b87 100644 --- a/drivers/gpu/drm/xe/xe_device.h +++ b/drivers/gpu/drm/xe/xe_device.h @@ -174,6 +174,7 @@ static inline bool xe_device_wedged(struct xe_device *xe) return atomic_read(&xe->wedged.flag); } +void xe_setup_wedge_recovery(struct xe_device *xe); void xe_device_declare_wedged(struct xe_device *xe); struct xe_file *xe_file_get(struct xe_file *xef); diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c index a1d08e20cd34..60a8a60f1d9f 100644 --- a/drivers/gpu/drm/xe/xe_pci.c +++ b/drivers/gpu/drm/xe/xe_pci.c @@ -872,6 +872,8 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (err) goto err_driver_cleanup; + xe_setup_wedge_recovery(xe); + drm_dbg(&xe->drm, "d3cold: capable=%s\n", str_yes_no(xe->d3cold.capable));
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 supported by DRM core, make use of it. 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=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 | 17 +++++++++++++++-- drivers/gpu/drm/xe/xe_device.h | 1 + drivers/gpu/drm/xe/xe_pci.c | 2 ++ 3 files changed, 18 insertions(+), 2 deletions(-)