| Message ID | 20200128104602.1459802-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm: Complain if drivers still use the ->load callback | expand |
Quoting Daniel Vetter (2020-01-28 10:45:58) > Kinda time to get this sorted. The locking around this really is not > nice. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/drm_drv.c | 6 ++++++ > include/drm/drm_drv.h | 3 +++ > 2 files changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 7c18a980cd4b..8deff75b484c 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -948,6 +948,12 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > > mutex_lock(&drm_global_mutex); > > + if (dev->driver->load) { > + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) > + DRM_INFO("drm driver %s is using deprecated ->load callback\n", > + dev->driver->name); DRM_WARN() if the plan is to remove it? That should encourage people to complain louder. -Chris
Quoting Chris Wilson (2020-01-28 10:47:59) > Quoting Daniel Vetter (2020-01-28 10:45:58) > > Kinda time to get this sorted. The locking around this really is not > > nice. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/gpu/drm/drm_drv.c | 6 ++++++ > > include/drm/drm_drv.h | 3 +++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > index 7c18a980cd4b..8deff75b484c 100644 > > --- a/drivers/gpu/drm/drm_drv.c > > +++ b/drivers/gpu/drm/drm_drv.c > > @@ -948,6 +948,12 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > > > > mutex_lock(&drm_global_mutex); > > > > + if (dev->driver->load) { > > + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) > > + DRM_INFO("drm driver %s is using deprecated ->load callback\n", > > + dev->driver->name); > > DRM_WARN() if the plan is to remove it? Either way though, Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
Hi Am 28.01.20 um 11:45 schrieb Daniel Vetter: > Kinda time to get this sorted. The locking around this really is not > nice. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/drm_drv.c | 6 ++++++ > include/drm/drm_drv.h | 3 +++ > 2 files changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 7c18a980cd4b..8deff75b484c 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -948,6 +948,12 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > > mutex_lock(&drm_global_mutex); > > + if (dev->driver->load) { > + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) > + DRM_INFO("drm driver %s is using deprecated ->load callback\n", > + dev->driver->name); > + } > + > ret = drm_minor_register(dev, DRM_MINOR_RENDER); > if (ret) > goto err_minors; > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 77685ed7aa65..77bc63de0a91 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -173,6 +173,9 @@ struct drm_driver { > * > * This is deprecated, do not use! > * > + * FIXME: A few non-DRIVER_LEGACY drivers still use this, and should be > + * converted. > + * I recently converted several of them. The status here is that only radeon and amdgpu still use load. I've only not been able to convert them because they do some debugfs registering that relies on the device being registered early. I've not had time to convert the code. On the patch: Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > * Returns: > * > * Zero on success, non-zero value on failure. >
On Tue, Jan 28, 2020 at 02:41:06PM +0100, Thomas Zimmermann wrote: > Hi > > Am 28.01.20 um 11:45 schrieb Daniel Vetter: > > Kinda time to get this sorted. The locking around this really is not > > nice. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/gpu/drm/drm_drv.c | 6 ++++++ > > include/drm/drm_drv.h | 3 +++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > index 7c18a980cd4b..8deff75b484c 100644 > > --- a/drivers/gpu/drm/drm_drv.c > > +++ b/drivers/gpu/drm/drm_drv.c > > @@ -948,6 +948,12 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > > > > mutex_lock(&drm_global_mutex); > > > > + if (dev->driver->load) { > > + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) > > + DRM_INFO("drm driver %s is using deprecated ->load callback\n", > > + dev->driver->name); > > + } > > + > > ret = drm_minor_register(dev, DRM_MINOR_RENDER); > > if (ret) > > goto err_minors; > > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > > index 77685ed7aa65..77bc63de0a91 100644 > > --- a/include/drm/drm_drv.h > > +++ b/include/drm/drm_drv.h > > @@ -173,6 +173,9 @@ struct drm_driver { > > * > > * This is deprecated, do not use! > > * > > + * FIXME: A few non-DRIVER_LEGACY drivers still use this, and should be > > + * converted. > > + * > > I recently converted several of them. The status here is that only > radeon and amdgpu still use load. I've only not been able to convert > them because they do some debugfs registering that relies on the device > being registered early. I've not had time to convert the code. Oh nice, this sounds great. We have an outreachy project to helpt simplify the debugfs stuff, so might be worth waiting for that. I guess I'll keep this patch meanwhile (expect if Alex or Harry ack it, they'll see the fallout if we merge this early). -Daniel > On the patch: > > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> > > > > * Returns: > > * > > * Zero on success, non-zero value on failure. > > > > -- > 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 Tue, Jan 28, 2020 at 10:47:59AM +0000, Chris Wilson wrote: > Quoting Daniel Vetter (2020-01-28 10:45:58) > > Kinda time to get this sorted. The locking around this really is not > > nice. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/gpu/drm/drm_drv.c | 6 ++++++ > > include/drm/drm_drv.h | 3 +++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > index 7c18a980cd4b..8deff75b484c 100644 > > --- a/drivers/gpu/drm/drm_drv.c > > +++ b/drivers/gpu/drm/drm_drv.c > > @@ -948,6 +948,12 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > > > > mutex_lock(&drm_global_mutex); > > > > + if (dev->driver->load) { > > + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) > > + DRM_INFO("drm driver %s is using deprecated ->load callback\n", > > + dev->driver->name); > > DRM_WARN() if the plan is to remove it? Consensus from the security check work that Kees Cook is doing seems to be: - Put new thing in place, convert lots - Start to do opt-in/informational stuff - Once you're sure it's all gone, put in the big splat that kills the box/driver. Apparently radeon/amdgpu are the hold-outs, once those are done I think I'll just outright disable ->load/unload for !DRIVER_LEGACY. Cheers, Daniel > That should encourage people to complain louder. > -Chris > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 7c18a980cd4b..8deff75b484c 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -948,6 +948,12 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) mutex_lock(&drm_global_mutex); + if (dev->driver->load) { + if (!drm_core_check_feature(dev, DRIVER_LEGACY)) + DRM_INFO("drm driver %s is using deprecated ->load callback\n", + dev->driver->name); + } + ret = drm_minor_register(dev, DRM_MINOR_RENDER); if (ret) goto err_minors; diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 77685ed7aa65..77bc63de0a91 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -173,6 +173,9 @@ struct drm_driver { * * This is deprecated, do not use! * + * FIXME: A few non-DRIVER_LEGACY drivers still use this, and should be + * converted. + * * Returns: * * Zero on success, non-zero value on failure.
Kinda time to get this sorted. The locking around this really is not nice. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/drm_drv.c | 6 ++++++ include/drm/drm_drv.h | 3 +++ 2 files changed, 9 insertions(+)