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

[Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 2019/06/21, Koenig, Christian wrote:
> No this should apply to all drivers, not just amdgpu.
> 
> > While I suggested:
> >   - keeping amdgpu consistent with the rest
> >   - exposing the KConfig option for the whole of the kernel, and
> >   - merging it alongside this work
> >
> > So I effectively waited for weeks, instead of simply going ahead and
> > writing/submitting patches. That's a bit unfortunate...
> >
> >>> Because we simply made sure that userspace is using the render node for
> >>> command submission and not the primary node.
> >>>
> >>> So nobody can go ahead and remove render node support any more :)
> >> How does this work? I thought the entire problem of DRM_AUTH removal
> >> for you was that it broke stuff, and you didn't want to deal with
> >> that. We still have that exact problem afaics ... old userspace
> >> doesn't simply vanish, even if you entirely nuke it going forward.
> >>
> >> Also, testing on the amdgpu stack isn't really testing a hole lot
> >> here, it's all the various silly compositors out there I'd expect to
> >> fall over. Will gbm/radeonsi/whatever just internally do all the
> >> necessary dma-buf import/export between the primary node and display
> >> node to keep this all working?
> > If I understood Christian, feel free to correct me, the fact that my
> > earlier patch broke RADV was not the argument. The problem was was the
> > patch does.
> 
> Well partially. That RADV broke is unfortunate, but we have done so many 
> customized userspace stuff both based on Mesa/DDX as well as closed 
> source components that it is just highly likely that we would break 
> something else as well.
> 
As an engineer I like to work with tangibles. The highly likely part is
probable, but as mentioned multiple times the series allows for a _dead_
trivial way to address any such problems.

> > In particular, that user-space will "remove" render nodes.
> 
> Yes, that is my main concern here. You basically make render nodes 
> superfluously. That gives userspace all legitimization to also remove 
> support for them. That is not stupid or evil or whatever, userspace 
> would just follow the kernel design.
> 
Do you have an example of userspace doing such an extremely silly thing?
It does seem like suspect you're a couple of steps beyond overcautious,
perhaps rightfully so. Maybe you've seen some closed-source user-space
going crazy? Or any other projects?

In other words, being cautious is great, but without references of
misuse it's very hard for others to draw the full picture.

> > I'm really sad that amdgpu insists on standing out, hope one day it will
> > converge. Yet since all other maintainers are ok with the series, I'll
> > start merging patches in a few hours. I'm really sad that amdgpu wants
> > to stand out, hope it will converge sooner rather than later.
> >
> > Christian, how would you like amdgpu handled - with a separate flag in
> > the driver or shall we special case amdgpu within core drm?
> 
> No, I ask you to please stick around DRM_AUTH for at least amdgpu and 
> radeon. And NOT enable any render node functionality for them on the 
> primary node.
> 
My question is how do you want this handled:
 - DRM_PLEASE_FORCE_AUTH - added to AMDGPU/RADEON, or
 - driver_name == amdgpu, in core DRM.


Thanks
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx