[PATCH 0/2] drm/msm: fix dma-buf sharing on targets without per-process pgtables

[PATCH 0/2] drm/msm: fix dma-buf sharing on targets without per-process pgtables

[Dmitry Baryshkov]

Targets without per-process pgtables -- everything pre-a6xx, plus a6xx
parts where split pagetables are unavailable -- hand every DRM file the
GPU's global VM.  The rest of the driver still treats a context VM as
private to that context, so closing a handle tears down mappings other
files are still using, and frees the iova for immediate reuse.

A dma-buf shared between two clients therefore keeps sampling an address
that the next allocation has taken over.  On a530 this is every
ext_image_dma_buf_import sampling test, reading back all zeros.

A VM shared between contexts needs the lazy, refcounted teardown the
driver already implements for kms->vm.  Rather than adding a second
mechanism, the first patch makes that machinery reusable and the second
one puts the shared GPU VM behind it.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
Dmitry Baryshkov (2):
      drm/msm: factor out a locking put_iova_spaces() wrapper
      drm/msm: don't tear down shared VM mappings on handle close

 drivers/gpu/drm/msm/msm_gem.c | 45 ++++++++++++++++++++++++++++---------------
 drivers/gpu/drm/msm/msm_gpu.c |  4 +++-
 drivers/gpu/drm/msm/msm_gpu.h |  3 +++
 3 files changed, 36 insertions(+), 16 deletions(-)
---
base-commit: 6b8c8af514d739d0335f5579b585e02babe8a727
change-id: 20260822-msm-fix-export-c4a4cd9239fc

Best regards,
--  
With best wishes
Dmitry

[PATCH 1/2] drm/msm: factor out a locking put_iova_spaces() wrapper

[Dmitry Baryshkov]

put_iova_spaces() asserts that the caller already holds the VM and object
locks, but is not named accordingly.  Both of the callers which do not
hold those locks yet -- msm_gem_close() and msm_gem_vma_put() -- open-code
the same lock, tear down, unlock sequence around it.

Rename it to put_iova_spaces_locked() and give the plain name to a wrapper
taking both locks, mirroring the get_and_pin_iova_range_locked() /
msm_gem_get_and_pin_iova_range() pair in the same file.  No functional
change.

Assisted-by: LLM
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_gem.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index c4cff3d53d81..f90afffe3442 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -47,13 +47,23 @@ static int msm_gem_open(struct drm_gem_object *obj, struct drm_file *file)
 	return 0;
 }
 
+static void put_iova_spaces_locked(struct drm_gem_object *obj,
+				   struct drm_gpuvm *vm, bool close,
+				   const char *reason);
+
 static void put_iova_spaces(struct drm_gem_object *obj, struct drm_gpuvm *vm,
-			    bool close, const char *reason);
+			    bool close, const char *reason)
+{
+	struct drm_exec exec;
+
+	msm_gem_lock_vm_and_obj(&exec, obj, vm);
+	put_iova_spaces_locked(obj, vm, close, reason);
+	drm_exec_fini(&exec);     /* drop locks */
+}
 
 static void msm_gem_close(struct drm_gem_object *obj, struct drm_file *file)
 {
 	struct msm_context *ctx = file->driver_priv;
-	struct drm_exec exec;
 
 	update_ctx_mem(file, -obj->size);
 	msm_gem_vma_put(obj);
@@ -81,9 +91,7 @@ static void msm_gem_close(struct drm_gem_object *obj, struct drm_file *file)
 	dma_resv_wait_timeout(obj->resv, DMA_RESV_USAGE_BOOKKEEP, false,
 			      MAX_SCHEDULE_TIMEOUT);
 
-	msm_gem_lock_vm_and_obj(&exec, obj, ctx->vm);
 	put_iova_spaces(obj, ctx->vm, true, "close");
-	drm_exec_fini(&exec);     /* drop locks */
 }
 
 /*
@@ -106,11 +114,7 @@ void msm_gem_vma_put(struct drm_gem_object *obj)
 		return;
 
 #ifdef CONFIG_DRM_MSM_KMS
-	struct drm_exec exec;
-
-	msm_gem_lock_vm_and_obj(&exec, obj, priv->kms->vm);
 	put_iova_spaces(obj, priv->kms->vm, true, "vma_put");
-	drm_exec_fini(&exec);     /* drop locks */
 #endif
 }
 
@@ -409,8 +413,8 @@ static struct drm_gpuva *lookup_vma(struct drm_gem_object *obj,
  * mapping.
  */
 static void
-put_iova_spaces(struct drm_gem_object *obj, struct drm_gpuvm *vm,
-		bool close, const char *reason)
+put_iova_spaces_locked(struct drm_gem_object *obj, struct drm_gpuvm *vm,
+		       bool close, const char *reason)
 {
 	struct drm_gpuvm_bo *vm_bo, *tmp;
 
@@ -669,7 +673,7 @@ void msm_gem_unpin_iova(struct drm_gem_object *obj, struct drm_gpuvm *vm)
 		msm_gem_unpin_locked(obj);
 	}
 	if (!is_kms_vm(vm))
-		put_iova_spaces(obj, vm, true, "close");
+		put_iova_spaces_locked(obj, vm, true, "close");
 	drm_exec_fini(&exec);     /* drop locks */
 }
 
@@ -831,7 +835,7 @@ void msm_gem_purge(struct drm_gem_object *obj)
 	GEM_WARN_ON(!is_purgeable(msm_obj));
 
 	/* Get rid of any iommu mapping(s): */
-	put_iova_spaces(obj, NULL, false, "purge");
+	put_iova_spaces_locked(obj, NULL, false, "purge");
 
 	msm_gem_vunmap(obj);
 
@@ -869,7 +873,7 @@ void msm_gem_evict(struct drm_gem_object *obj)
 	GEM_WARN_ON(is_unevictable(msm_obj));
 
 	/* Get rid of any iommu mapping(s): */
-	put_iova_spaces(obj, NULL, false, "evict");
+	put_iova_spaces_locked(obj, NULL, false, "evict");
 
 	drm_vma_node_unmap(&obj->vma_node, dev->anon_inode->i_mapping);
 
@@ -1082,7 +1086,7 @@ static void msm_gem_free_object(struct drm_gem_object *obj)
 				drm_exec_retry_on_contention(&exec);
 			}
 		}
-		put_iova_spaces(obj, NULL, true, "free");
+		put_iova_spaces_locked(obj, NULL, true, "free");
 		drm_exec_fini(&exec);     /* drop locks */
 	}
 

-- 
2.47.3

[PATCH 2/2] drm/msm: don't tear down shared VM mappings on handle close

[Dmitry Baryshkov]

On targets (like A530 / MSM8996) without per-process pgtables
msm_gpu_create_private_vm() uses the global VM, so all DRM files share
one GPU address space.  msm_gem_close() unmaps the object from ctx->vm,
which on those targets pulls the buffer out from under every other file
that still has it open, and frees the iova for immediate reuse.

A dma-buf imported into a second file hits this as soon as the exporter
closes its handle: the importer's texture keeps sampling the old
address, which the next allocation has taken over.  On a530 this is
every ext_image_dma_buf_import sampling test, reading back all zeros.

The VMA teardown a shared VM needs is the one already used for kms->vm
-- defer it to the @vma_ref drop, when the last handle and dma_buf
reference are gone.  That restores the pre-drm_gpuvm lifetime without
reintroducing the reference loop, since a BO with a live vma_ref is held
by userspace anyway.

Fixes: 111fdd2198e6 ("drm/msm: drm_gpuvm conversion")
Assisted-by: LLM
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_gem.c | 13 ++++++++++++-
 drivers/gpu/drm/msm/msm_gpu.c |  4 +++-
 drivers/gpu/drm/msm/msm_gpu.h |  3 +++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index f90afffe3442..e73cf49c360e 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -63,6 +63,7 @@ static void put_iova_spaces(struct drm_gem_object *obj, struct drm_gpuvm *vm,
 
 static void msm_gem_close(struct drm_gem_object *obj, struct drm_file *file)
 {
+	struct msm_drm_private *priv = obj->dev->dev_private;
 	struct msm_context *ctx = file->driver_priv;
 
 	update_ctx_mem(file, -obj->size);
@@ -84,6 +85,10 @@ static void msm_gem_close(struct drm_gem_object *obj, struct drm_file *file)
 	if (msm_context_is_vmbind(ctx))
 		return;
 
+	/* A global VM's VMAs are torn down by the @vma_ref drop above */
+	if (priv->gpu && ctx->vm == priv->gpu->vm)
+		return;
+
 	/*
 	 * TODO we might need to kick this to a queue to avoid blocking
 	 * in CLOSE ioctl
@@ -95,7 +100,7 @@ static void msm_gem_close(struct drm_gem_object *obj, struct drm_file *file)
 }
 
 /*
- * Get/put for kms->vm VMA
+ * Get/put for VMAs in VMs shared between contexts: kms->vm, gpu->vm
  */
 
 void msm_gem_vma_get(struct drm_gem_object *obj)
@@ -110,6 +115,12 @@ void msm_gem_vma_put(struct drm_gem_object *obj)
 	if (atomic_dec_return(&to_msm_bo(obj)->vma_ref))
 		return;
 
+	if (priv->gpu && priv->gpu->vm_shared) {
+		dma_resv_wait_timeout(obj->resv, DMA_RESV_USAGE_BOOKKEEP, false,
+				      MAX_SCHEDULE_TIMEOUT);
+		put_iova_spaces(obj, priv->gpu->vm, true, "vma_put");
+	}
+
 	if (!priv->kms)
 		return;
 
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 0c2c35636251..31d84e2b123a 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -879,8 +879,10 @@ msm_gpu_create_private_vm(struct msm_gpu *gpu, struct task_struct *task,
 			to_msm_vm(vm)->pid = get_pid(task_pid(task));
 	}
 
-	if (IS_ERR_OR_NULL(vm) && kernel_managed)
+	if (IS_ERR_OR_NULL(vm) && kernel_managed) {
 		vm = drm_gpuvm_get(gpu->vm);
+		gpu->vm_shared = true;
+	}
 
 	return vm;
 }
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index d27d54bdb7a7..7722776e9129 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -223,6 +223,9 @@ struct msm_gpu {
 
 	struct drm_gpuvm *vm;
 
+	/** @vm_shared: Has @vm been handed out as a context VM? */
+	bool vm_shared;
+
 	/* Power Control: */
 	struct regulator *gpu_reg, *gpu_cx;
 	struct clk_bulk_data *grp_clks;

-- 
2.47.3