Date:   Tue, 7 Feb 2023 15:30:48 +0200
From:   Pekka Paalanen <ppaalanen@gmail.com>
To:     =?UTF-8?B?QW5kcsOp?= Almeida <andrealmeid@igalia.com>
Cc:     Christian =?UTF-8?B?S8O2bmln?= <christian.koenig@amd.com>,
        dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, kernel-dev@igalia.com,
        alexander.deucher@amd.com, contactshashanksharma@gmail.com,
        amaranath.somalapuram@amd.com, pierre-eric.pelloux-prayer@amd.com,
        Simon Ser <contact@emersion.fr>,
        Rob Clark <robdclark@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        Daniel Stone <daniel@fooishbar.org>,
        'Marek =?UTF-8?B?T2zFocOhayc=?= <maraeo@gmail.com>,
        Dave Airlie <airlied@gmail.com>,
        "Pierre-Loup A . Griffais" <pgriffais@valvesoftware.com>
Subject: Re: [RFC PATCH] drm: Create documentation about device resets
Message-ID: <20230207153048.78697e7b@eldfell>
In-Reply-To: <37e78013-5ed6-e80f-4a1c-1e61bed59735@amd.com>
References: <20230123202646.356592-1-andrealmeid@igalia.com>
        <20230123202646.356592-2-andrealmeid@igalia.com>
        <37e78013-5ed6-e80f-4a1c-1e61bed59735@amd.com>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="Sig_/9db4WWm0I_X2MQvPfj4lskX";
 protocol="application/pgp-signature"; micalg=pgp-sha256
Precedence: bulk

--Sig_/9db4WWm0I_X2MQvPfj4lskX
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

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

> Am 23.01.23 um 21:26 schrieb Andr=C3=A9 Almeida:
> > Create a document that specifies how to deal with DRM device resets for
> > kernel and userspace drivers.
> >
> > Signed-off-by: Andr=C3=A9 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-re=
set.rst
> > new file mode 100644
> > index 000000000000..0dd11a469cf9
> > --- /dev/null
> > +++ b/Documentation/gpu/drm-reset.rst
> > @@ -0,0 +1,51 @@
> > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > +DRM Device Reset
> > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > +
> > +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 r=
ecover
> > +from this kind of state, sometimes is needed to reset the GPU. Unprope=
r handling
> > +of GPU resets can lead to an unstable userspace. This page describes w=
hat's the
> > +expected behaviour from DRM drivers to do in those situations, from us=
ermode
> > +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 d=
river
> > +detects that it's in broken state. Specifically guidelines for some AP=
Is:
> > + =20
>=20
> > +- OpenGL: During a reset, KMD kill processes that haven't ARB Robustne=
ss
> > +  enabled, assuming they can't recover. =20
>=20
> This is a pretty clear NAK from my side to this approach. The KMD should=
=20
> never mess with an userspace process directly in such a way.
>=20
> Instead use something like this "OpenGL: KMD signals the abortion of=20
> submitted commands and the UMD should then react accordingly and abort=20
> the application.".
>=20
> > +- Vulkan: Assumes that every app is able to deal with ``VK_ERROR_DEVIC=
E_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. =20
>=20
> Again, pleas remove the "KMD kill" reference.
>=20
> > +
> > +Kernel mode driver
> > +------------------
> > +
> > +The KMD should be able to detect that something is wrong with the appl=
ication =20
>=20
> Please replace *should* with *must* here, this is mandatory or otherwise=
=20
> core memory management can run into deadlocks during reclaim.
>=20
> Regards,
> Christian.
>=20
> > +and that a reset is needed to take place to recover the device (e.g. a=
n 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 r=
est 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 t=
hat the
> > +context is broken and for robust apps the recovery should happen for t=
he
> > +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 de=
al with it
> > +errors. Init systems should be aware of the compositor status and rese=
t 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 =20
>=20


--Sig_/9db4WWm0I_X2MQvPfj4lskX
Content-Type: application/pgp-signature
Content-Description: OpenPGP digital signature

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEJQjwWQChkWOYOIONI1/ltBGqqqcFAmPiUogACgkQI1/ltBGq
qqelTQ//fJlGZa8DbtsnhMjlSHNEkUscr33DsHa67lB+rVgH5IlOLmaTRryrc/Cm
FKNPaSxM8ejQWj5ROl4t1h5uNfK3u5nt15nzg44yOOs2FcCHKHeDsEWqKpznt//q
QH198JFUnEHv0+HJCLEwo7FPnooQlXKZlx77s6NgOCy3BLolv9I2+hOSw5Mvl7Ld
rrwXkfpWeRfqz0ILViEa4t37om4qNZxbcCikmOeckqQcPagWjiLNCRG3UJY2oIGU
eCPrw/eOt1pAPFiYWeKJ3Y3HMIFvLo+/yCQnPYjq69Oufy5UtyXPZMI5JFSUZN2/
9fi6MBDFh7WUbMp5iAcvFtOm4msQq6629RrGgYAuV2xkaTb5ifx+f69wHuuq9TZh
6vyLiBTZhT80U+hd9r5atf0Gm0lACaufEl57OuCPFE6UlQ1WFD0Rg/AurWoPI5Or
LIBfoxU6ZPSJOnhDfh0oDZmyCrJjlvY9ty5geFrxjOzvqD450ckzX/TtHzMfQokg
ELGdSvOLhlsMJBV4I+HdvXqVZm5GxS0BPnhnrHXN684y6SWn/P3MZLEU2buCtA3M
LefRCdXUPB1OR9b/cWSdw8XSKggmuGTqKkbYora4pW2L2t15kkTSdPoewavQJGPI
qOzgQ0GNyJ+wNKW2LLRMdF+N5kQs+uO6h2SgCQaOwdINKg3hZZo=
=7m4W
-----END PGP SIGNATURE-----

--Sig_/9db4WWm0I_X2MQvPfj4lskX--