Re: [PATCH] drm/doc: device hot-unplug for userspace

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 10240 bytes --]

On Tue, 19 May 2020 10:37:12 -0400
Andrey Grodzovsky <Andrey.Grodzovsky@amd.com> wrote:

> Thanks for the summary, does put things in order and makes it easier to 
> comprehend all the TODOs, some questions bellow
> 
> On 5/19/20 6:06 AM, Pekka Paalanen wrote:
> > From: Pekka Paalanen <pekka.paalanen@collabora.com>
> >
> > Set up the expectations on how hot-unplugging a DRM device should look like to
> > userspace.
> >
> > Written by Daniel Vetter's request and largely based on his comments in IRC and
> > from https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Fdri-devel%2F2020-May%2F265484.html&amp;data=02%7C01%7Candrey.grodzovsky%40amd.com%7Ce8e13dc4c85648e5fcd408d7fbdc5f2b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637254796242596783&amp;sdata=bA%2FAy3VGvzNqmV1kGigNROSZQEws2E98JibDxvEICNs%3D&amp;reserved=0 .
> >
> > Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Sean Paul <sean@poorly.run>
> >
> > ---
> >
> > Disclaimer: I am a userspace developer writing for other userspace developers.
> > I took some liberties in defining what should happen without knowing what is
> > actually possible or what existing drivers already implement.
> > ---
> >   Documentation/gpu/drm-uapi.rst | 75 ++++++++++++++++++++++++++++++++++
> >   1 file changed, 75 insertions(+)
> >
> > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> > index 56fec6ed1ad8..80db4abd2cbd 100644
> > --- a/Documentation/gpu/drm-uapi.rst
> > +++ b/Documentation/gpu/drm-uapi.rst
> > @@ -1,3 +1,5 @@
> > +.. Copyright 2020 DisplayLink (UK) Ltd.
> > +
> >   ===================
> >   Userland interfaces
> >   ===================
> > @@ -162,6 +164,79 @@ other hand, a driver requires shared state between clients which is
> >   visible to user-space and accessible beyond open-file boundaries, they
> >   cannot support render nodes.
> >   
> > +Device Hot-Unplug
> > +=================
> > +
> > +.. note::
> > +   The following is the plan. Implementation is not there yet
> > +   (2020 May 13).
> > +
> > +Graphics devices (display and/or render) may be connected via USB (e.g.
> > +display adapters or docking stations) or Thunderbolt (e.g. eGPU). An end
> > +user is able to hot-unplug this kind of devices while they are being
> > +used, and expects that the very least the machine does not crash. Any
> > +damage from hot-unplugging a DRM device needs to be limited as much as
> > +possible and userspace must be given the chance to handle it if it wants
> > +to. Ideally, unplugging a DRM device still lets a desktop to continue
> > +running, but that is going to need explicit support throughout the whole
> > +graphics stack: from kernel and userspace drivers, through display
> > +servers, via window system protocols, and in applications and libraries.
> > +
> > +Other scenarios that should lead to the same are: unrecoverable GPU
> > +crash, PCI device disappearing off the bus, or forced unbind of a driver
> > +from the physical device.
> > +
> > +In other words, from userspace perspective everything needs to keep on
> > +working more or less, until userspace stops using the disappeared DRM
> > +device and closes it completely. Userspace will learn of the device
> > +disappearance from the device removed uevent or in some cases specific
> > +ioctls returning EIO.
> > +
> > +This goal raises at least the following requirements for the kernel and
> > +drivers:
> > +
> > +- The kernel must not hang, crash or oops, no matter what userspace was
> > +  in the middle of doing when the device disappeared.
> > +
> > +- All GPU jobs that can no longer run must have their fences
> > +  force-signalled to avoid inflicting hangs to userspace.
> > +
> > +- KMS connectors must change their status to disconnected.
> > +
> > +- Legacy modesets and pageflips fake success.
> > +
> > +- Atomic commits, both real and TEST_ONLY, fake success.  
> 
> 
> Why wouldn't we return -EIO for the atommic commit IOTCL/legasy pflip 
> and modeset ioctls here same way as you suggested returning -EIO for 
> render ioctl ?

Hi,

that is more of a question for Daniel Vetter than me. I believe he is
worried that userspace will get the error handling horribly wrong
anyway, because it needs to be handled in every single display server
project. Render ioctl errors OTOH need to be handled only in the
corresponding Mesa or other userspace driver, and for render there are
API (OpenGL/EGL, Vulkan) specs that say how it must be handled to fill
the API contract. Because of the API contract, those are more likely to
have reasonable error handling in place already.

I first thought it would be obvious for at least atomic commits to fail
appropriately, but then thinking again, it will only introduce new
failures that are currently very hard to test for (VKMS to the rescue),
and userspace would need to have code to correctly bail out for EIO
rather than attempt fallbacks. The uevent telling the device is gone is
going to come anyway. In between the device disappearance and uevent
handling, it doesn't matter much what happens: the device is abruptly
gone and the user will see a glitch anyway - he knows, he just yanked
out the cable.

To me this decision is just to make life in userspace easier by not
introducing a whole new type of error. However, if the kernel did
return EIO instead, it would be impossible for me to see that as a
kernel regression because the old behaviour was probably explosions
everywhere anyway.

I heard that UDL and maybe some other drivers in the kernel already
handle hot-unplug somehow. Maybe those are what would regress if the
kernel returned EIO?


> > +
> > +- Pending non-blocking KMS operations deliver the DRM events userspace
> > +  is expecting.
> > +
> > +- If underlying memory disappears, the mmaps are replaced with harmless
> > +  zero pages where access does not raise SIGBUS. Reads return zeros,
> > +  writes are ignored.
> > +
> > +- dmabuf which point to memory that has disappeared are rewritten to
> > +  point to harmless zero pages, similar to mmaps. Imports still succeed
> > +  both ways: an existing device importing a dmabuf pointing to
> > +  disappeared memory, and a disappeared device importing any dmabuf.  
> 
> 
> Same as above, I would expect drmPrimeHandleToFD and drmPrimeFDToHandle 
> to return -EIO in case device is detached

I imagined a use case behind this: a Wayland compositor attempting to
import a client's dmabuf. The GPU the client was using is
hot-unplugged, meaning the dmabufs lose their storage. It is up to the
client to handle its rendering GPU disappearance on its own, but also
the compositor should not disconnect it because the GPU disappeared.
It's not the client's fault the GPU disappeared.

In Wayland, failure to use a wl_buffer is considered a protocol error,
and protocol errors are always fatal: the client is disconnected. The
rationale is that the compositor must always be able to present the
client buffer somehow. If the compositor cannot, then the client did
not obey the protocol.

The fallback presentation path in a compositor is usually importing the
dmabuf to EGL, to be sampled from OpenGL. Normally the protocol
guarantees that this works, so any failure to do so is a protocol
violation. But if the GPU used by the client suddenly disappears and
the imports start to fail, that is interpreted as a protocol violation
unless the compositor can see why the import failed. Since the import
is done via EGL, getting the right error code plumbed through from
libdrm functions to the EGL caller would be a hassle. I don't see any
error code in EGL_EXT_image_dma_buf_import reserved for "the dmabuf
storage was hot-unplugged", and I doubt there is anything exclusively
for only that in the EGL base spec either.

The cost of lying that the import worked is that the compositor will
paint black or transparent where the window was supposed to be. It's a
graphical glitch that is contrary to the Wayland design principles, but
in this case a glitch is unavoidable: even if the compositor knew this
buffer is now bad, what would it paint instead? It has nothing else to
paint from. I'm assuming the compositor is using a different GPU than
what disappeared.

Ideally, the client will eventually react to losing the GPU and either
crash, quit, or switch its rendering to something that works which
simply gives the compositor a new, working buffer without losing any
window state in the process. If we risk the compositor disconnecting
the client, then the client might not recover even if it wanted to.

> > +
> > +- Render ioctls return EIO which is then handled in userspace drivers,
> > +  e.g. Mesa, to have the device disappearance handled in the way
> > +  specified for each API (OpenGL, GL ES: GL_KHR_robustness;
> > +  Vulkan: VK_ERROR_DEVICE_LOST; etc.)  
> 
> 
> As far as I understood from our discussion we would reject any IOCTL 
> aimed at a device which is gone and not only render ioctls.

Daniel?


Thanks,
pq

> 
> Andrey
> 
> 
> > +
> > +Raising SIGBUS is not an option, because userspace cannot realistically
> > +handle it.  Signal handlers are global, which makes them extremely
> > +difficult to use correctly from libraries like Mesa produces. Signal
> > +handlers are not composable, you can't have different handlers for GPU1
> > +and GPU2 from different vendors, and a third handler for mmapped regular
> > +files.  Threads cause additional pain with signal handling as well.
> > +
> > +Only after userspace has closed all relevant DRM device and dmabuf file
> > +descriptors and removed all mmaps, the DRM driver can tear down its
> > +instance for the device that no longer exists. If the same physical
> > +device somehow comes back in the mean time, it shall be a new DRM
> > +device.
> > +
> >   .. _drm_driver_ioctl:
> >   
> >   IOCTL Support on Device Nodes  


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel