gpu: nvgpu: Add synchronization to comptag alloc and clearing

Comptags allocation and clearing was not synchronized for a
buffer. Fix this race by serializing the operations with the
gk20a_dmabuf_priv lock. While doing that, add an error check in
the cbc_ctrl call.

Bug 1902982

Change-Id: Icd96f1855eb5e5340651bcc85849b5ccc199b821
Signed-off-by: Sami Kiminki <skiminki@nvidia.com>
Reviewed-on: https://git-master.nvidia.com/r/1597904
Reviewed-by: Alex Waterman <alexw@nvidia.com>
Reviewed-by: Terje Bergstrom <tbergstrom@nvidia.com>
Reviewed-by: mobile promotions <svcmobile_promotions@nvidia.com>
Tested-by: mobile promotions <svcmobile_promotions@nvidia.com>

3 files changed, 80 insertions, 17 deletions

diff --git a/drivers/gpu/nvgpu/common/linux/comptags.c b/drivers/gpu/nvgpu/common/linux/comptags.c
index 7d095793..353f6363 100644
--- a/drivers/gpu/nvgpu/common/linux/comptags.c
+++ b/drivers/gpu/nvgpu/common/linux/comptags.c
@@ -37,7 +37,9 @@ void gk20a_get_comptags(struct nvgpu_os_buffer *buf,
                 return;
         }
 
+        nvgpu_mutex_acquire(&priv->lock);
         *comptags = priv->comptags;
+        nvgpu_mutex_release(&priv->lock);
 }
 
 int gk20a_alloc_or_get_comptags(struct gk20a *g,
@@ -55,20 +57,26 @@ int gk20a_alloc_or_get_comptags(struct gk20a *g,
         if (!priv)
                 return -ENOSYS;
 
+        nvgpu_mutex_acquire(&priv->lock);
+
         if (priv->comptags.allocated) {
                 /*
                  * already allocated
                  */
                 *comptags = priv->comptags;
-                return 0;
+
+                err = 0;
+                goto exit_locked;
         }
 
         ctag_granularity = g->ops.fb.compression_page_size(g);
         lines = DIV_ROUND_UP_ULL(buf->dmabuf->size, ctag_granularity);
 
         /* 0-sized buffer? Shouldn't occur, but let's check anyways. */
-        if (lines < 1)
-                return -EINVAL;
+        if (lines < 1) {
+                err = -EINVAL;
+                goto exit_locked;
+        }
 
         /* store the allocator so we can use it when we free the ctags */
         priv->comptag_allocator = allocator;
@@ -89,16 +97,44 @@ int gk20a_alloc_or_get_comptags(struct gk20a *g,
          * would not be safe to re-allocate comptags anyways on
          * successive calls, as that would break map aliasing.
          */
+        err = 0;
         priv->comptags.allocated = true;
 
         *comptags = priv->comptags;
-        return 0;
+
+exit_locked:
+        nvgpu_mutex_release(&priv->lock);
+
+        return err;
 }
 
-void gk20a_mark_comptags_cleared(struct nvgpu_os_buffer *buf)
+bool gk20a_comptags_start_clear(struct nvgpu_os_buffer *buf)
 {
         struct gk20a_dmabuf_priv *priv = dma_buf_get_drvdata(buf->dmabuf,
                                                              buf->dev);
-        if (priv)
-                priv->comptags.needs_clear = false;
+        bool clear_started = false;
+
+        if (priv) {
+                nvgpu_mutex_acquire(&priv->lock);
+
+                clear_started = priv->comptags.needs_clear;
+
+                if (!clear_started)
+                        nvgpu_mutex_release(&priv->lock);
+        }
+
+        return clear_started;
+}
+
+void gk20a_comptags_finish_clear(struct nvgpu_os_buffer *buf,
+                                 bool clear_successful)
+{
+        struct gk20a_dmabuf_priv *priv = dma_buf_get_drvdata(buf->dmabuf,
+                                                             buf->dev);
+        if (priv) {
+                if (clear_successful)
+                        priv->comptags.needs_clear = false;
+
+                nvgpu_mutex_release(&priv->lock);
+        }
 }
diff --git a/drivers/gpu/nvgpu/common/mm/vm.c b/drivers/gpu/nvgpu/common/mm/vm.c
index c3f6b79d..ebe8e381 100644
--- a/drivers/gpu/nvgpu/common/mm/vm.c
+++ b/drivers/gpu/nvgpu/common/mm/vm.c
@@ -874,12 +874,17 @@ struct nvgpu_mapped_buf *nvgpu_vm_map(struct vm_gk20a *vm,
                  */
                 if (comptags.needs_clear) {
                         if (g->ops.ltc.cbc_ctrl) {
-                                g->ops.ltc.cbc_ctrl(
-                                        g, gk20a_cbc_op_clear,
-                                        comptags.offset,
-                                        (comptags.offset +
-                                         comptags.lines - 1));
-                                gk20a_mark_comptags_cleared(os_buf);
+                                if (gk20a_comptags_start_clear(os_buf)) {
+                                        err = g->ops.ltc.cbc_ctrl(
+                                                g, gk20a_cbc_op_clear,
+                                                comptags.offset,
+                                                (comptags.offset +
+                                                 comptags.lines - 1));
+                                        gk20a_comptags_finish_clear(
+                                                os_buf, err == 0);
+                                        if (err)
+                                                goto clean_up;
+                                }
                         } else {
                                 /*
                                  * Cleared as part of gmmu map
@@ -920,6 +925,9 @@ struct nvgpu_mapped_buf *nvgpu_vm_map(struct vm_gk20a *vm,
                 goto clean_up;
         }
 
+        if (clear_ctags)
+                clear_ctags = gk20a_comptags_start_clear(os_buf);
+
         map_addr = g->ops.mm.gmmu_map(vm,
                                       map_addr,
                                       sgt,
@@ -935,14 +943,15 @@ struct nvgpu_mapped_buf *nvgpu_vm_map(struct vm_gk20a *vm,
                                       false,
                                       batch,
                                       aperture);
+
+        if (clear_ctags)
+                gk20a_comptags_finish_clear(os_buf, map_addr != 0);
+
         if (!map_addr) {
                 err = -ENOMEM;
                 goto clean_up;
         }
 
-        if (clear_ctags)
-                gk20a_mark_comptags_cleared(os_buf);
-
         nvgpu_init_list_node(&mapped_buffer->buffer_list);
         nvgpu_ref_init(&mapped_buffer->ref);
         mapped_buffer->addr         = map_addr;
diff --git a/drivers/gpu/nvgpu/include/nvgpu/comptags.h b/drivers/gpu/nvgpu/include/nvgpu/comptags.h
index 99e2d657..679a0f9e 100644
--- a/drivers/gpu/nvgpu/include/nvgpu/comptags.h
+++ b/drivers/gpu/nvgpu/include/nvgpu/comptags.h
@@ -74,6 +74,24 @@ int gk20a_alloc_or_get_comptags(struct gk20a *g,
                                 struct gk20a_comptags *comptags);
 void gk20a_get_comptags(struct nvgpu_os_buffer *buf,
                         struct gk20a_comptags *comptags);
-void gk20a_mark_comptags_cleared(struct nvgpu_os_buffer *buf);
+
+/*
+ * These functions must be used to synchronize comptags clear. The usage:
+ *
+ *   if (gk20a_comptags_start_clear(os_buf)) {
+ *           // we now hold the buffer lock for clearing
+ *
+ *           bool successful = hw_clear_comptags();
+ *
+ *           // mark the buf cleared (or not) and release the buffer lock
+ *           gk20a_comptags_finish_clear(os_buf, successful);
+ *   }
+ *
+ *  If gk20a_start_comptags_clear() returns false, another caller has
+ *  already cleared the comptags.
+ */
+bool gk20a_comptags_start_clear(struct nvgpu_os_buffer *buf);
+void gk20a_comptags_finish_clear(struct nvgpu_os_buffer *buf,
+                                 bool clear_successful);
 
 #endif