Re: [PATCH] drm: enable render nodes wherever buffer sharing is supported

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Apr 30, 2020 at 12:57:40PM -0700, Eric Anholt wrote:
> On Thu, Apr 30, 2020 at 3:44 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Tue, Apr 28, 2020 at 2:46 PM Peter Collingbourne <pcc@google.com> wrote:
> > >
> > > Render nodes are not just useful for devices supporting GPU hardware
> > > acceleration. Even on devices that only support dumb frame buffers,
> > > they are useful in situations where composition (using software
> > > rasterization) and KMS are done in different processes with buffer
> > > sharing being used to send frame buffers between them. This is the
> > > situation on Android, where surfaceflinger is the compositor and the
> > > composer HAL uses KMS to display the buffers. Thus it is beneficial
> > > to expose render nodes on all devices that support buffer sharing.
> > >
> > > Because all drivers that currently support render nodes also support
> > > buffer sharing, the DRIVER_RENDER flag is no longer necessary to mark
> > > devices as supporting render nodes, so remove it and just rely on
> > > the presence of a prime_handle_to_fd function pointer to determine
> > > whether buffer sharing is supported.
> >
> > The idea behind render nodes is that you can freely pass these to
> > unpriviledged users, and nothing bad happens. That's why we have gpu
> > reset code in drivers, proper pagetables, and also (in at least the
> > solid drivers) ban code so that repeat offenders from userspace who
> > constantly submit endless batch buffers and funny stuff like that
> > can't DOS the gpu.
> >
> > Ofc in practice there's hw issues and fun stuff like that sometimes,
> > and driver bugs, and all that. But that's the aspiration.
> >
> > Now many of these display-only drivers need contiguous buffers, and
> > there's not endless amounts of that around. So if you allow random
> > clients to allocate buffers, they can easily exhaust that, and not
> > just upset the render side of the gpu, but essentially make it
> > impossible for a compositor to allocate more framebuffers. I don't
> > think that's a good idea.
> >
> > I know there's hw like vc4 which needs contiguous buffers for
> > everything, but that's kinda the places where aspiration falls a bit
> > short.
> >
> > So from that pov I'm a rather worried with handing out render rights
> > to everyone for these display-only buffers. It's not entirely
> > harmless.
> 
> This doesn't feel like a contiguous-mem-specific concern to me.  We
> don't have resource limits on renderer GPU nodes today, you can
> allocate memory there to fill up and DOS the system, and unless
> something changed since last time I was looking, we don't even tell
> the OOM killer about our allocations so they can kill the right app!
> (my compositor always got picked, in my experience)
> 
> And, keep in mind what we're fixing at a system level here: the
> current workaround is you either get your compositor to hand you a GPU
> fd, at which point you can DOS the system, or you open the master node
> yourself and try to drop master before the compositor comes up, then
> DOS the system. "But there are access controls for the compositor or
> the card node!" you say: yes, but there are for render nodes too.  I
> know my distro doesn't default to open access to /dev/dri/render*

Hm indeed, I thought we've limited dumb buffer creation to master only on
the primary device.

But that kinda leaves me wondering, how do you allocate buffers on these
then? Exposing dumb on render nodes gets the Dave "it's not for rendering"
Airlie nack, at least for drivers which do have real userspace sitting on
the same drm driver. And exposing dumb only for these display-only devices
feels somewhat silly and inconsistent too.

So not sure.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel