Re: Static inline DRM functions calling into GPL-only code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/11/2017 09:09 AM, Harry Wentland wrote:
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@xxxxxxxx> wrote:
On Tue, Apr 11, 2017 at 6:14 AM, Nikhil Mahale <nmahale@xxxxxxxxxx>
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?

This is our usual Linux driver download landing page:

https://www.nvidia.com/object/unix.html

We don't break out the nvidia-drm source into a separate package like we do for some of our other open-source components, but it's included when you download the full driver. You can unpack it without installing, e.g:

  $ sh ~/Downloads/NVIDIA-Linux-x86_64-378.13.run -x

Then it will be in ./NVIDIA-Linux-x86_64-378.13/kernel/nvidia-drm/

Feedback welcome.

Thanks,
-James

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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux