| Message ID | 1464268647-18464-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 26, 2016 at 02:17:27PM +0100, Chris Wilson wrote: > Currently debugfs files are created before the driver is even loads. > This gives the opportunity for userspace to open that interface and poke > around before the backing data structures are initialised - with the > possibility of oopsing or worse. > > Move the creation of the debugfs files to our registration phase, where > we announce our presence to the world when we are ready. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Makes sense to me. I never understood why these kinds of hooks existed in the first place. So the sequence changes from drm_dev_register() -> drm_minor_register() -> drm_debugfs_init() -> i915_debugfs_init() -> i915_driver_load() to something like drm_dev_register() -> drm_minor_register() -> drm_debugfs_init() -> i915_driver_load() -> i915_debugfs_register() Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++-- > drivers/gpu/drm/i915/i915_dma.c | 2 ++ > drivers/gpu/drm/i915/i915_drv.c | 4 ---- > drivers/gpu/drm/i915/i915_drv.h | 6 ++++-- > 4 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 615cef736356..4a3546f114c6 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -5498,8 +5498,9 @@ void intel_display_crc_init(struct drm_device *dev) > } > } > > -int i915_debugfs_init(struct drm_minor *minor) > +int i915_debugfs_register(struct drm_i915_private *dev_priv) > { > + struct drm_minor *minor = dev_priv->dev->primary; > int ret, i; > > ret = i915_forcewake_create(minor->debugfs_root, minor); > @@ -5525,8 +5526,9 @@ int i915_debugfs_init(struct drm_minor *minor) > minor->debugfs_root, minor); > } > > -void i915_debugfs_cleanup(struct drm_minor *minor) > +void i915_debugfs_unregister(struct drm_i915_private *dev_priv) > { > + struct drm_minor *minor = dev_priv->dev->primary; > int i; > > drm_debugfs_remove_files(i915_debugfs_list, > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 07edaed9d5a2..af0a6ce6f0b9 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1372,6 +1372,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) > if (intel_vgpu_active(dev_priv)) > I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY); > > + i915_debugfs_register(dev_priv); > i915_setup_sysfs(dev); > > if (INTEL_INFO(dev_priv)->num_pipes) { > @@ -1397,6 +1398,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv) > acpi_video_unregister(); > intel_opregion_unregister(dev_priv); > i915_teardown_sysfs(dev_priv->dev); > + i915_debugfs_unregister(dev_priv); > i915_gem_shrinker_cleanup(dev_priv); > } > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 943d7b222fd4..fa9e16b2247c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1761,10 +1761,6 @@ static struct drm_driver driver = { > .postclose = i915_driver_postclose, > .set_busid = drm_pci_set_busid, > > -#if defined(CONFIG_DEBUG_FS) > - .debugfs_init = i915_debugfs_init, > - .debugfs_cleanup = i915_debugfs_cleanup, > -#endif > .gem_free_object = i915_gem_free_object, > .gem_vm_ops = &i915_gem_vm_ops, > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index b9d9a4205992..8f2994ef4386 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3543,12 +3543,14 @@ int i915_verify_lists(struct drm_device *dev); > #endif > > /* i915_debugfs.c */ > -int i915_debugfs_init(struct drm_minor *minor); > -void i915_debugfs_cleanup(struct drm_minor *minor); > #ifdef CONFIG_DEBUG_FS > +int i915_debugfs_register(struct drm_i915_private *dev_priv); > +void i915_debugfs_unregister(struct drm_i915_private *dev_priv); > int i915_debugfs_connector_add(struct drm_connector *connector); > void intel_display_crc_init(struct drm_device *dev); > #else > +static inline int i915_debugfs_register(struct drm_i915_private *) {return 0;} > +static inline void i915_debugfs_unregister(struct drm_i915_private *) {} > static inline int i915_debugfs_connector_add(struct drm_connector *connector) > { return 0; } > static inline void intel_display_crc_init(struct drm_device *dev) {} > -- > 2.8.1
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 615cef736356..4a3546f114c6 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -5498,8 +5498,9 @@ void intel_display_crc_init(struct drm_device *dev) } } -int i915_debugfs_init(struct drm_minor *minor) +int i915_debugfs_register(struct drm_i915_private *dev_priv) { + struct drm_minor *minor = dev_priv->dev->primary; int ret, i; ret = i915_forcewake_create(minor->debugfs_root, minor); @@ -5525,8 +5526,9 @@ int i915_debugfs_init(struct drm_minor *minor) minor->debugfs_root, minor); } -void i915_debugfs_cleanup(struct drm_minor *minor) +void i915_debugfs_unregister(struct drm_i915_private *dev_priv) { + struct drm_minor *minor = dev_priv->dev->primary; int i; drm_debugfs_remove_files(i915_debugfs_list, diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 07edaed9d5a2..af0a6ce6f0b9 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1372,6 +1372,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) if (intel_vgpu_active(dev_priv)) I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY); + i915_debugfs_register(dev_priv); i915_setup_sysfs(dev); if (INTEL_INFO(dev_priv)->num_pipes) { @@ -1397,6 +1398,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv) acpi_video_unregister(); intel_opregion_unregister(dev_priv); i915_teardown_sysfs(dev_priv->dev); + i915_debugfs_unregister(dev_priv); i915_gem_shrinker_cleanup(dev_priv); } diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 943d7b222fd4..fa9e16b2247c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1761,10 +1761,6 @@ static struct drm_driver driver = { .postclose = i915_driver_postclose, .set_busid = drm_pci_set_busid, -#if defined(CONFIG_DEBUG_FS) - .debugfs_init = i915_debugfs_init, - .debugfs_cleanup = i915_debugfs_cleanup, -#endif .gem_free_object = i915_gem_free_object, .gem_vm_ops = &i915_gem_vm_ops, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b9d9a4205992..8f2994ef4386 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3543,12 +3543,14 @@ int i915_verify_lists(struct drm_device *dev); #endif /* i915_debugfs.c */ -int i915_debugfs_init(struct drm_minor *minor); -void i915_debugfs_cleanup(struct drm_minor *minor); #ifdef CONFIG_DEBUG_FS +int i915_debugfs_register(struct drm_i915_private *dev_priv); +void i915_debugfs_unregister(struct drm_i915_private *dev_priv); int i915_debugfs_connector_add(struct drm_connector *connector); void intel_display_crc_init(struct drm_device *dev); #else +static inline int i915_debugfs_register(struct drm_i915_private *) {return 0;} +static inline void i915_debugfs_unregister(struct drm_i915_private *) {} static inline int i915_debugfs_connector_add(struct drm_connector *connector) { return 0; } static inline void intel_display_crc_init(struct drm_device *dev) {}
Currently debugfs files are created before the driver is even loads. This gives the opportunity for userspace to open that interface and poke around before the backing data structures are initialised - with the possibility of oopsing or worse. Move the creation of the debugfs files to our registration phase, where we announce our presence to the world when we are ready. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++-- drivers/gpu/drm/i915/i915_dma.c | 2 ++ drivers/gpu/drm/i915/i915_drv.c | 4 ---- drivers/gpu/drm/i915/i915_drv.h | 6 ++++-- 4 files changed, 10 insertions(+), 8 deletions(-)