[PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround

[Daniel Vetter]

On Fri, Jun 21, 2019 at 05:15:22PM +0200, Michel Dänzer wrote:
> On 2019-06-21 1:50 p.m., Daniel Vetter wrote:
> > On Fri, Jun 21, 2019 at 1:37 PM Christian König
> > <ckoenig.leichtzumerken at gmail.com> wrote:
> >> Am 21.06.19 um 13:03 schrieb Daniel Vetter:
> >>> So if you want to depracate render functionality on primary nodes, you
> >>> _have_ to do that hiding in userspace. Or you'll break a lot of
> >>> compositors everywhere. Just testing -amdgpu doesn't really prove
> >>> anything here. So you won't move the larger ecosystem with this at
> >>> all, that ship sailed.
> >>
> >> So what else can we do? That sounds like you suggest we should
> >> completely drop render node functionality.
> >>
> >> I mean it's not my preferred option, but certainly something that
> >> everybody could do.
> >>
> >> My primary concern is that we somehow need to get rid of thinks like GEM
> >> flink and all that other crufty stuff we still have around on the
> >> primary node (we should probably make a list of that).
> >>
> >> Switching everything over to render nodes just sounded like the best
> >> alternative so far to archive that.
> > 
> > tbh I do like that plan too, but it's a lot more work. And I think to
> > have any push whatsoever we probably need to roll it out in gbm as a
> > hack to keep things going. But probably not enough.
> > 
> > I also think that at least some compositors will bother to do the
> > right thing, and actually bother with render nodes and all that
> > correctly. It's just that there's way more which dont.
> 
> With amdgpu, we can make userspace always use render nodes for rendering
> with changes to libdrm_amdgpu/radeonsi/xf86-video-amdgpu (and maybe some
> amdgpu-pro components) only. No GBM/compositor changes needed.

This is a very non-exhaustive list of userspace that runs on your driver
... This wayland compositor thing, actually shipping now, and there's many :-)

> >>> At that point this all feels like a bikeshed, and for a bikeshed I
> >>> don't care what the color is we pick, as long as they're all painted
> >>> the same.
> 
> Then some sheds shouldn't have been re-painted without DRM_AUTH already...

Christian proposed that and then didn't feel like reverting it, plus vc4,
and tegra never bothered with it. There's quite a bit more than the tail
out of this particular bag already.

> >>> Once we picked a color it's a simple technical matter of how to roll
> >>> it out, using Kconfig options, or only enabling on new hw, or "merge
> >>> and fix the regressions as they come" or any of the other well proven
> >>> paths forward.
> >>
> >> Yeah, the problem is I don't see an option which currently works for
> >> everyone.
> >>
> >> I absolutely need a grace time of multiple years until we can apply this
> >> to amdgpu/radeon.
> > 
> > Yeah that's what I meant with "pick a color, pick a way to roll it
> > out". "enable for new hw, roll out years and years later" is one of
> > the options for roll out.
> 
> One gotcha being that generic userspace such as the Xorg modesetting
> driver may still try to use phased out functionality such as DRI2 with
> new hardware.

There's a lot more generic userspace than -modesetting. That was the
entire point of kms, and it succeed really well. That's why I don't think
amdgpu doing their own flavour pick is going to help anyone here, except
maybe if all you care about is the amd stand-alone stack only. But then
why do you bother with this upstream stuff at all ...

> >> How about this:
> >> 1. We keep DRM_AUTH around for amdgpu/radeon for now.
> >> 2. We add a Kconfig option to ignore DRM_AUTH, currently default to N.
> >> This also works as a workaround for old RADV.
> >> 3. We start to work on further removing old cruft from the primary node.
> >> Possible the Kconfig option I suggested to disable GEM flink.
> > 
> > Hm I'm not worried about flink really. It's gem_open which is the
> > security gap, and that one has to stay master-only forever.
> 
> GEM_OPEN is used by DRI2 clients, it can't be master-only. It's probably
> one of the main reasons for the existence of DRM_AUTH.

Oh sweet I forgot we're allowing this both ways :-/ Well doesn't change that
much really, once we break one of these the other isn't useful anymore
either.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch