drm/nouveau: fix a race condition in nouveau_dma_wait()

Can be triggered easily on certain cards (NV46 and NV50 of mine) by
running "dmesg", the DRM's channel will lockup.

Signed-off-by: Ben Skeggs <bskeggs@redhat.com>

1 files changed, 47 insertions, 29 deletions

diff --git a/drivers/gpu/drm/nouveau/nouveau_dma.c b/drivers/gpu/drm/nouveau/nouveau_dma.c
index 7afbe8b40d51..50d9e67745af 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dma.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dma.c
@@ -126,47 +126,52 @@ OUT_RINGp(struct nouveau_channel *chan, const void *data, unsigned nr_dwords)
         chan->dma.cur += nr_dwords;
 }
 
-static inline bool
-READ_GET(struct nouveau_channel *chan, uint32_t *get)
+/* Fetch and adjust GPU GET pointer
+ *
+ * Returns:
+ *  value >= 0, the adjusted GET pointer
+ *  -EINVAL if GET pointer currently outside main push buffer
+ *  -EBUSY if timeout exceeded
+ */
+static inline int
+READ_GET(struct nouveau_channel *chan, uint32_t *prev_get, uint32_t *timeout)
 {
         uint32_t val;
 
         val = nvchan_rd32(chan, chan->user_get);
-        if (val < chan->pushbuf_base ||
-            val > chan->pushbuf_base + (chan->dma.max << 2)) {
-                /* meaningless to dma_wait() except to know whether the
-                 * GPU has stalled or not
-                 */
-                *get = val;
-                return false;
+
+        /* reset counter as long as GET is still advancing, this is
+         * to avoid misdetecting a GPU lockup if the GPU happens to
+         * just be processing an operation that takes a long time
+         */
+        if (val != *prev_get) {
+                *prev_get = val;
+                *timeout = 0;
+        }
+
+        if ((++*timeout & 0xff) == 0) {
+                DRM_UDELAY(1);
+                if (*timeout > 100000)
+                        return -EBUSY;
         }
 
-        *get = (val - chan->pushbuf_base) >> 2;
-        return true;
+        if (val < chan->pushbuf_base ||
+            val > chan->pushbuf_base + (chan->dma.max << 2))
+                return -EINVAL;
+
+        return (val - chan->pushbuf_base) >> 2;
 }
 
 int
 nouveau_dma_wait(struct nouveau_channel *chan, int size)
 {
-        uint32_t get, prev_get = 0, cnt = 0;
-        bool get_valid;
+        uint32_t prev_get = 0, cnt = 0;
+        int get;
 
         while (chan->dma.free < size) {
-                /* reset counter as long as GET is still advancing, this is
-                 * to avoid misdetecting a GPU lockup if the GPU happens to
-                 * just be processing an operation that takes a long time
-                 */
-                get_valid = READ_GET(chan, &get);
-                if (get != prev_get) {
-                        prev_get = get;
-                        cnt = 0;
-                }
-
-                if ((++cnt & 0xff) == 0) {
-                        DRM_UDELAY(1);
-                        if (cnt > 100000)
-                                return -EBUSY;
-                }
+                get = READ_GET(chan, &prev_get, &cnt);
+                if (unlikely(get == -EBUSY))
+                        return -EBUSY;
 
                 /* loop until we have a usable GET pointer.  the value
                  * we read from the GPU may be outside the main ring if
@@ -177,7 +182,7 @@ nouveau_dma_wait(struct nouveau_channel *chan, int size)
                  * from the SKIPS area, so the code below doesn't have to deal
                  * with some fun corner cases.
                  */
-                if (!get_valid || get < NOUVEAU_DMA_SKIPS)
+                if (unlikely(get == -EINVAL) || get < NOUVEAU_DMA_SKIPS)
                         continue;
 
                 if (get <= chan->dma.cur) {
@@ -203,6 +208,19 @@ nouveau_dma_wait(struct nouveau_channel *chan, int size)
                          * after processing the currently pending commands.
                          */
                         OUT_RING(chan, chan->pushbuf_base | 0x20000000);
+
+                        /* wait for GET to depart from the skips area.
+                         * prevents writing GET==PUT and causing a race
+                         * condition that causes us to think the GPU is
+                         * idle when it's not.
+                         */
+                        do {
+                                get = READ_GET(chan, &prev_get, &cnt);
+                                if (unlikely(get == -EBUSY))
+                                        return -EBUSY;
+                                if (unlikely(get == -EINVAL))
+                                        continue;
+                        } while (get <= NOUVEAU_DMA_SKIPS);
                         WRITE_PUT(NOUVEAU_DMA_SKIPS);
 
                         /* we're now submitting commands at the start of