Message-ID: <20230207153048.78697e7b@eldfell>
Date:   Tue, 7 Feb 2023 15:30:48 +0200
From:   Pekka Paalanen <ppaalanen@...il.com>
To:     André Almeida <andrealmeid@...lia.com>
Cc:     Christian König <christian.koenig@....com>,
        dri-devel@...ts.freedesktop.org, amd-gfx@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, kernel-dev@...lia.com,
        alexander.deucher@....com, contactshashanksharma@...il.com,
        amaranath.somalapuram@....com, pierre-eric.pelloux-prayer@....com,
        Simon Ser <contact@...rsion.fr>,
        Rob Clark <robdclark@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Daniel Stone <daniel@...ishbar.org>,
        'Marek Olšák' <maraeo@...il.com>,
        Dave Airlie <airlied@...il.com>,
        "Pierre-Loup A . Griffais" <pgriffais@...vesoftware.com>
Subject: Re: [RFC PATCH] drm: Create documentation about device resets

On Mon, 23 Jan 2023 21:38:11 +0100
Christian König <christian.koenig@....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@...lia.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  
> 


Content of type "application/pgp-signature" skipped

Powered by blists - more mailing lists