[PATCH] drm/i915: Use drm_dev_unplug()

[PATCH] drm/i915: Use drm_dev_unplug()

[Janusz Krzysztofik]

From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>

The driver does not currently support unbinding from a device which is
in use.  Since open file descriptors may still be pointing into kernel
memory where the device structures used to be, entirely correct kernel
panics protect the driver from being unbound as we should not be
unbinding it before those dangling pointers have been made safe.

According to the documentation found inside drivers/gpu/drm/drm_drv.c,
drm_dev_unplug() should be used instead of drm_dev_unregister() in
order to make a device inaccessible to users as soon as it is unpluged.
Follow that advice to make those possibly dangling pointers safe,
protected by DRM layer from a user who is otherwise left pointing into
possibly reused kernel memory after the driver has been unbound from
the device.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9df65d386d11..66163378c481 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1596,7 +1596,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	i915_pmu_unregister(dev_priv);
 
 	i915_teardown_sysfs(dev_priv);
-	drm_dev_unregister(&dev_priv->drm);
+	drm_dev_unplug(&dev_priv->drm);
 
 	i915_gem_shrinker_unregister(dev_priv);
 }
-- 
2.20.1

Re: [PATCH] drm/i915: Use drm_dev_unplug()

[Chris Wilson]

Quoting Janusz Krzysztofik (2019-04-05 08:26:57)
> From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> 
> The driver does not currently support unbinding from a device which is
> in use.  Since open file descriptors may still be pointing into kernel
> memory where the device structures used to be, entirely correct kernel
> panics protect the driver from being unbound as we should not be
> unbinding it before those dangling pointers have been made safe.
> 
> According to the documentation found inside drivers/gpu/drm/drm_drv.c,
> drm_dev_unplug() should be used instead of drm_dev_unregister() in
> order to make a device inaccessible to users as soon as it is unpluged.
> Follow that advice to make those possibly dangling pointers safe,
> protected by DRM layer from a user who is otherwise left pointing into
> possibly reused kernel memory after the driver has been unbound from
> the device.
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9df65d386d11..66163378c481 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1596,7 +1596,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>         i915_pmu_unregister(dev_priv);
>  
>         i915_teardown_sysfs(dev_priv);
> -       drm_dev_unregister(&dev_priv->drm);
> +       drm_dev_unplug(&dev_priv->drm);

I think we may have our onion inverted here. We want to stop the users
as the first step, then start removing the entries. (That will also
nicely invert the order from register, which is what we typically
expect).

After calling i915_driver_unregister(); call i915_gem_set_wedged() to
immediately (give or take external fences) cancel inflight operations.
-Chris

Re: [PATCH] drm/i915: Use drm_dev_unplug()

[Janusz Krzysztofik]

On Fri, 2019-04-05 at 08:41 +0100, Chris Wilson wrote:
> Quoting Janusz Krzysztofik (2019-04-05 08:26:57)
> > From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> > 
> > The driver does not currently support unbinding from a device which
> > is
> > in use.  Since open file descriptors may still be pointing into
> > kernel
> > memory where the device structures used to be, entirely correct
> > kernel
> > panics protect the driver from being unbound as we should not be
> > unbinding it before those dangling pointers have been made safe.
> > 
> > According to the documentation found inside
> > drivers/gpu/drm/drm_drv.c,
> > drm_dev_unplug() should be used instead of drm_dev_unregister() in
> > order to make a device inaccessible to users as soon as it is
> > unpluged.
> > Follow that advice to make those possibly dangling pointers safe,
> > protected by DRM layer from a user who is otherwise left pointing
> > into
> > possibly reused kernel memory after the driver has been unbound
> > from
> > the device.
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 9df65d386d11..66163378c481 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1596,7 +1596,7 @@ static void i915_driver_unregister(struct
> > drm_i915_private *dev_priv)
> >         i915_pmu_unregister(dev_priv);
> >  
> >         i915_teardown_sysfs(dev_priv);
> > -       drm_dev_unregister(&dev_priv->drm);
> > +       drm_dev_unplug(&dev_priv->drm);
> 
> I think we may have our onion inverted here. We want to stop the
> users
> as the first step, then start removing the entries. (That will also
> nicely invert the order from register, which is what we typically
> expect).
> 
> After calling i915_driver_unregister(); call i915_gem_set_wedged() to
> immediately (give or take external fences) cancel inflight
> operations.

OK, thanks.  Do you prefer them squashed or as serparate patches?

Thanks,
Janusz

> -Chris
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/i915: Use drm_dev_unplug()

[Chris Wilson]

Quoting Janusz Krzysztofik (2019-04-05 09:11:54)
> On Fri, 2019-04-05 at 08:41 +0100, Chris Wilson wrote:
> > Quoting Janusz Krzysztofik (2019-04-05 08:26:57)
> > > From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> > > 
> > > The driver does not currently support unbinding from a device which
> > > is
> > > in use.  Since open file descriptors may still be pointing into
> > > kernel
> > > memory where the device structures used to be, entirely correct
> > > kernel
> > > panics protect the driver from being unbound as we should not be
> > > unbinding it before those dangling pointers have been made safe.
> > > 
> > > According to the documentation found inside
> > > drivers/gpu/drm/drm_drv.c,
> > > drm_dev_unplug() should be used instead of drm_dev_unregister() in
> > > order to make a device inaccessible to users as soon as it is
> > > unpluged.
> > > Follow that advice to make those possibly dangling pointers safe,
> > > protected by DRM layer from a user who is otherwise left pointing
> > > into
> > > possibly reused kernel memory after the driver has been unbound
> > > from
> > > the device.
> > > 
> > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index 9df65d386d11..66163378c481 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -1596,7 +1596,7 @@ static void i915_driver_unregister(struct
> > > drm_i915_private *dev_priv)
> > >         i915_pmu_unregister(dev_priv);
> > >  
> > >         i915_teardown_sysfs(dev_priv);
> > > -       drm_dev_unregister(&dev_priv->drm);
> > > +       drm_dev_unplug(&dev_priv->drm);
> > 
> > I think we may have our onion inverted here. We want to stop the
> > users
> > as the first step, then start removing the entries. (That will also
> > nicely invert the order from register, which is what we typically
> > expect).
> > 
> > After calling i915_driver_unregister(); call i915_gem_set_wedged() to
> > immediately (give or take external fences) cancel inflight
> > operations.
> 
> OK, thanks.  Do you prefer them squashed or as serparate patches?

Quite happy to do the s/unregister/unplug/ and move in one go. Have a
pre-emptive
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
on that as that seems to be the right thing to do.

And there should be no issues in placing a i915_gem_set_wedged()
immediately after the call to i915_driver_unregister, so if you include
a line of commentary about why, for example

/*
 * After unregistering the device to prevent any new users, cancel
 * all in-flight requests so that we can quickly unbind the active
 * resources.
 */
i915_gem_set_wedged(dev_priv);

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

I think overall though, we need to go through i915_driver_unload() and
push the module cleanup operations to i915_driver_release -- that will
take a bit of surgery to separate the different phases that are
currently smashed together.
-Chris

Re: [PATCH] drm/i915: Use drm_dev_unplug()

[Janusz Krzysztofik]

On Fri, 2019-04-05 at 09:24 +0100, Chris Wilson wrote:
> Quoting Janusz Krzysztofik (2019-04-05 09:11:54)
> > On Fri, 2019-04-05 at 08:41 +0100, Chris Wilson wrote:
> > > Quoting Janusz Krzysztofik (2019-04-05 08:26:57)
> > > > From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> > > > 
> > > > The driver does not currently support unbinding from a device
> > > > which
> > > > is
> > > > in use.  Since open file descriptors may still be pointing into
> > > > kernel
> > > > memory where the device structures used to be, entirely correct
> > > > kernel
> > > > panics protect the driver from being unbound as we should not
> > > > be
> > > > unbinding it before those dangling pointers have been made
> > > > safe.
> > > > 
> > > > According to the documentation found inside
> > > > drivers/gpu/drm/drm_drv.c,
> > > > drm_dev_unplug() should be used instead of drm_dev_unregister()
> > > > in
> > > > order to make a device inaccessible to users as soon as it is
> > > > unpluged.
> > > > Follow that advice to make those possibly dangling pointers
> > > > safe,
> > > > protected by DRM layer from a user who is otherwise left
> > > > pointing
> > > > into
> > > > possibly reused kernel memory after the driver has been unbound
> > > > from
> > > > the device.
> > > > 
> > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com
> > > > >
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > > b/drivers/gpu/drm/i915/i915_drv.c
> > > > index 9df65d386d11..66163378c481 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -1596,7 +1596,7 @@ static void i915_driver_unregister(struct
> > > > drm_i915_private *dev_priv)
> > > >         i915_pmu_unregister(dev_priv);
> > > >  
> > > >         i915_teardown_sysfs(dev_priv);
> > > > -       drm_dev_unregister(&dev_priv->drm);
> > > > +       drm_dev_unplug(&dev_priv->drm);
> > > 
> > > I think we may have our onion inverted here. We want to stop the
> > > users
> > > as the first step, then start removing the entries. (That will
> > > also
> > > nicely invert the order from register, which is what we typically
> > > expect).
> > > 
> > > After calling i915_driver_unregister(); call
> > > i915_gem_set_wedged() to
> > > immediately (give or take external fences) cancel inflight
> > > operations.
> > 
> > OK, thanks.  Do you prefer them squashed or as serparate patches?
> 
> Quite happy to do the s/unregister/unplug/ and move in one go. Have a
> pre-emptive
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> on that as that seems to be the right thing to do.
> 
> And there should be no issues in placing a i915_gem_set_wedged()
> immediately after the call to i915_driver_unregister, so if you
> include
> a line of commentary about why, for example
> 
> /*
>  * After unregistering the device to prevent any new users, cancel
>  * all in-flight requests so that we can quickly unbind the active
>  * resources.
>  */
> i915_gem_set_wedged(dev_priv);
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

I've given it some testing, no side effects with test workloads I've
tried, and looks like it at least helps to prevent from making the
device actually wedged.

With these two patches, plus the one we discussed yesterday, and yet
another one I'm going to submit soon, I'm now able to unbind the driver
from a device while a workload is running on it, unload the module,
reload it and successfully perform basic GEM health checks, all in a
quick succession :-).

Unfortunately, not 100% reproducible, as well as not the case with
device unplug simulated by writing 1 to device/remove sysfs file.
Surely that needs the work you describe below to be done first.

Thanks for your cooperation,
Janusz


> 
> I think overall though, we need to go through i915_driver_unload()
> and
> push the module cleanup operations to i915_driver_release -- that
> will
> take a bit of surgery to separate the different phases that are
> currently smashed together.
> -Chris
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 0/2] Stop users from using the device on driver unbind

[Janusz Krzysztofik]

Use drm_dev_unplug() to have device resources protected from user access
by DRM layer as soon as the driver is going to be unbound.  Also, cancel
all pending work so associated resources can be quickly released.

Janusz Krzysztofik (2):
  drm/i915: Use drm_dev_unplug()
  drm/i915: Mark GEM wedged right after marking device unplugged

 drivers/gpu/drm/i915/i915_drv.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

I'm resending these two patches together in series to make the robot
happy about the second one.  Also, I've added the Suggested-by: clause
to credit actual Chris' contribution.

Thanks,
Janusz
-- 
2.20.1

[PATCH 1/2] drm/i915: Use drm_dev_unplug()

[Janusz Krzysztofik]

From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>

The driver does not currently support unbinding from a device which is
in use.  Since open file descriptors may still be pointing into kernel
memory where the device structures used to be, entirely correct kernel
panics protect the driver from being unbound as we should not be
unbinding it before those dangling pointers have been made safe.

According to the documentation found inside drivers/gpu/drm/drm_drv.c,
drm_dev_unplug() should be used instead of drm_dev_unregister() in
order to make a device inaccessible to users as soon as it is unpluged.
Follow that advice to make those possibly dangling pointers safe,
protected by DRM layer from a user who is otherwise left pointing into
possibly reused kernel memory after the driver has been unbound from
the device.  Once done, also cancel inflight operations immediately by
calling i915_gem_set_wedged().

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9df65d386d11..66163378c481 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1596,7 +1596,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	i915_pmu_unregister(dev_priv);
 
 	i915_teardown_sysfs(dev_priv);
-	drm_dev_unregister(&dev_priv->drm);
+	drm_dev_unplug(&dev_priv->drm);
 
 	i915_gem_shrinker_unregister(dev_priv);
 }
-- 
2.20.1

[PATCH 2/2] drm/i915: Mark GEM wedged right after marking device unplugged

[Janusz Krzysztofik]

As soon as a device is considered unplugged, not only prevent pending
users from accessing the device structures but also cancel all their
pending requests so all consumed resources can be cleaned up as soon
as possible.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 66163378c481..03a563ce7e6b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1598,6 +1598,13 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	i915_teardown_sysfs(dev_priv);
 	drm_dev_unplug(&dev_priv->drm);
 
+	/*
+	 * After unregistering the device to prevent any new users, cancel
+	 * all in-flight requests so that we can quickly unbind the active
+	 * resources.
+	 */
+	i915_gem_set_wedged(dev_priv);
+
 	i915_gem_shrinker_unregister(dev_priv);
 }
 
-- 
2.20.1

Re: [PATCH] drm/i915: Use drm_dev_unplug()

[Daniel Vetter]

On Fri, Apr 05, 2019 at 08:41:16AM +0100, Chris Wilson wrote:
> Quoting Janusz Krzysztofik (2019-04-05 08:26:57)
> > From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> > 
> > The driver does not currently support unbinding from a device which is
> > in use.  Since open file descriptors may still be pointing into kernel
> > memory where the device structures used to be, entirely correct kernel
> > panics protect the driver from being unbound as we should not be
> > unbinding it before those dangling pointers have been made safe.
> > 
> > According to the documentation found inside drivers/gpu/drm/drm_drv.c,
> > drm_dev_unplug() should be used instead of drm_dev_unregister() in
> > order to make a device inaccessible to users as soon as it is unpluged.
> > Follow that advice to make those possibly dangling pointers safe,
> > protected by DRM layer from a user who is otherwise left pointing into
> > possibly reused kernel memory after the driver has been unbound from
> > the device.
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 9df65d386d11..66163378c481 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1596,7 +1596,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
> >         i915_pmu_unregister(dev_priv);
> >  
> >         i915_teardown_sysfs(dev_priv);
> > -       drm_dev_unregister(&dev_priv->drm);
> > +       drm_dev_unplug(&dev_priv->drm);
> 
> I think we may have our onion inverted here. We want to stop the users
> as the first step, then start removing the entries. (That will also
> nicely invert the order from register, which is what we typically
> expect).
> 
> After calling i915_driver_unregister(); call i915_gem_set_wedged() to
> immediately (give or take external fences) cancel inflight operations.

I think we still need the above patch, since drm_dev_unplug ==
drm_dev_unregister + "make sure userspace can't get at us anymore". We
could/should probably drop drm_dev_unplug and move that additional code to
drm_dev_unregister, but there's some minutea in how we refcount the
drm_device between the two. So not quite as clean a job.

There's also drm_put_dev (not to be mistaken with drm_dev_put), for added
confusion. I think ideally we'd unify all three of drm_dev_unregister,
drm_dev_unplug and drm_put_dev to one, deprecating all the others. But
that's work :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/2] drm/i915: Use drm_dev_unplug()

[Daniel Vetter]

On Fri, Apr 05, 2019 at 03:02:34PM +0200, Janusz Krzysztofik wrote:
> From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> 
> The driver does not currently support unbinding from a device which is
> in use.  Since open file descriptors may still be pointing into kernel
> memory where the device structures used to be, entirely correct kernel
> panics protect the driver from being unbound as we should not be
> unbinding it before those dangling pointers have been made safe.
> 
> According to the documentation found inside drivers/gpu/drm/drm_drv.c,
> drm_dev_unplug() should be used instead of drm_dev_unregister() in
> order to make a device inaccessible to users as soon as it is unpluged.
> Follow that advice to make those possibly dangling pointers safe,
> protected by DRM layer from a user who is otherwise left pointing into
> possibly reused kernel memory after the driver has been unbound from
> the device.  Once done, also cancel inflight operations immediately by
> calling i915_gem_set_wedged().
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9df65d386d11..66163378c481 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1596,7 +1596,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>  	i915_pmu_unregister(dev_priv);
>  
>  	i915_teardown_sysfs(dev_priv);
> -	drm_dev_unregister(&dev_priv->drm);
> +	drm_dev_unplug(&dev_priv->drm);
>  
>  	i915_gem_shrinker_unregister(dev_priv);
>  }
> -- 
> 2.20.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v2 0/1] Stop users from using the device on driver unbind

[Janusz Krzysztofik]

Use drm_dev_unplug() to have device resources protected from user access
by DRM layer as soon as the driver is going to be unbound.

Janusz Krzysztofik (1):
  drm/i915: Use drm_dev_unplug()

 drivers/gpu/drm/i915/i915_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Since this patch should be now safe for use if merged with current
drm-next or drm-tip branch which no longer suffer from incorrectly
resolved merge confilct that was breaking it, finally fixed by commit
bd53280ef042 ("drm/drv: Fix incorrect resolution of merge conflict"),
I'm resending it with Daniel's Reviewed-by: added.

Former patch 2/2 has been dropped as it is already in drm-intel-next as
commit 141f3767e7b8 ("drm/i915: Mark GEM wedged right after marking
device unplugged").  BTW, the wersion I sent was screwed up, not
reflecting Chris' intention precisely enough, but Chris was vigilant and
fixed it.  Sorry Chris.

Thanks,
Janusz
-- 
2.20.1

[PATCH v2 1/1] drm/i915: Use drm_dev_unplug()

[Janusz Krzysztofik]

From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>

The driver does not currently support unbinding from a device which is
in use.  Since open file descriptors may still be pointing into kernel
memory where the device structures used to be, entirely correct kernel
panics protect the driver from being unbound as we should not be
unbinding it before those dangling pointers have been made safe.

According to the documentation found inside drivers/gpu/drm/drm_drv.c,
drm_dev_unplug() should be used instead of drm_dev_unregister() in
order to make a device inaccessible to users as soon as it is unpluged.
Follow that advice to make those possibly dangling pointers safe,
protected by DRM layer from a user who is otherwise left pointing into
possibly reused kernel memory after the driver has been unbound from
the device.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9df65d386d11..66163378c481 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1596,7 +1596,7 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	i915_pmu_unregister(dev_priv);
 
 	i915_teardown_sysfs(dev_priv);
-	drm_dev_unregister(&dev_priv->drm);
+	drm_dev_unplug(&dev_priv->drm);
 
 	i915_gem_shrinker_unregister(dev_priv);
 }
-- 
2.20.1