| Message ID | 20210121112919.1460322-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Update todo.rst | expand |
Hi Daniel, On Thu, Jan 21, 2021 at 12:29:19PM +0100, Daniel Vetter wrote: > Interrnship season is starting, let's review this. One thing that's ^ internship > pending is Maxime's work to roll out drm_atomic_state pointers to all > callbacks, he said he'll remove that entry once it's all done. I plan on sending it by the end of the week > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > --- > Documentation/gpu/todo.rst | 28 +++++++++++++++------------- > 1 file changed, 15 insertions(+), 13 deletions(-) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 009d8e6c7e3c..492768dd2fd9 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -577,20 +577,24 @@ Contact: Daniel Vetter > > Level: Intermediate > > -KMS cleanups > ------------- > +Object lifetime fixes > +--------------------- > + > +There's two related issues here > + > +- Cleanup up the various ->destroy callbacks, which often are all the same > + simple code. > > -Some of these date from the very introduction of KMS in 2008 ... > +- Lots of drivers erroneously allocate DRM modeset objects using devm_kzalloc, > + which results in use-after free issues on driver unload. This can be serious > + trouble even for drivers for hardwared integrated on the SoC due to ^ hardware? > + EPROBE_DEFERRED backoff. Thanks! Maxime
Hi we talked about making dma_resv the default lock for GEM objects. Could you add an entry for this? Some interns might feel adventurous. :) Best regards Thomas Am 21.01.21 um 12:29 schrieb Daniel Vetter: > Interrnship season is starting, let's review this. One thing that's > pending is Maxime's work to roll out drm_atomic_state pointers to all > callbacks, he said he'll remove that entry once it's all done. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > --- > Documentation/gpu/todo.rst | 28 +++++++++++++++------------- > 1 file changed, 15 insertions(+), 13 deletions(-) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 009d8e6c7e3c..492768dd2fd9 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -577,20 +577,24 @@ Contact: Daniel Vetter > > Level: Intermediate > > -KMS cleanups > ------------- > +Object lifetime fixes > +--------------------- > + > +There's two related issues here > + > +- Cleanup up the various ->destroy callbacks, which often are all the same > + simple code. > > -Some of these date from the very introduction of KMS in 2008 ... > +- Lots of drivers erroneously allocate DRM modeset objects using devm_kzalloc, > + which results in use-after free issues on driver unload. This can be serious > + trouble even for drivers for hardwared integrated on the SoC due to > + EPROBE_DEFERRED backoff. > > -- Make ->funcs and ->helper_private vtables optional. There's a bunch of empty > - function tables in drivers, but before we can remove them we need to make sure > - that all the users in helpers and drivers do correctly check for a NULL > - vtable. > +Both these problems can be solved by switching over to drmm_kzalloc(), and the > +various convenience wrappers provided, e.g. drmm_crtc_alloc_with_planes(), > +drmm_universal_plane_alloc(), ... and so on. > > -- Cleanup up the various ->destroy callbacks. A lot of them just wrapt the > - drm_*_cleanup implementations and can be removed. Some tack a kfree() at the > - end, for which we could add drm_*_cleanup_kfree(). And then there's the (for > - historical reasons) misnamed drm_primary_helper_destroy() function. > +Contact: Daniel Vetter > > Level: Intermediate > > @@ -626,8 +630,6 @@ See the documentation of :ref:`VKMS <vkms>` for more details. This is an ideal > internship task, since it only requires a virtual machine and can be sized to > fit the available time. > > -Contact: Daniel Vetter > - > Level: See details > > Backlight Refactoring >
On Thu, Jan 21, 2021 at 3:31 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Hi > > we talked about making dma_resv the default lock for GEM objects. Could > you add an entry for this? Some interns might feel adventurous. :) Level: Too hard for Daniel Not sure that's a great internship tasks :-P But yeah I'll try to type up something around this maybe. -Daniel > > Best regards > Thomas > > Am 21.01.21 um 12:29 schrieb Daniel Vetter: > > Interrnship season is starting, let's review this. One thing that's > > pending is Maxime's work to roll out drm_atomic_state pointers to all > > callbacks, he said he'll remove that entry once it's all done. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Maxime Ripard <mripard@kernel.org> > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > Cc: David Airlie <airlied@linux.ie> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > --- > > Documentation/gpu/todo.rst | 28 +++++++++++++++------------- > > 1 file changed, 15 insertions(+), 13 deletions(-) > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > index 009d8e6c7e3c..492768dd2fd9 100644 > > --- a/Documentation/gpu/todo.rst > > +++ b/Documentation/gpu/todo.rst > > @@ -577,20 +577,24 @@ Contact: Daniel Vetter > > > > Level: Intermediate > > > > -KMS cleanups > > ------------- > > +Object lifetime fixes > > +--------------------- > > + > > +There's two related issues here > > + > > +- Cleanup up the various ->destroy callbacks, which often are all the same > > + simple code. > > > > -Some of these date from the very introduction of KMS in 2008 ... > > +- Lots of drivers erroneously allocate DRM modeset objects using devm_kzalloc, > > + which results in use-after free issues on driver unload. This can be serious > > + trouble even for drivers for hardwared integrated on the SoC due to > > + EPROBE_DEFERRED backoff. > > > > -- Make ->funcs and ->helper_private vtables optional. There's a bunch of empty > > - function tables in drivers, but before we can remove them we need to make sure > > - that all the users in helpers and drivers do correctly check for a NULL > > - vtable. > > +Both these problems can be solved by switching over to drmm_kzalloc(), and the > > +various convenience wrappers provided, e.g. drmm_crtc_alloc_with_planes(), > > +drmm_universal_plane_alloc(), ... and so on. > > > > -- Cleanup up the various ->destroy callbacks. A lot of them just wrapt the > > - drm_*_cleanup implementations and can be removed. Some tack a kfree() at the > > - end, for which we could add drm_*_cleanup_kfree(). And then there's the (for > > - historical reasons) misnamed drm_primary_helper_destroy() function. > > +Contact: Daniel Vetter > > > > Level: Intermediate > > > > @@ -626,8 +630,6 @@ See the documentation of :ref:`VKMS <vkms>` for more details. This is an ideal > > internship task, since it only requires a virtual machine and can be sized to > > fit the available time. > > > > -Contact: Daniel Vetter > > - > > Level: See details > > > > Backlight Refactoring > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer >
On Thu, Jan 21, 2021 at 02:21:53PM +0100, Maxime Ripard wrote: > Hi Daniel, > > On Thu, Jan 21, 2021 at 12:29:19PM +0100, Daniel Vetter wrote: > > Interrnship season is starting, let's review this. One thing that's > > ^ internship > > > pending is Maxime's work to roll out drm_atomic_state pointers to all > > callbacks, he said he'll remove that entry once it's all done. > > I plan on sending it by the end of the week Typos fixed and pushed with your irc-ack. -Daniel > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Maxime Ripard <mripard@kernel.org> > > Cc: Thomas Zimmermann <tzimmermann@suse.de> > > Cc: David Airlie <airlied@linux.ie> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > --- > > Documentation/gpu/todo.rst | 28 +++++++++++++++------------- > > 1 file changed, 15 insertions(+), 13 deletions(-) > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > index 009d8e6c7e3c..492768dd2fd9 100644 > > --- a/Documentation/gpu/todo.rst > > +++ b/Documentation/gpu/todo.rst > > @@ -577,20 +577,24 @@ Contact: Daniel Vetter > > > > Level: Intermediate > > > > -KMS cleanups > > ------------- > > +Object lifetime fixes > > +--------------------- > > + > > +There's two related issues here > > + > > +- Cleanup up the various ->destroy callbacks, which often are all the same > > + simple code. > > > > -Some of these date from the very introduction of KMS in 2008 ... > > +- Lots of drivers erroneously allocate DRM modeset objects using devm_kzalloc, > > + which results in use-after free issues on driver unload. This can be serious > > + trouble even for drivers for hardwared integrated on the SoC due to > > ^ hardware? > > > + EPROBE_DEFERRED backoff. > > Thanks! > Maxime
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 009d8e6c7e3c..492768dd2fd9 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -577,20 +577,24 @@ Contact: Daniel Vetter Level: Intermediate -KMS cleanups ------------- +Object lifetime fixes +--------------------- + +There's two related issues here + +- Cleanup up the various ->destroy callbacks, which often are all the same + simple code. -Some of these date from the very introduction of KMS in 2008 ... +- Lots of drivers erroneously allocate DRM modeset objects using devm_kzalloc, + which results in use-after free issues on driver unload. This can be serious + trouble even for drivers for hardwared integrated on the SoC due to + EPROBE_DEFERRED backoff. -- Make ->funcs and ->helper_private vtables optional. There's a bunch of empty - function tables in drivers, but before we can remove them we need to make sure - that all the users in helpers and drivers do correctly check for a NULL - vtable. +Both these problems can be solved by switching over to drmm_kzalloc(), and the +various convenience wrappers provided, e.g. drmm_crtc_alloc_with_planes(), +drmm_universal_plane_alloc(), ... and so on. -- Cleanup up the various ->destroy callbacks. A lot of them just wrapt the - drm_*_cleanup implementations and can be removed. Some tack a kfree() at the - end, for which we could add drm_*_cleanup_kfree(). And then there's the (for - historical reasons) misnamed drm_primary_helper_destroy() function. +Contact: Daniel Vetter Level: Intermediate @@ -626,8 +630,6 @@ See the documentation of :ref:`VKMS <vkms>` for more details. This is an ideal internship task, since it only requires a virtual machine and can be sized to fit the available time. -Contact: Daniel Vetter - Level: See details Backlight Refactoring
Interrnship season is starting, let's review this. One thing that's pending is Maxime's work to roll out drm_atomic_state pointers to all callbacks, he said he'll remove that entry once it's all done. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> --- Documentation/gpu/todo.rst | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)