Static inline DRM functions calling into GPL-only code

[Harry Wentland]

On 2017-04-11 11:15 AM, James Jones wrote:
> On 04/10/2017 11:20 PM, Daniel Vetter wrote:
>> On Tue, Apr 11, 2017 at 7:52 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
>>> On Tue, Apr 11, 2017 at 6:14 AM, Nikhil Mahale <nmahale at nvidia.com>
>>> wrote:
>>>> My name is Nikhil Mahale, and I work at NVIDIA in the Linux drivers
>>>> team.
>>>>
>>>> I have been working on adding DRM KMS support to our driver. The NVIDIA
>>>> GPU driver package (364.12 and higher) provides a kernel module,
>>>> nvidia-drm.ko, which is licensed as "MIT". This module registers a DRM
>>>> driver with the DRM subsystem of the Linux kernel and advertises KMS
>>>> capability on Linux kernel v4.1 or higher, with CONFIG_DRM and
>>>> CONFIG_DRM_KMS_HELPER enabled.
>>>>
>>>> We have been able to maintain compatibility between nvidia-drm.ko and
>>>> Linux kernels from v2.6.9 to v4.10. Unfortunately
>>>> with release candidates of v4.11:
>>>>
>>>> * Commit 10383aea2f445bce9b2a2b308def08134b438c8e changed the kernel's
>>>> kref implementation to use refcount_inc and refcount_dec_and_test.
>>>> * Commit 29dee3c03abce04cd527878ef5f9e5f91b7b83f4 made refcount_inc and
>>>> refcount_dec_and_test EXPORT_SYMBOL_GPL.
>>>>
>>>> DRM drivers call refcount_inc through static inline function callchains
>>>> such as:
>>>>
>>>>     drm_crtc_commit_put() => kref_put() => refcount_dec_and_test()
>>>>     drm_crtc_commit_get() => kref_get() => refcount_inc()
>>>>
>>>>     drm_atomic_state_put() => kref_put() => refcount_dec_and_test()
>>>>     drm_atomic_state_get() => kref_get() => refcount_inc()
>>>>
>>>>     drm_gem_object_reference() => kref_get => refcount_inc()
>>>>
>>>> This causes nvidia-drm.ko to inadvertently pick up references to
>>>> EXPORT_SYMBOL_GPL symbols.
>>>>
>>>> There is not interest in relaxing the export of refcount_inc, and
>>>> changing the license of nvidia-drm.ko isn't viable right now.
>>>>
>>>> So, the remaining options we see are:
>>>>
>>>> * Make these static inline DRM functions EXPORT_SYMBOL instead of
>>>> inline.
>>>>
>>>> * Make these static inline DRM functions not use kref.
>>>>
>>>> * Make nvidia-drm.ko not use these static inline DRM functions.
>>>>
>>>> None of those seem good, though the first might be least bad.  Do
>>>> any of
>>>> those seem reasonable?
>>>
>>> * Open-source the nvidia kernel driver? tbh I'm not sure how much you
>>> can still make the case that your driver is fully an independent thing
>>> if you're adopting stuff like atomic modesetting. Might be better to
>>> make all the glue/remapping code from linux atomic to the shared
>>> cross-os code at least open
>
> As the original message stated, this code is already open (MIT license).
>

Just out of curiosity, can I find this on any public repo or webpage?

If inlining is the issue it looks like this is not used by any upstream 
DRM driver (or DAL) directly but only from a bunch of atomic functions, 
none of which are inline.

If this is an issue for NVidia would this also be an issue for any other 
MIT licensed code, such as drm_atomic_helper.c?

Harry

> Thanks,
> -James
>
>>> ... And atomic is pretty much guaranteed
>>> to change all the time anyway, we're definitely not going to make a
>>> stable kabi for you folks, so you might want to do that for practical
>>> reasons anyway.
>>>
>>> Just my 2cents, personal opinion, not reflecting intel's, not legal
>>> advice, yadayada and all that :-)
>>
>> Apparently coffee didn't work yet, so let me retry the more serious
>> part of my reply. I'd go with a shim that essentially remaps the linux
>> atomic to whatever cross-os datastructures and semantics you have in
>> the blob. That also has the benefit of insulating you a bit more from
>> upstream changes in atomic (which will happen), and enthusiasts might
>> get around to porting to new kernels before you do. Essentially pick
>> the architecture of amd's DAL, then fully open the glue layer. With my
>> maintainer hat on I'm at least not inclinced to add the "is this fair
>> use or not" hacks on upstream's side, simply because sooner or later
>> we'll break them and then we have the angry users, instead of nvidia.
>> And that's the wrong place for bug reports for blobs :-)
>> -Daniel
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel