[RFC] drm: Create documentation about device resets

  Create a document that specifies how to deal with DRM device resets for
kernel and userspace drivers.

Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
 Documentation/gpu/drm-reset.rst | 51 +++++++++++++++++++++++++++++++++
 Documentation/gpu/index.rst     |  1 +
 2 files changed, 52 insertions(+)
 create mode 100644 Documentation/gpu/drm-reset.rst
  

Am 23.01.23 um 21:26 schrieb André Almeida:
> Create a document that specifies how to deal with DRM device resets for
> kernel and userspace drivers.
>
> Signed-off-by: André Almeida <andrealmeid@igalia.com>
> ---
>   Documentation/gpu/drm-reset.rst | 51 +++++++++++++++++++++++++++++++++
>   Documentation/gpu/index.rst     |  1 +
>   2 files changed, 52 insertions(+)
>   create mode 100644 Documentation/gpu/drm-reset.rst
>
> diff --git a/Documentation/gpu/drm-reset.rst b/Documentation/gpu/drm-reset.rst
> new file mode 100644
> index 000000000000..0dd11a469cf9
> --- /dev/null
> +++ b/Documentation/gpu/drm-reset.rst
> @@ -0,0 +1,51 @@
> +================
> +DRM Device Reset
> +================
> +
> +The GPU stack is really complex and is prone to errors, from hardware bugs,
> +faulty applications and everything in the many layers in between. To recover
> +from this kind of state, sometimes is needed to reset the GPU. Unproper handling
> +of GPU resets can lead to an unstable userspace. This page describes what's the
> +expected behaviour from DRM drivers to do in those situations, from usermode
> +drivers and compositors as well.
> +
> +Robustness
> +----------
> +
> +First of all, application robust APIs, when available, should be used. This
> +allows the application to correctly recover and continue to run after a reset.
> +Apps that doesn't use this should be promptly killed when the kernel driver
> +detects that it's in broken state. Specifically guidelines for some APIs:
> +

> +- OpenGL: During a reset, KMD kill processes that haven't ARB Robustness
> +  enabled, assuming they can't recover.

This is a pretty clear NAK from my side to this approach. The KMD should 
never mess with an userspace process directly in such a way.

Instead use something like this "OpenGL: KMD signals the abortion of 
submitted commands and the UMD should then react accordingly and abort 
the application.".

> +- Vulkan: Assumes that every app is able to deal with ``VK_ERROR_DEVICE_LOST``,
> +  so KMD doesn't kill any. If it doesn't do it right, it's considered a broken
> +  application and UMD will deal with it.

Again, pleas remove the "KMD kill" reference.

> +
> +Kernel mode driver
> +------------------
> +
> +The KMD should be able to detect that something is wrong with the application

Please replace *should* with *must* here, this is mandatory or otherwise 
core memory management can run into deadlocks during reclaim.

Regards,
Christian.

> +and that a reset is needed to take place to recover the device (e.g. an endless
> +wait). It needs to properly track the context that is broken and mark it as
> +dead, so any other syscalls to that context should be further rejected. The
> +other contexts should be preserved when possible, avoid crashing the rest of
> +userspace. KMD can ban a file descriptor that keeps causing resets, as it's
> +likely in a broken loop.
> +
> +User mode driver
> +----------------
> +
> +During a reset, UMD should be aware that rejected syscalls indicates that the
> +context is broken and for robust apps the recovery should happen for the
> +context. Non-robust apps would be already terminated by KMD. If no new context
> +is created for some time, it is assumed that the recovery didn't work, so UMD
> +should terminate it.
> +
> +Compositors
> +-----------
> +
> +(In the long term) compositors should be robust as well to properly deal with it
> +errors. Init systems should be aware of the compositor status and reset it if is
> +broken.
> diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst
> index b99dede9a5b1..300b2529bd39 100644
> --- a/Documentation/gpu/index.rst
> +++ b/Documentation/gpu/index.rst
> @@ -9,6 +9,7 @@ Linux GPU Driver Developer's Guide
>      drm-mm
>      drm-kms
>      drm-kms-helpers
> +   drm-reset
>      drm-uapi
>      drm-usage-stats
>      driver-uapi
  

On Mon, 23 Jan 2023 21:38:11 +0100
Christian König <christian.koenig@amd.com> wrote:

> Am 23.01.23 um 21:26 schrieb André Almeida:
> > Create a document that specifies how to deal with DRM device resets for
> > kernel and userspace drivers.
> >
> > Signed-off-by: André Almeida <andrealmeid@igalia.com>
> > ---
> >   Documentation/gpu/drm-reset.rst | 51 +++++++++++++++++++++++++++++++++
> >   Documentation/gpu/index.rst     |  1 +
> >   2 files changed, 52 insertions(+)
> >   create mode 100644 Documentation/gpu/drm-reset.rst
> >
> > diff --git a/Documentation/gpu/drm-reset.rst b/Documentation/gpu/drm-reset.rst
> > new file mode 100644
> > index 000000000000..0dd11a469cf9
> > --- /dev/null
> > +++ b/Documentation/gpu/drm-reset.rst
> > @@ -0,0 +1,51 @@
> > +================
> > +DRM Device Reset
> > +================
> > +
> > +The GPU stack is really complex and is prone to errors, from hardware bugs,
> > +faulty applications and everything in the many layers in between. To recover
> > +from this kind of state, sometimes is needed to reset the GPU. Unproper handling
> > +of GPU resets can lead to an unstable userspace. This page describes what's the
> > +expected behaviour from DRM drivers to do in those situations, from usermode
> > +drivers and compositors as well.
> > +
> > +Robustness
> > +----------
> > +
> > +First of all, application robust APIs, when available, should be used. This
> > +allows the application to correctly recover and continue to run after a reset.
> > +Apps that doesn't use this should be promptly killed when the kernel driver
> > +detects that it's in broken state. Specifically guidelines for some APIs:
> > +  
> 
> > +- OpenGL: During a reset, KMD kill processes that haven't ARB Robustness
> > +  enabled, assuming they can't recover.  
> 
> This is a pretty clear NAK from my side to this approach. The KMD should 
> never mess with an userspace process directly in such a way.
> 
> Instead use something like this "OpenGL: KMD signals the abortion of 
> submitted commands and the UMD should then react accordingly and abort 
> the application.".
> 
> > +- Vulkan: Assumes that every app is able to deal with ``VK_ERROR_DEVICE_LOST``,
> > +  so KMD doesn't kill any. If it doesn't do it right, it's considered a broken
> > +  application and UMD will deal with it.  
> 
> Again, pleas remove the "KMD kill" reference.
> 
> > +
> > +Kernel mode driver
> > +------------------
> > +
> > +The KMD should be able to detect that something is wrong with the application  
> 
> Please replace *should* with *must* here, this is mandatory or otherwise 
> core memory management can run into deadlocks during reclaim.
> 
> Regards,
> Christian.
> 
> > +and that a reset is needed to take place to recover the device (e.g. an endless
> > +wait). It needs to properly track the context that is broken and mark it as
> > +dead, so any other syscalls to that context should be further rejected. The
> > +other contexts should be preserved when possible, avoid crashing the rest of
> > +userspace. KMD can ban a file descriptor that keeps causing resets, as it's
> > +likely in a broken loop.
> > +
> > +User mode driver
> > +----------------
> > +
> > +During a reset, UMD should be aware that rejected syscalls indicates that the
> > +context is broken and for robust apps the recovery should happen for the
> > +context. Non-robust apps would be already terminated by KMD. If no new context
> > +is created for some time, it is assumed that the recovery didn't work, so UMD
> > +should terminate it.

Hi,

what Christian said, plus I would not assume that robust programs will
always respond by creating a new context. They could also switch
to a software renderer, or simply not do graphics again until something
else happens.

> > +
> > +Compositors
> > +-----------
> > +
> > +(In the long term) compositors should be robust as well to properly deal with it
> > +errors. Init systems should be aware of the compositor status and reset it if is
> > +broken.

I don't know how init systems could do that, or what difference does it
make to an init system whether the display server is robust or not.
Display servers can get stuck for other reasons as well. They may also
be live-stuck, where they respond to keepalive, serve clients, and
deliver input events, but still do not update the screen. You can't
tell if that's a malfunction or expected.



Have you checked
https://www.kernel.org/doc/html/latest/gpu/drm-uapi.html#device-hot-unplug
that you are consistent with hot-unplug plans?


Thanks,
pq

> > diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst
> > index b99dede9a5b1..300b2529bd39 100644
> > --- a/Documentation/gpu/index.rst
> > +++ b/Documentation/gpu/index.rst
> > @@ -9,6 +9,7 @@ Linux GPU Driver Developer's Guide
> >      drm-mm
> >      drm-kms
> >      drm-kms-helpers
> > +   drm-reset
> >      drm-uapi
> >      drm-usage-stats
> >      driver-uapi  
>
  

On 2/7/23 14:30, Pekka Paalanen wrote:
> On Mon, 23 Jan 2023 21:38:11 +0100
> Christian König <christian.koenig@amd.com> wrote:
>> Am 23.01.23 um 21:26 schrieb André Almeida:
>>>
>>> diff --git a/Documentation/gpu/drm-reset.rst b/Documentation/gpu/drm-reset.rst
>>> new file mode 100644
>>> index 000000000000..0dd11a469cf9
>>> --- /dev/null
>>> +++ b/Documentation/gpu/drm-reset.rst
>>> @@ -0,0 +1,51 @@
>>> +================
>>> +DRM Device Reset
>>> +================
>>> +
>>> +The GPU stack is really complex and is prone to errors, from hardware bugs,
>>> +faulty applications and everything in the many layers in between. To recover
>>> +from this kind of state, sometimes is needed to reset the GPU. Unproper handling
>>> +of GPU resets can lead to an unstable userspace. This page describes what's the
>>> +expected behaviour from DRM drivers to do in those situations, from usermode
>>> +drivers and compositors as well.
>>> +
>>> +Robustness
>>> +----------
>>> +
>>> +First of all, application robust APIs, when available, should be used. This
>>> +allows the application to correctly recover and continue to run after a reset.
>>> +Apps that doesn't use this should be promptly killed when the kernel driver
>>> +detects that it's in broken state. Specifically guidelines for some APIs:
>>> +  
>>
>>> +- OpenGL: During a reset, KMD kill processes that haven't ARB Robustness
>>> +  enabled, assuming they can't recover.  
>>
>> This is a pretty clear NAK from my side to this approach. The KMD should 
>> never mess with an userspace process directly in such a way.
>>
>> Instead use something like this "OpenGL: KMD signals the abortion of 
>> submitted commands and the UMD should then react accordingly and abort 
>> the application.".
> 
> what Christian said, plus I would not assume that robust programs will
> always respond by creating a new context. They could also switch
> to a software renderer, [...]

That is indeed what Firefox does.