[PATCH] drm/amdgpu: fix drm leases being broken on radv

[Daniel Vetter]

On Wed, Apr 17, 2019 at 02:46:06PM +0200, Christian König wrote:
> Am 17.04.19 um 14:35 schrieb Daniel Vetter:
> > On Wed, Apr 17, 2019 at 12:06:32PM +0000, Koenig, Christian wrote:
> > > Am 17.04.19 um 14:00 schrieb Daniel Vetter:
> > > > On Wed, Apr 17, 2019 at 11:18:35AM +0000, Koenig, Christian wrote:
> > > > > Am 17.04.19 um 13:06 schrieb Daniel Vetter:
> > > > > > On Wed, Apr 17, 2019 at 12:29 PM Koenig, Christian
> > > > > > <Christian.Koenig at amd.com> wrote:
> > > > > > [SNIP]
> > > > > Well, what you guys did here is a serious no-go.
> > > > Not really understanding your reasons for an unconditional nak on all this
> > > > still.
> > > Well, let me summarize: You worked around a problem with libva by
> > > breaking another driver and now proposing a rather ugly and only halve
> > > backed workaround for that driver.
> > > 
> > > This is a serious no-go. I've already send out a i915 specific change to
> > > remove DRM_AUTH from IOCTLs which also have DRM_RENDER_ALLOW and revert
> > > the offending patch.
> > Oh I'm totally fine with the revert if that's what we go with. But that
> > still leaves the issue that it would make sense to drop DRM_AUTH from
> > DRIVER_RENDER capable drivers (not just for libva, but in general). And
> > there's fundamentally 2 options to do that:
> > 
> > - This one here (or anything implementing the same idea), to keep radv
> >    working.
> > 
> > - We drop DRM_AUTH from all drivers with DRIVER_RENDER except amdgpu. How
> >    exactly that's doen doesn't matter, but it leaves amdgpu out in the
> >    cold. It also doesn't matter whether we get there with revert + lots of
> >    patches, or a special hack for amdgpu, in the end amdgpu would be
> >    different. Also note that your patch isn't enough, since it doesn't fix
> >    the core ioctls.
> > 
> > I think from an overall platform pov, the first is the better option.
> > After all we try really hard to avoid driver special cases for these kinds
> > of things.
> 
> Well, I initially thought that radv is doing something unusual or broken,
> but after looking deeper into this that is actually not the case.
> 
> What radv does is calling an IOCTL and expecting an error message when it is
> not authenticated.
> 
> It looks like libdrm_amdgpu has switched to using DRM_IOCTL_GET_CLIENT to
> figure out if it is authenticated or not, but I clearly remember that we
> haven't done this from the beginning.

Maybe in the radoen code, or some earlier internal amdgpu libdrm code?
First public commit has all the bits: getauth, GetVersion, then the accel
query.

> Thinking more about this I don't think that this problem is actually amdgpu
> specific. So I wouldn't drop DRM_AUTH from all render node drivers at all,
> but only from those who have the specific issue with libva.

libva was the initial motivation, the goal of Emil's patch wasn't to just
duct-tape over libva. That would be easier to fix in libva. The point was
that we should be able to allow this in general.

And we can, except for the radv issue uncovered here.

So please don't get 100% hung up on the libva thing, that wasn't really
the goal, just the initial motivation. And I still thinks it makes for all
drivers. So again we have:

- radv hack
- make amdgpu behave different from everyone else
- keep tilting windmills about "pls use rendernodes, thx"

I neither like tilting windmills nor making drivers behave different for
no reaason at all.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch