[PATCH] drm: do not expose vblank data before drm_vblank_init completes

2012-06-09 Thread Marcin Slusarz
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

2012-06-08 Thread Marcin Slusarz
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

2012-05-27 Thread Marcin Slusarz
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

2012-05-27 Thread Marcin Slusarz
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