All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/nouveau: hold mutex while syncing to kernel channel
@ 2014-01-14 15:48 Maarten Lankhorst
  2014-01-14 15:52 ` [PATCH 2/2] drm/nouveau: require cli->mutex in RING_SPACE Maarten Lankhorst
  0 siblings, 1 reply; 2+ messages in thread
From: Maarten Lankhorst @ 2014-01-14 15:48 UTC (permalink / raw)
  To: Ben Skeggs; +Cc: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org

Not holding the mutex potentially causes corruption of the kernel
channel when page flipping.

Cc: stable@vger.kernel.org #3.13
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
---
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 29c3efdfc7dd..76e3cf025c10 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -603,6 +603,14 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
  	if (!s)
  		return -ENOMEM;
  
+	if (new_bo != old_bo) {
+		ret = nouveau_bo_pin(new_bo, TTM_PL_FLAG_VRAM);
+		if (ret)
+			goto fail_free;
+	}
+
+	mutex_lock(&chan->cli->mutex);
+
  	/* synchronise rendering channel with the kernel's channel */
  	spin_lock(&new_bo->bo.bdev->fence_lock);
  	fence = nouveau_fence_ref(new_bo->bo.sync_obj);
@@ -612,13 +620,6 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
  	if (ret)
  		return ret;
  
-	if (new_bo != old_bo) {
-		ret = nouveau_bo_pin(new_bo, TTM_PL_FLAG_VRAM);
-		if (ret)
-			goto fail_free;
-	}
-
-	mutex_lock(&chan->cli->mutex);
  	ret = ttm_bo_reserve(&old_bo->bo, true, false, false, NULL);
  	if (ret)
  		goto fail_unpin;

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH 2/2] drm/nouveau: require cli->mutex in RING_SPACE
  2014-01-14 15:48 [PATCH 1/2] drm/nouveau: hold mutex while syncing to kernel channel Maarten Lankhorst
@ 2014-01-14 15:52 ` Maarten Lankhorst
  0 siblings, 0 replies; 2+ messages in thread
From: Maarten Lankhorst @ 2014-01-14 15:52 UTC (permalink / raw)
  To: Ben Skeggs; +Cc: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org

There are some cases where it's fine to not hold the cli->mutex, but in those cases we
are the only caller and we should really be able to acquire cli->mutex without blocking.

This patch will prevent future occurrences where commands are sent to a channel without
proper locking.

Signed-off-by: Maarten Lankhorst<maarten.lankhorst@canonical.com>
---
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index 900fae01793e..20959e445d05 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -153,10 +153,12 @@ nouveau_abi16_fini(struct nouveau_abi16 *abi16)
  	struct nouveau_cli *cli = (void *)abi16->client;
  	struct nouveau_abi16_chan *chan, *temp;
  
+	mutex_lock(&cli->mutex);
  	/* cleanup channels */
  	list_for_each_entry_safe(chan, temp, &abi16->channels, head) {
  		nouveau_abi16_chan_fini(abi16, chan);
  	}
+	mutex_unlock(&cli->mutex);
  
  	/* destroy the device object */
  	nouveau_object_del(abi16->client, NVDRM_CLIENT, NVDRM_DEVICE);
diff --git a/drivers/gpu/drm/nouveau/nouveau_dma.h b/drivers/gpu/drm/nouveau/nouveau_dma.h
index 984004d66a6d..82465bd0b59b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dma.h
+++ b/drivers/gpu/drm/nouveau/nouveau_dma.h
@@ -107,6 +107,7 @@ RING_SPACE(struct nouveau_channel *chan, int size)
  {
  	int ret;
  
+	lockdep_assert_held(&chan->cli->mutex);
  	ret = nouveau_dma_wait(chan, 1, size);
  	if (ret)
  		return ret;
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index f7a6568cfa6a..a60c404d59e4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -476,7 +476,9 @@ nouveau_drm_load(struct drm_device *dev, unsigned long flags)
  
  	nouveau_sysfs_init(dev);
  	nouveau_hwmon_init(dev);
+	mutex_lock(&drm->client.mutex);
  	nouveau_accel_init(drm);
+	mutex_unlock(&drm->client.mutex);
  	nouveau_fbcon_init(dev);
  
  	if (nouveau_runtime_pm != 0) {
@@ -562,17 +564,17 @@ nouveau_do_suspend(struct drm_device *dev)
  	ttm_bo_evict_mm(&drm->ttm.bdev, TTM_PL_VRAM);
  
  	NV_INFO(drm, "waiting for kernel channels to go idle...\n");
-	if (drm->cechan) {
+	ret = 0;
+	mutex_lock(&drm->client.mutex);
+	if (drm->cechan)
  		ret = nouveau_channel_idle(drm->cechan);
-		if (ret)
-			return ret;
-	}
  
-	if (drm->channel) {
+	if (drm->channel && !ret)
  		ret = nouveau_channel_idle(drm->channel);
-		if (ret)
-			return ret;
-	}
+	mutex_unlock(&drm->client.mutex);
+	if (ret)
+		return ret;
+
  
  	NV_INFO(drm, "suspending client object trees...\n");
  	if (drm->fence && nouveau_fence(drm)->suspend) {
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 7903e0ed3c75..aec87e1d3855 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -357,7 +357,7 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
  
  	mutex_unlock(&dev->struct_mutex);
  
-	if (chan) {
+	if (chan && !WARN_ON(!mutex_trylock(&drm->client.mutex))) {
  		ret = -ENODEV;
  		if (device->card_type < NV_50)
  			ret = nv04_fbcon_accel_init(info);
@@ -366,6 +366,7 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
  			ret = nv50_fbcon_accel_init(info);
  		else
  			ret = nvc0_fbcon_accel_init(info);
+		mutex_unlock(&drm->client.mutex);
  
  		if (ret == 0)
  			info->fbops = &nouveau_fbcon_ops;

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-01-14 15:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-14 15:48 [PATCH 1/2] drm/nouveau: hold mutex while syncing to kernel channel Maarten Lankhorst
2014-01-14 15:52 ` [PATCH 2/2] drm/nouveau: require cli->mutex in RING_SPACE Maarten Lankhorst

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.