On Tue, May 28, 2019 at 8:58 AM Koenig, Christian <christian.koe...@amd.com> wrote: > > Am 27.05.19 um 17:26 schrieb Daniel Vetter: > > On Mon, May 27, 2019 at 3:42 PM Koenig, Christian > > <christian.koe...@amd.com> wrote: > >> Am 27.05.19 um 15:26 schrieb Emil Velikov: > >>> On 2019/05/27, Daniel Vetter wrote: > >>>> On Mon, May 27, 2019 at 10:47:39AM +0000, Koenig, Christian wrote: > >>>>> Am 27.05.19 um 10:17 schrieb Emil Velikov: > >>>>>> From: Emil Velikov <emil.veli...@collabora.com> > >>>>>> > >>>>>> Currently one can circumvent DRM_AUTH, when the ioctl is exposed via > >>>>>> the > >>>>>> render node. A seemingly deliberate design decision. > >>>>>> > >>>>>> Hence we can drop the DRM_AUTH all together (details in follow-up > >>>>>> patch) > >>>>>> yet not all userspace checks if it's authenticated, but instead uses > >>>>>> uncommon assumptions. > >>>>>> > >>>>>> After days of digging through git log and testing, only a single > >>>>>> (ab)use > >>>>>> was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > >>>>>> assuming that failure implies lack of authentication. > >>>>>> > >>>>>> Affected versions are: > >>>>>> - the whole 18.2.x series, which is EOL > >>>>>> - the whole 18.3.x series, which is EOL > >>>>>> - the 19.0.x series, prior to 19.0.4 > >>>>> Well you could have saved your time, cause this is still a NAK. > >>>>> > >>>>> For the record: I strongly think that we don't want to expose any render > >>>>> functionality on the primary node. > >>>>> > >>>>> To even go a step further I would say that at least on AMD hardware we > >>>>> should completely disable DRI2 for one of the next generations. > >>>>> > >>>>> As a follow up I would then completely disallow the DRM authentication > >>>>> for amdgpu, so that the command submission interface on the primary node > >>>>> can only be used by the display server. > >>>> So amdgpu is running in one direction, while everyone else is running in > >>>> the other direction? Please note that your patch to change i915 landed > >>>> already, so that ship is sailing (but we could ofc revert that back > >>>> again). > >>>> > >>>> Imo really not a great idea if we do a amdgpu vs. everyone else split > >>>> here. If we want to deprecate dri2/flink/rendering on primary nodes > >>>> across > >>>> the stack, then that should be a stack wide decision, not an amdgpu one. > >>>> > >>> Cannot agree more - I would love to see drivers stay consistent. > >> Yeah, completely agree to that. That's why I think we should not do this > >> at all and just let Intel fix it's userspace bugs :P > > So you're planning to submit that revert? You did jump the gun with > > sending out that patch after all too ... (aside from it got merged > > because of some other mixup with r-b tags and what not). > > Well already regretting submitting that. On the other hand what is the > minimum IOCTLs we need to get working to fix the vainfo issue?
We have a bit more time, it's only going to be in 5.3. But yeah if we don't bottom out on any of the options here I think revert it has to be. > Might be a good idea looking into reverting it partially, so that at > least command submission and buffer allocation is still blocked. I thought the issue is a lot more than vainfo, it's pretty much every hacked up compositor under the sun getting this wrong one way or another. Thinking about this some more, I also have no idea how you'd want to deprecate rendering on primary nodes in general. Apparently that breaks -modesetting already, and probably lots more compositors. And it looks like we're finally achieve the goal kms set out to 10 years ago, and new compositors are sprouting up all the time. I guess we could just break them all (on new hardware) and tell them to all suck it up. But I don't think that's a great option. And just deprecating this on amdgpu is going to be even harder, since then everywhere else it'll keep working, and it's just amdgpu.ko that looks broken. Aside: I'm not supporting Emil's idea here because it fixes any issues Intel has - Intel doesn't care. I support it because reality sucks, people get this render vs. primary vs. multi-gpu prime wrong all the time (that's also why we have hardcoded display+gpu pairs in mesa for the various soc combinations out there), and this looks like a pragmatic solution. It'd be nice if every compositor and everything else would perfectly support multi gpu and only use render nodes for rendering, and only primary nodes for display. But reality is that people hack on stuff until gears on screen and then move on to more interesting things (to them). So I don't think we'll ever win this :-/ -Daniel > Christian. > > > -Daniel > > > >> Anyway my concern is really that we should stop extending functionality > >> on the primary node. > >> > >> E.g. the render node is for use by the clients and the primary node for > >> mode setting and use by the display server only. > >> > >> Regards, > >> Christian. > >> > >>> Fwiw, this series consistently paves the way toward nuking DRM_AUTH ;-) > >>> > >>> Thanks > >>> Emil > > > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
