Static inline DRM functions calling into GPL-only code

[James Jones]

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).

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
>