| Message ID | 20241115050733.806934-2-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: > Introduce device wedged event, which notifies userspace of 'wedged' > (hanged/unusable) state of the DRM device through a uevent. This is > useful especially in cases where the device is no longer operating as > expected and has become unrecoverable from driver context. Purpose of > this implementation is to provide drivers a generic way to recover with > the help of userspace intervention without taking any drastic measures > in the driver. > > A 'wedged' device is basically a dead device that needs attention. The > uevent is the notification that is sent to userspace along with a hint > about what could possibly be attempted to recover the device and bring > it back to usable state. Different drivers may have different ideas of > a 'wedged' device depending on their hardware implementation, and hence > the vendor agnostic nature of the event. It is up to the drivers to > decide when they see the need for recovery and how they want to recover > from the available methods. > > Prerequisites > ------------- > > The driver, before opting for recovery, needs to make sure that the > 'wedged' device doesn't harm the system as a whole by taking care of the > prerequisites. Necessary actions must include disabling DMA to system > memory as well as any communication channels with other devices. Further, > the driver must ensure that all dma_fences are signalled and any device > state that the core kernel might depend on are cleaned up. Once the event > is sent, the device must be kept in 'wedged' state until the recovery is > performed. New accesses to the device (IOCTLs) should be blocked, > preferably with an error code that resembles the type of failure the > device has encountered. This will signify the reason for wegeding which > can be reported to the application if needed. should we even drop the mmaps we created? Thanks, Aravind. > > Recovery > -------- > > Current implementation defines three recovery methods, out of which, > drivers can use any one, multiple or none. Method(s) of choice will be > sent in the uevent environment as ``WEDGED=<method1>[,<method2>]`` in > order of less to more side-effects. If driver is unsure about recovery > or method is unknown (like soft/hard reboot, firmware flashing, hardware > replacement or any other procedure which can't be attempted on the fly), > ``WEDGED=unknown`` will be sent instead. > > Userspace consumers can parse this event and attempt recovery as per the > following expectations. > > =============== ================================ > Recovery method Consumer expectations > =============== ================================ > none optional telemetry collection > rebind unbind + bind driver > bus-reset unbind + reset bus device + bind > unknown admin/user policy > =============== ================================ > > The only exception to this is ``WEDGED=none``, which signifies that the > device was temporarily 'wedged' at some point but was able to recover > using device specific methods like reset. No explicit action is expected > from userspace consumers in this case, but they can still take additional > steps like gathering telemetry information (devcoredump, syslog). This is > useful because the first hang is usually the most critical one which can > result in consequential hangs or complete wedging. > > Example > ------- > > Udev rule:: > > SUBSYSTEM=="drm", ENV{WEDGED}=="rebind", DEVPATH=="*/drm/card[0-9]", > RUN+="/path/to/rebind.sh $env{DEVPATH}" > > Recovery script:: > > #!/bin/sh > > DEVPATH=$(readlink -f /sys/$1/device) > DEVICE=$(basename $DEVPATH) > DRIVER=$(readlink -f $DEVPATH/driver) > > echo -n $DEVICE > $DRIVER/unbind > sleep 1 > echo -n $DEVICE > $DRIVER/bind > > Customization > ------------- > > Although basic recovery is possible with a simple script, admin/users can > define custom policies around recovery action. For example, if the driver > supports multiple recovery methods, consumers can opt for the suitable one > based on policy definition. Consumers can also choose to have the device > available for debugging or additional data collection before performing > the recovery. This is useful especially when the driver is unsure about > recovery or method is unknown. > > v4: s/drm_dev_wedged/drm_dev_wedged_event > Use drm_info() (Jani) > Kernel doc adjustment (Aravind) > v5: Send recovery method with uevent (Lina) > v6: Access wedge_recovery_opts[] using helper function (Jani) > Use snprintf() (Jani) > v7: Convert recovery helpers into regular functions (Andy, Jani) > Aesthetic adjustments (Andy) > Handle invalid method cases > v8: Allow sending multiple methods with uevent (Lucas, Michal) > static_assert() globally (Andy) > v9: Provide 'none' method for reset cases (Christian) > Provide recovery opts using switch cases > > Signed-off-by: Raag Jadav <raag.jadav@intel.com> > --- > drivers/gpu/drm/drm_drv.c | 63 +++++++++++++++++++++++++++++++++++++++ > include/drm/drm_device.h | 8 +++++ > include/drm/drm_drv.h | 1 + > 3 files changed, 72 insertions(+) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index c2c172eb25df..115e1d1c80ea 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -26,6 +26,7 @@ > * DEALINGS IN THE SOFTWARE. > */ > > +#include <linux/bitops.h> > #include <linux/debugfs.h> > #include <linux/fs.h> > #include <linux/module.h> > @@ -33,6 +34,7 @@ > #include <linux/mount.h> > #include <linux/pseudo_fs.h> > #include <linux/slab.h> > +#include <linux/sprintf.h> > #include <linux/srcu.h> > #include <linux/xarray.h> > > @@ -497,6 +499,67 @@ void drm_dev_unplug(struct drm_device *dev) > } > EXPORT_SYMBOL(drm_dev_unplug); > > +/* > + * Available recovery methods for wedged device. To be sent along with device > + * wedged uevent. > + */ > +static const char *drm_get_wedge_recovery(unsigned int opt) > +{ > + switch (BIT(opt)) { > + case DRM_WEDGE_RECOVERY_NONE: > + return "none"; > + case DRM_WEDGE_RECOVERY_REBIND: > + return "rebind"; > + case DRM_WEDGE_RECOVERY_BUS_RESET: > + return "bus-reset"; > + default: > + return NULL; > + } > +} > + > +/** > + * drm_dev_wedged_event - generate a device wedged uevent > + * @dev: DRM device > + * @method: method(s) to be used for recovery > + * > + * This generates a device wedged uevent for the DRM device specified by @dev. > + * Recovery @method\(s) of choice will be sent in the uevent environment as > + * ``WEDGED=<method1>[,<method2>]`` in order of less to more side-effects. > + * If caller is unsure about recovery or @method is unknown (0), > + * ``WEDGED=unknown`` will be sent instead. > + * > + * Returns: 0 on success, negative error code otherwise. > + */ > +int drm_dev_wedged_event(struct drm_device *dev, unsigned long method) > +{ > + const char *recovery = NULL; > + unsigned int len, opt; > + /* Event string length up to 28+ characters with available methods */ > + char event_string[32]; > + char *envp[] = { event_string, NULL }; > + > + len = scnprintf(event_string, sizeof(event_string), "%s", "WEDGED="); > + > + for_each_set_bit(opt, &method, BITS_PER_TYPE(method)) { > + recovery = drm_get_wedge_recovery(opt); > + if (drm_WARN(dev, !recovery, "device wedged, invalid recovery method %u\n", opt)) > + break; > + > + len += scnprintf(event_string + len, sizeof(event_string), "%s,", recovery); > + } > + > + if (recovery) > + /* Get rid of trailing comma */ > + event_string[len - 1] = '\0'; > + else > + /* Caller is unsure about recovery, do the best we can at this point. */ > + snprintf(event_string, sizeof(event_string), "%s", "WEDGED=unknown"); > + > + drm_info(dev, "device wedged, needs recovery\n"); > + return kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp); > +} > +EXPORT_SYMBOL(drm_dev_wedged_event); > + > /* > * DRM internal mount > * We want to be able to allocate our own "struct address_space" to control > diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h > index c91f87b5242d..6ea54a578cda 100644 > --- a/include/drm/drm_device.h > +++ b/include/drm/drm_device.h > @@ -21,6 +21,14 @@ struct inode; > struct pci_dev; > struct pci_controller; > > +/* > + * Recovery methods for wedged device in order of less to more side-effects. > + * To be used with drm_dev_wedged_event() as recovery @method. Callers can > + * use any one, multiple (or'd) or none depending on their needs. > + */ > +#define DRM_WEDGE_RECOVERY_NONE BIT(0) /* optional telemetry collection */ > +#define DRM_WEDGE_RECOVERY_REBIND BIT(1) /* unbind + bind driver */ > +#define DRM_WEDGE_RECOVERY_BUS_RESET BIT(2) /* unbind + reset bus device + bind */ > > /** > * enum switch_power_state - power state of drm device > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 1bbbcb8e2d23..f41a82839e28 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -479,6 +479,7 @@ void drm_put_dev(struct drm_device *dev); > bool drm_dev_enter(struct drm_device *dev, int *idx); > void drm_dev_exit(int idx); > void drm_dev_unplug(struct drm_device *dev); > +int drm_dev_wedged_event(struct drm_device *dev, unsigned long method); > > /** > * drm_dev_is_unplugged - is a DRM device unplugged
On Mon, Nov 18, 2024 at 08:26:37PM +0530, Aravind Iddamsetty wrote: > On 15/11/24 10:37, Raag Jadav wrote: > > Introduce device wedged event, which notifies userspace of 'wedged' > > (hanged/unusable) state of the DRM device through a uevent. This is > > useful especially in cases where the device is no longer operating as > > expected and has become unrecoverable from driver context. Purpose of > > this implementation is to provide drivers a generic way to recover with > > the help of userspace intervention without taking any drastic measures > > in the driver. > > > > A 'wedged' device is basically a dead device that needs attention. The > > uevent is the notification that is sent to userspace along with a hint > > about what could possibly be attempted to recover the device and bring > > it back to usable state. Different drivers may have different ideas of > > a 'wedged' device depending on their hardware implementation, and hence > > the vendor agnostic nature of the event. It is up to the drivers to > > decide when they see the need for recovery and how they want to recover > > from the available methods. > > > > Prerequisites > > ------------- > > > > The driver, before opting for recovery, needs to make sure that the > > 'wedged' device doesn't harm the system as a whole by taking care of the > > prerequisites. Necessary actions must include disabling DMA to system > > memory as well as any communication channels with other devices. Further, > > the driver must ensure that all dma_fences are signalled and any device > > state that the core kernel might depend on are cleaned up. Once the event > > is sent, the device must be kept in 'wedged' state until the recovery is > > performed. New accesses to the device (IOCTLs) should be blocked, > > preferably with an error code that resembles the type of failure the > > device has encountered. This will signify the reason for wegeding which > > can be reported to the application if needed. > > should we even drop the mmaps we created? Whatever is required for a clean recovery, yes. Although how would this play out? Do we risk loosing display? Or any other possible side-effects? Raag
Am 22.11.24 um 08:07 schrieb Raag Jadav: > On Mon, Nov 18, 2024 at 08:26:37PM +0530, Aravind Iddamsetty wrote: >> On 15/11/24 10:37, Raag Jadav wrote: >>> Introduce device wedged event, which notifies userspace of 'wedged' >>> (hanged/unusable) state of the DRM device through a uevent. This is >>> useful especially in cases where the device is no longer operating as >>> expected and has become unrecoverable from driver context. Purpose of >>> this implementation is to provide drivers a generic way to recover with >>> the help of userspace intervention without taking any drastic measures >>> in the driver. >>> >>> A 'wedged' device is basically a dead device that needs attention. The >>> uevent is the notification that is sent to userspace along with a hint >>> about what could possibly be attempted to recover the device and bring >>> it back to usable state. Different drivers may have different ideas of >>> a 'wedged' device depending on their hardware implementation, and hence >>> the vendor agnostic nature of the event. It is up to the drivers to >>> decide when they see the need for recovery and how they want to recover >>> from the available methods. >>> >>> Prerequisites >>> ------------- >>> >>> The driver, before opting for recovery, needs to make sure that the >>> 'wedged' device doesn't harm the system as a whole by taking care of the >>> prerequisites. Necessary actions must include disabling DMA to system >>> memory as well as any communication channels with other devices. Further, >>> the driver must ensure that all dma_fences are signalled and any device >>> state that the core kernel might depend on are cleaned up. Once the event >>> is sent, the device must be kept in 'wedged' state until the recovery is >>> performed. New accesses to the device (IOCTLs) should be blocked, >>> preferably with an error code that resembles the type of failure the >>> device has encountered. This will signify the reason for wegeding which >>> can be reported to the application if needed. >> should we even drop the mmaps we created? > Whatever is required for a clean recovery, yes. > > Although how would this play out? Do we risk loosing display? > Or any other possible side-effects? Before sending a wedge event all DMA transfers of the device have to be blocked. So yes, all display, mmap() and file descriptor connections you had with the device would need to be re-created. Regards, Christian. > > Raag
On Fri, Nov 22, 2024 at 11:09:32AM +0100, Christian König wrote: > Am 22.11.24 um 08:07 schrieb Raag Jadav: > > On Mon, Nov 18, 2024 at 08:26:37PM +0530, Aravind Iddamsetty wrote: > > > On 15/11/24 10:37, Raag Jadav wrote: > > > > Introduce device wedged event, which notifies userspace of 'wedged' > > > > (hanged/unusable) state of the DRM device through a uevent. This is > > > > useful especially in cases where the device is no longer operating as > > > > expected and has become unrecoverable from driver context. Purpose of > > > > this implementation is to provide drivers a generic way to recover with > > > > the help of userspace intervention without taking any drastic measures > > > > in the driver. > > > > > > > > A 'wedged' device is basically a dead device that needs attention. The > > > > uevent is the notification that is sent to userspace along with a hint > > > > about what could possibly be attempted to recover the device and bring > > > > it back to usable state. Different drivers may have different ideas of > > > > a 'wedged' device depending on their hardware implementation, and hence > > > > the vendor agnostic nature of the event. It is up to the drivers to > > > > decide when they see the need for recovery and how they want to recover > > > > from the available methods. > > > > > > > > Prerequisites > > > > ------------- > > > > > > > > The driver, before opting for recovery, needs to make sure that the > > > > 'wedged' device doesn't harm the system as a whole by taking care of the > > > > prerequisites. Necessary actions must include disabling DMA to system > > > > memory as well as any communication channels with other devices. Further, > > > > the driver must ensure that all dma_fences are signalled and any device > > > > state that the core kernel might depend on are cleaned up. Once the event > > > > is sent, the device must be kept in 'wedged' state until the recovery is > > > > performed. New accesses to the device (IOCTLs) should be blocked, > > > > preferably with an error code that resembles the type of failure the > > > > device has encountered. This will signify the reason for wegeding which > > > > can be reported to the application if needed. > > > should we even drop the mmaps we created? > > Whatever is required for a clean recovery, yes. > > > > Although how would this play out? Do we risk loosing display? > > Or any other possible side-effects? > > Before sending a wedge event all DMA transfers of the device have to be > blocked. > > So yes, all display, mmap() and file descriptor connections you had with the > device would need to be re-created. Does it mean we'd have to rely on userspace to unmap()? Raag
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index c2c172eb25df..115e1d1c80ea 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -26,6 +26,7 @@ * DEALINGS IN THE SOFTWARE. */ +#include <linux/bitops.h> #include <linux/debugfs.h> #include <linux/fs.h> #include <linux/module.h> @@ -33,6 +34,7 @@ #include <linux/mount.h> #include <linux/pseudo_fs.h> #include <linux/slab.h> +#include <linux/sprintf.h> #include <linux/srcu.h> #include <linux/xarray.h> @@ -497,6 +499,67 @@ void drm_dev_unplug(struct drm_device *dev) } EXPORT_SYMBOL(drm_dev_unplug); +/* + * Available recovery methods for wedged device. To be sent along with device + * wedged uevent. + */ +static const char *drm_get_wedge_recovery(unsigned int opt) +{ + switch (BIT(opt)) { + case DRM_WEDGE_RECOVERY_NONE: + return "none"; + case DRM_WEDGE_RECOVERY_REBIND: + return "rebind"; + case DRM_WEDGE_RECOVERY_BUS_RESET: + return "bus-reset"; + default: + return NULL; + } +} + +/** + * drm_dev_wedged_event - generate a device wedged uevent + * @dev: DRM device + * @method: method(s) to be used for recovery + * + * This generates a device wedged uevent for the DRM device specified by @dev. + * Recovery @method\(s) of choice will be sent in the uevent environment as + * ``WEDGED=<method1>[,<method2>]`` in order of less to more side-effects. + * If caller is unsure about recovery or @method is unknown (0), + * ``WEDGED=unknown`` will be sent instead. + * + * Returns: 0 on success, negative error code otherwise. + */ +int drm_dev_wedged_event(struct drm_device *dev, unsigned long method) +{ + const char *recovery = NULL; + unsigned int len, opt; + /* Event string length up to 28+ characters with available methods */ + char event_string[32]; + char *envp[] = { event_string, NULL }; + + len = scnprintf(event_string, sizeof(event_string), "%s", "WEDGED="); + + for_each_set_bit(opt, &method, BITS_PER_TYPE(method)) { + recovery = drm_get_wedge_recovery(opt); + if (drm_WARN(dev, !recovery, "device wedged, invalid recovery method %u\n", opt)) + break; + + len += scnprintf(event_string + len, sizeof(event_string), "%s,", recovery); + } + + if (recovery) + /* Get rid of trailing comma */ + event_string[len - 1] = '\0'; + else + /* Caller is unsure about recovery, do the best we can at this point. */ + snprintf(event_string, sizeof(event_string), "%s", "WEDGED=unknown"); + + drm_info(dev, "device wedged, needs recovery\n"); + return kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp); +} +EXPORT_SYMBOL(drm_dev_wedged_event); + /* * DRM internal mount * We want to be able to allocate our own "struct address_space" to control diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index c91f87b5242d..6ea54a578cda 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -21,6 +21,14 @@ struct inode; struct pci_dev; struct pci_controller; +/* + * Recovery methods for wedged device in order of less to more side-effects. + * To be used with drm_dev_wedged_event() as recovery @method. Callers can + * use any one, multiple (or'd) or none depending on their needs. + */ +#define DRM_WEDGE_RECOVERY_NONE BIT(0) /* optional telemetry collection */ +#define DRM_WEDGE_RECOVERY_REBIND BIT(1) /* unbind + bind driver */ +#define DRM_WEDGE_RECOVERY_BUS_RESET BIT(2) /* unbind + reset bus device + bind */ /** * enum switch_power_state - power state of drm device diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 1bbbcb8e2d23..f41a82839e28 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -479,6 +479,7 @@ void drm_put_dev(struct drm_device *dev); bool drm_dev_enter(struct drm_device *dev, int *idx); void drm_dev_exit(int idx); void drm_dev_unplug(struct drm_device *dev); +int drm_dev_wedged_event(struct drm_device *dev, unsigned long method); /** * drm_dev_is_unplugged - is a DRM device unplugged
Introduce device wedged event, which notifies userspace of 'wedged' (hanged/unusable) state of the DRM device through a uevent. This is useful especially in cases where the device is no longer operating as expected and has become unrecoverable from driver context. Purpose of this implementation is to provide drivers a generic way to recover with the help of userspace intervention without taking any drastic measures in the driver. A 'wedged' device is basically a dead device that needs attention. The uevent is the notification that is sent to userspace along with a hint about what could possibly be attempted to recover the device and bring it back to usable state. Different drivers may have different ideas of a 'wedged' device depending on their hardware implementation, and hence the vendor agnostic nature of the event. It is up to the drivers to decide when they see the need for recovery and how they want to recover from the available methods. Prerequisites ------------- The driver, before opting for recovery, needs to make sure that the 'wedged' device doesn't harm the system as a whole by taking care of the prerequisites. Necessary actions must include disabling DMA to system memory as well as any communication channels with other devices. Further, the driver must ensure that all dma_fences are signalled and any device state that the core kernel might depend on are cleaned up. Once the event is sent, the device must be kept in 'wedged' state until the recovery is performed. New accesses to the device (IOCTLs) should be blocked, preferably with an error code that resembles the type of failure the device has encountered. This will signify the reason for wegeding which can be reported to the application if needed. Recovery -------- Current implementation defines three recovery methods, out of which, drivers can use any one, multiple or none. Method(s) of choice will be sent in the uevent environment as ``WEDGED=<method1>[,<method2>]`` in order of less to more side-effects. If driver is unsure about recovery or method is unknown (like soft/hard reboot, firmware flashing, hardware replacement or any other procedure which can't be attempted on the fly), ``WEDGED=unknown`` will be sent instead. Userspace consumers can parse this event and attempt recovery as per the following expectations. =============== ================================ Recovery method Consumer expectations =============== ================================ none optional telemetry collection rebind unbind + bind driver bus-reset unbind + reset bus device + bind unknown admin/user policy =============== ================================ The only exception to this is ``WEDGED=none``, which signifies that the device was temporarily 'wedged' at some point but was able to recover using device specific methods like reset. No explicit action is expected from userspace consumers in this case, but they can still take additional steps like gathering telemetry information (devcoredump, syslog). This is useful because the first hang is usually the most critical one which can result in consequential hangs or complete wedging. Example ------- Udev rule:: SUBSYSTEM=="drm", ENV{WEDGED}=="rebind", DEVPATH=="*/drm/card[0-9]", RUN+="/path/to/rebind.sh $env{DEVPATH}" Recovery script:: #!/bin/sh DEVPATH=$(readlink -f /sys/$1/device) DEVICE=$(basename $DEVPATH) DRIVER=$(readlink -f $DEVPATH/driver) echo -n $DEVICE > $DRIVER/unbind sleep 1 echo -n $DEVICE > $DRIVER/bind Customization ------------- Although basic recovery is possible with a simple script, admin/users can define custom policies around recovery action. For example, if the driver supports multiple recovery methods, consumers can opt for the suitable one based on policy definition. Consumers can also choose to have the device available for debugging or additional data collection before performing the recovery. This is useful especially when the driver is unsure about recovery or method is unknown. v4: s/drm_dev_wedged/drm_dev_wedged_event Use drm_info() (Jani) Kernel doc adjustment (Aravind) v5: Send recovery method with uevent (Lina) v6: Access wedge_recovery_opts[] using helper function (Jani) Use snprintf() (Jani) v7: Convert recovery helpers into regular functions (Andy, Jani) Aesthetic adjustments (Andy) Handle invalid method cases v8: Allow sending multiple methods with uevent (Lucas, Michal) static_assert() globally (Andy) v9: Provide 'none' method for reset cases (Christian) Provide recovery opts using switch cases Signed-off-by: Raag Jadav <raag.jadav@intel.com> --- drivers/gpu/drm/drm_drv.c | 63 +++++++++++++++++++++++++++++++++++++++ include/drm/drm_device.h | 8 +++++ include/drm/drm_drv.h | 1 + 3 files changed, 72 insertions(+)