On 2019/06/14, Koenig, Christian wrote: > Am 14.06.19 um 14:09 schrieb Emil Velikov: > > On 2019/05/27, Emil Velikov wrote: > > [SNIP] > > Hi Christian, > > > > > > In the following, I would like to summarise and emphasize the need for > > DRM_AUTH removal. I would kindly ask you to spend a couple of minutes > > extra reading it. > > > > > > Today DRM drivers* do not make any distinction between primary and > > render node clients. > > That is actually not 100% correct. We have a special case where a DRM > master is allowed to change the priority of render node clients. > Can you provide a link? I cannot find that code. > > Thus for a render capable driver, any premise of > > separation, security or otherwise imposed via DRM_AUTH is a fallacy. > > Yeah, that's what I agree on. I just don't think that removing DRM_AUTH > now is the right direction to take. > Could have been clearer - I'm talking about DRM_AUTH | DRM_RENDER_ALLOW ioctls. That aside, can you propose an alternative solution that addresses this and the second point just below? > > Considering the examples of flaky or outright missing drmAuth in prime > > open-source projects (mesa, kmscube, libva) we can reasonably assume > > other projects exbibit the same problem. > > > > For these and/or other reasons, two DRM drivers have dropped DRM_AUTH > > since day one. > > > > Since we are interested in providing consistent and predictable > > behaviour to user-space, dropping DRM_AUTH seems to be the most > > effective way forward. > > Well and what do you do with drivers which doesn't implement render nodes? > AFAICT there is a single non-DC driver which does not expose - QXL. I would expect it runs on a rather customised stack. Would be great to fix that, but ETIME and it's the only exception to the rule. > > Of course, I'd be more than happy to hear for any other way to achieve > > the goal outlined. > > > > Based on the series, other maintainers agree with the idea/goal here. > > Amdgpu not following the same pattern would compromise predictability > > across drivers and complicate userspace, so I would kindly ask you to > > reconsider. > > > > Alternatively, I see two ways to special case: > > - New flag annotating amdgpu/radeon - akin to the one proposed earlier > > - Check for amdgpu/radeon in core DRM > > I perfectly agree that I don't want any special handling for amdgpu/radeon. > > My key point is that with DRM_AUTH we created an authorization and > authentication mess in DRM which is functionality which doesn't belong > into the DRM subsystem in the first place. > Precisely and I've outlined below how to resolve this in the long run. > > Now on your pain point - not allowing render iocts via the primary node, > > and how this patch could make it harder to achieve that goal. > > > > While I love the idea, there are number of obstacles that prevent us > > from doing so at this time: > > - Ensuring both old and new userspace does not regress > > Yeah, agree totally. That's why I said we should probably start doing > this for only for upcoming hardware generations. > That will side-step the regression issue, but will enforce driver specific behaviour outlined before. > > - Drivers (QXL, others?) do not expose a render node > > Well isn't that is a rather big problem for the removal of DRM_AUTH in > general? > > E.g. at least QXL would then expose functionality on the primary node > without authentication. > With this series QXL remains functionally unchanged. I would love to fix that as well yet ETIME as mentioned above. > > - We want to avoid driver specific behaviour > > > > The only way forward that I can see is: > > - Address QXL/others to expose render nodes > > - Add a Kconfig toggle to disable !KMS ioctls via the primary node > > - Jump-start ^^ with distribution X > > - Fix user-space fallouts > > - X months down the line, flip the Kconfig > > - In case of regressions - revert the KConfig, goto Fix user-space... > > Well that at least sounds like a plan :) Let's to this! > We're talking about months if not years until it comes to fruition. We need something quicker. That said, I'm quite happy to take the first stab, yet this is not a replacement for this series. > > That said, the proposal will not conflict with the DRM_AUTH removal. If > > anything it is step 0.5 of the grand master plan. > > That's the point I strongly disagree on. > > By lowering the DRM_AUTH restriction you are encouraging userspace to > use the shortcut and use the primary node for rendering instead of > fixing the code and using the render node. > Have to disagree here. After working on the user-space side and fixing issues in various projects I can honestly say that most user-space is sane and developers _care_ about doing things correctly. > So at the end of the day userspace will most likely completely drop > support for the render node, simply for the reason that it became > superfluous. You can just open up the primary node and get the same > functionality. > I think you're being overly pessimistic here. This is coming from a person who is often closer to the pessimistic side of things. As a middle ground how about we do the following: - Get this series as-is, alongside - picking the first three items from the grand master plan - happy to start a discussion about QXL - a patch for the Kconfig toggle, is coming in a few minutes - happy to prod my distribution of choice (Arch) and work with them What do you think? Thanks Emil _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
