[PATCH] drm: do not expose vblank data before drm_vblank_init completes
On Sun, May 27, 2012 at 10:25:21PM +0200, Marcin Slusarz wrote: > On Thu, May 24, 2012 at 08:09:41PM +0200, Bruno Pr?mont wrote: > > I can easily trigger a crash in nouveau interrupt handler by unbinding > > and rebinding the GPU. > > > > The command used: > > echo $pci_device > nouveau/unbind && \ > > sleep 5 && \ > > echo $pci_device > nouveau/bind > > > > > > Kernel is 3.4.0 with modular drm/nouveau. > > GPU is NVidia nForce IGP (NV11) > > > > > > Unbinding seems to work fine, display switching back to VGA text mode. > > Rebinding the GPU slightly later causes the below trace: > > > > (...) > > It crashed because Nouveau failed to disable vblank interrupt on unbind > and this interrupt triggered while drm was initializing vblank data. > > Nouveau side was fixed by "drm/nv04/disp: disable vblank interrupts when > disabling display" by Ben Skeggs (3.5 merge window), drm side can be fixed > by this: > > --- > From: Marcin Slusarz > Subject: [PATCH] drm: do not expose vblank data before drm_vblank_init > completes > > It fixes oops in drm_handle_vblank when vblank interrupt triggers before > drm_vblank_init completes. Driver side should not let this happen, but let's > be on the safe side and handle this case. > > Reported-by: Bruno Pr?mont > Tested-by: Bruno Pr?mont > Signed-off-by: Marcin Slusarz > --- > drivers/gpu/drm/drm_irq.c | 21 + > 1 files changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index c869436..7dda18c 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -183,12 +183,8 @@ static void vblank_disable_fn(unsigned long arg) > } > } > > -void drm_vblank_cleanup(struct drm_device *dev) > +static void __drm_vblank_cleanup(struct drm_device *dev) > { > - /* Bail if the driver didn't call drm_vblank_init() */ > - if (dev->num_crtcs == 0) > - return; > - > del_timer(>vblank_disable_timer); > > vblank_disable_fn((unsigned long)dev); > @@ -201,6 +197,15 @@ void drm_vblank_cleanup(struct drm_device *dev) > kfree(dev->last_vblank_wait); > kfree(dev->vblank_inmodeset); > kfree(dev->_vblank_time); > +} > + > +void drm_vblank_cleanup(struct drm_device *dev) > +{ > + /* Bail if the driver didn't call drm_vblank_init() */ > + if (dev->num_crtcs == 0) > + return; > + > + __drm_vblank_cleanup(dev); > > dev->num_crtcs = 0; > } > @@ -215,8 +220,6 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) > spin_lock_init(>vbl_lock); > spin_lock_init(>vblank_time_lock); > > - dev->num_crtcs = num_crtcs; > - > dev->vbl_queue = kmalloc(sizeof(wait_queue_head_t) * num_crtcs, >GFP_KERNEL); > if (!dev->vbl_queue) > @@ -268,10 +271,12 @@ int drm_vblank_init(struct drm_device *dev, int > num_crtcs) > } > > dev->vblank_disable_allowed = 0; > + dev->num_crtcs = num_crtcs; > + > return 0; > > err: > - drm_vblank_cleanup(dev); > + __drm_vblank_cleanup(dev); > return ret; > } > EXPORT_SYMBOL(drm_vblank_init); > -- Dave?
Re: [PATCH] drm: do not expose vblank data before drm_vblank_init completes
On Sun, May 27, 2012 at 10:25:21PM +0200, Marcin Slusarz wrote: On Thu, May 24, 2012 at 08:09:41PM +0200, Bruno Prémont wrote: I can easily trigger a crash in nouveau interrupt handler by unbinding and rebinding the GPU. The command used: echo $pci_device nouveau/unbind \ sleep 5 \ echo $pci_device nouveau/bind Kernel is 3.4.0 with modular drm/nouveau. GPU is NVidia nForce IGP (NV11) Unbinding seems to work fine, display switching back to VGA text mode. Rebinding the GPU slightly later causes the below trace: (...) It crashed because Nouveau failed to disable vblank interrupt on unbind and this interrupt triggered while drm was initializing vblank data. Nouveau side was fixed by drm/nv04/disp: disable vblank interrupts when disabling display by Ben Skeggs (3.5 merge window), drm side can be fixed by this: --- From: Marcin Slusarz marcin.slus...@gmail.com Subject: [PATCH] drm: do not expose vblank data before drm_vblank_init completes It fixes oops in drm_handle_vblank when vblank interrupt triggers before drm_vblank_init completes. Driver side should not let this happen, but let's be on the safe side and handle this case. Reported-by: Bruno Prémont bonb...@linux-vserver.org Tested-by: Bruno Prémont bonb...@linux-vserver.org Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com --- drivers/gpu/drm/drm_irq.c | 21 + 1 files changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index c869436..7dda18c 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -183,12 +183,8 @@ static void vblank_disable_fn(unsigned long arg) } } -void drm_vblank_cleanup(struct drm_device *dev) +static void __drm_vblank_cleanup(struct drm_device *dev) { - /* Bail if the driver didn't call drm_vblank_init() */ - if (dev-num_crtcs == 0) - return; - del_timer(dev-vblank_disable_timer); vblank_disable_fn((unsigned long)dev); @@ -201,6 +197,15 @@ void drm_vblank_cleanup(struct drm_device *dev) kfree(dev-last_vblank_wait); kfree(dev-vblank_inmodeset); kfree(dev-_vblank_time); +} + +void drm_vblank_cleanup(struct drm_device *dev) +{ + /* Bail if the driver didn't call drm_vblank_init() */ + if (dev-num_crtcs == 0) + return; + + __drm_vblank_cleanup(dev); dev-num_crtcs = 0; } @@ -215,8 +220,6 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) spin_lock_init(dev-vbl_lock); spin_lock_init(dev-vblank_time_lock); - dev-num_crtcs = num_crtcs; - dev-vbl_queue = kmalloc(sizeof(wait_queue_head_t) * num_crtcs, GFP_KERNEL); if (!dev-vbl_queue) @@ -268,10 +271,12 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) } dev-vblank_disable_allowed = 0; + dev-num_crtcs = num_crtcs; + return 0; err: - drm_vblank_cleanup(dev); + __drm_vblank_cleanup(dev); return ret; } EXPORT_SYMBOL(drm_vblank_init); -- Dave? ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: do not expose vblank data before drm_vblank_init completes
On Thu, May 24, 2012 at 08:09:41PM +0200, Bruno Pr?mont wrote: > I can easily trigger a crash in nouveau interrupt handler by unbinding > and rebinding the GPU. > > The command used: > echo $pci_device > nouveau/unbind && \ > sleep 5 && \ > echo $pci_device > nouveau/bind > > > Kernel is 3.4.0 with modular drm/nouveau. > GPU is NVidia nForce IGP (NV11) > > > Unbinding seems to work fine, display switching back to VGA text mode. > Rebinding the GPU slightly later causes the below trace: > > (...) It crashed because Nouveau failed to disable vblank interrupt on unbind and this interrupt triggered while drm was initializing vblank data. Nouveau side was fixed by "drm/nv04/disp: disable vblank interrupts when disabling display" by Ben Skeggs (3.5 merge window), drm side can be fixed by this: --- From: Marcin Slusarz <marcin.slus...@gmail.com> Subject: [PATCH] drm: do not expose vblank data before drm_vblank_init completes It fixes oops in drm_handle_vblank when vblank interrupt triggers before drm_vblank_init completes. Driver side should not let this happen, but let's be on the safe side and handle this case. Reported-by: Bruno Pr?mont Tested-by: Bruno Pr?mont Signed-off-by: Marcin Slusarz --- drivers/gpu/drm/drm_irq.c | 21 + 1 files changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index c869436..7dda18c 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -183,12 +183,8 @@ static void vblank_disable_fn(unsigned long arg) } } -void drm_vblank_cleanup(struct drm_device *dev) +static void __drm_vblank_cleanup(struct drm_device *dev) { - /* Bail if the driver didn't call drm_vblank_init() */ - if (dev->num_crtcs == 0) - return; - del_timer(>vblank_disable_timer); vblank_disable_fn((unsigned long)dev); @@ -201,6 +197,15 @@ void drm_vblank_cleanup(struct drm_device *dev) kfree(dev->last_vblank_wait); kfree(dev->vblank_inmodeset); kfree(dev->_vblank_time); +} + +void drm_vblank_cleanup(struct drm_device *dev) +{ + /* Bail if the driver didn't call drm_vblank_init() */ + if (dev->num_crtcs == 0) + return; + + __drm_vblank_cleanup(dev); dev->num_crtcs = 0; } @@ -215,8 +220,6 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) spin_lock_init(>vbl_lock); spin_lock_init(>vblank_time_lock); - dev->num_crtcs = num_crtcs; - dev->vbl_queue = kmalloc(sizeof(wait_queue_head_t) * num_crtcs, GFP_KERNEL); if (!dev->vbl_queue) @@ -268,10 +271,12 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) } dev->vblank_disable_allowed = 0; + dev->num_crtcs = num_crtcs; + return 0; err: - drm_vblank_cleanup(dev); + __drm_vblank_cleanup(dev); return ret; } EXPORT_SYMBOL(drm_vblank_init); -- 1.7.8.6
[PATCH] drm: do not expose vblank data before drm_vblank_init completes
On Thu, May 24, 2012 at 08:09:41PM +0200, Bruno Prémont wrote: I can easily trigger a crash in nouveau interrupt handler by unbinding and rebinding the GPU. The command used: echo $pci_device nouveau/unbind \ sleep 5 \ echo $pci_device nouveau/bind Kernel is 3.4.0 with modular drm/nouveau. GPU is NVidia nForce IGP (NV11) Unbinding seems to work fine, display switching back to VGA text mode. Rebinding the GPU slightly later causes the below trace: (...) It crashed because Nouveau failed to disable vblank interrupt on unbind and this interrupt triggered while drm was initializing vblank data. Nouveau side was fixed by drm/nv04/disp: disable vblank interrupts when disabling display by Ben Skeggs (3.5 merge window), drm side can be fixed by this: --- From: Marcin Slusarz marcin.slus...@gmail.com Subject: [PATCH] drm: do not expose vblank data before drm_vblank_init completes It fixes oops in drm_handle_vblank when vblank interrupt triggers before drm_vblank_init completes. Driver side should not let this happen, but let's be on the safe side and handle this case. Reported-by: Bruno Prémont bonb...@linux-vserver.org Tested-by: Bruno Prémont bonb...@linux-vserver.org Signed-off-by: Marcin Slusarz marcin.slus...@gmail.com --- drivers/gpu/drm/drm_irq.c | 21 + 1 files changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index c869436..7dda18c 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -183,12 +183,8 @@ static void vblank_disable_fn(unsigned long arg) } } -void drm_vblank_cleanup(struct drm_device *dev) +static void __drm_vblank_cleanup(struct drm_device *dev) { - /* Bail if the driver didn't call drm_vblank_init() */ - if (dev-num_crtcs == 0) - return; - del_timer(dev-vblank_disable_timer); vblank_disable_fn((unsigned long)dev); @@ -201,6 +197,15 @@ void drm_vblank_cleanup(struct drm_device *dev) kfree(dev-last_vblank_wait); kfree(dev-vblank_inmodeset); kfree(dev-_vblank_time); +} + +void drm_vblank_cleanup(struct drm_device *dev) +{ + /* Bail if the driver didn't call drm_vblank_init() */ + if (dev-num_crtcs == 0) + return; + + __drm_vblank_cleanup(dev); dev-num_crtcs = 0; } @@ -215,8 +220,6 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) spin_lock_init(dev-vbl_lock); spin_lock_init(dev-vblank_time_lock); - dev-num_crtcs = num_crtcs; - dev-vbl_queue = kmalloc(sizeof(wait_queue_head_t) * num_crtcs, GFP_KERNEL); if (!dev-vbl_queue) @@ -268,10 +271,12 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) } dev-vblank_disable_allowed = 0; + dev-num_crtcs = num_crtcs; + return 0; err: - drm_vblank_cleanup(dev); + __drm_vblank_cleanup(dev); return ret; } EXPORT_SYMBOL(drm_vblank_init); -- 1.7.8.6 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel