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.slusarz@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 bonbons@linux-vserver.org Tested-by: Bruno Prémont bonbons@linux-vserver.org Signed-off-by: Marcin Slusarz marcin.slusarz@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);