[PATCH libdrm] libdrm: Allow dynamic drm majors on linux

[Emil Velikov]

On 5 September 2018 at 11:10, Thomas Hellstrom <thellstrom at vmware.com> wrote:
> Hi, Emil,
>
>
> On 09/05/2018 11:33 AM, Emil Velikov wrote:
>>
>> On 4 September 2018 at 23:33, Dave Airlie <airlied at gmail.com> wrote:
>>>
>>> On Tue, 4 Sep 2018 at 03:00, Thomas Hellstrom <thellstrom at vmware.com>
>>> wrote:
>>>>
>>>> On 09/03/2018 06:33 PM, Daniel Vetter wrote:
>>>>>
>>>>> On Mon, Sep 03, 2018 at 11:16:29AM +0200, Thomas Hellstrom wrote:
>>>>>>
>>>>>> On 08/31/2018 05:30 PM, Thomas Hellstrom wrote:
>>>>>>>
>>>>>>> On 08/31/2018 05:27 PM, Emil Velikov wrote:
>>>>>>>>
>>>>>>>> On 31 August 2018 at 15:38, Michel Dänzer <michel at daenzer.net>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> [ Adding the amd-gfx list ]
>>>>>>>>>
>>>>>>>>> On 2018-08-31 3:05 p.m., Thomas Hellstrom wrote:
>>>>>>>>>>
>>>>>>>>>> On 08/31/2018 02:30 PM, Emil Velikov wrote:
>>>>>>>>>>>
>>>>>>>>>>> On 31 August 2018 at 12:54, Thomas Hellstrom
>>>>>>>>>>> <thellstrom at vmware.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> To determine whether a device node is a drm device
>>>>>>>>>>>> node or not, the code
>>>>>>>>>>>> currently compares the node's major number to the static drm
>>>>>>>>>>>> major
>>>>>>>>>>>> device
>>>>>>>>>>>> number.
>>>>>>>>>>>>
>>>>>>>>>>>> This breaks the standalone vmwgfx driver on XWayland dri
>>>>>>>>>>>> clients,
>>>>>>>>>>>>
>>>>>>>>>>> Any particular reason why the code doesn't use a fixed node
>>>>>>>>>>> there?
>>>>>>>>>>> It will make the diff vs the in-kernel driver a bit smaller.
>>>>>>>>>>
>>>>>>>>>> Because then it won't be able to interoperate with other in-tree
>>>>>>>>>> drivers, like virtual drm drivers or passthrough usb drm drivers.
>>>>>>>>>> There is no clean way to share the minor number allocation
>>>>>>>>>> with in-tree
>>>>>>>>>> drm, so standalone vmwgfx is using dynamic major allocation.
>>>>>>>>>
>>>>>>>>> I wonder why I haven't heard of any of these issues with the
>>>>>>>>> standalone
>>>>>>>>> version of amdgpu shipped in packaged AMD releases. Does that
>>>>>>>>> also use a
>>>>>>>>> different major number? If yes, maybe it's just that nobody has
>>>>>>>>> tried
>>>>>>>>> Xwayland clients with that driver. If no, how does it avoid the
>>>>>>>>> other
>>>>>>>>> issues described above?
>>>>>>>>>
>>>>>>>> AFAICT, the difference is that the standalone vmwgfx uses an
>>>>>>>> internal
>>>>>>>> copy of drm core.
>>>>>>>> It doesn't reuse the in-kernel drm, hence it cannot know which minor
>>>>>>>> it can use.
>>>>>>>>
>>>>>>>> -Emil
>>>>>>>
>>>>>>> Actually, standalone vmwgfx could perhaps also try to allocate minors
>>>>>>> from 63 and downwards. That might work, but needs some verification.
>>>>>>>
>>>>>> So unfortuntately this doesn't work since the in-tree drm's file
>>>>>> operations
>>>>>> are registered with the DRM_MAJOR.
>>>>>> So I still think the patch is the way to go. If people are concerned
>>>>>> that
>>>>>> also fbdev file descriptors are allowed, perhaps there are other sysfs
>>>>>> traits we can look at?
>>>>>
>>>>> Somewhat out of curiosity, but why do you have to overwrite all of drm?
>>>>> amdgpu seems to be able to pull their stunt off without ...
>>>>> -Daniel
>>>>
>>>> At the time we launched the standalone vmwgfx, the DRM <-> driver
>>>> interface was moving considerably more rapidly than the DRM <-> kernel
>>>> interface. I think that's still the case. Hence less work for us. Also
>>>> meant we can install the full driver stack with latest features on
>>>> fairly old VMs without backported DRM functionality.
>>>>
>>> I think this should be fine for 99% of drm usage, there may be corner
>>> cases in wierd places, but I can't point to any that really matter
>>> (maybe strace?)
>>>
>> Having a closer look, I think this will break the Firefox/Chrome
>> sandboxing :-\
>> I cannot see the path /sys/dev/char/%d:%d/device/drm in the allowed
>> list [1] [2].
>
> Thanks for pointing this out.
>
> The function drmGetMinorNameForFD() already opens this path, so any
> user-space using that function would not work before either.
>
> Also mozilla/firefox adds /sys/dev/char/226:* Which means that while it
> still won't work on standalone vmwgfx, there should at least be no
> regression.
>
> For Chromium it seems they allow /sys/dev/char/ for AmdGpu, but only under
> ChromOS, so I'll ping those to be safe.
>
> I also won't be doing an immediate release after pushing.
>
In that case, please give me 24h to do a libdrm release before pushing.
I had to push some workarounds for the sandboxing mentioned earlier :-\

Thanks
Emil