[PATCH 2/2] drm/msm: Fix VM_BIND UNMAP locking

[PATCH 2/2] drm/msm: Fix VM_BIND UNMAP locking

[Rob Clark]

Wrong argument meant that the objs involved in UNMAP ops were not always
getting locked.

Since _NO_SHARE objs share a common resv with the VM (which is always
locked) this would only show up with non-_NO_SHARE BOs.

Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_gem_vma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
index 68a8f8f592fb..40892e206dbd 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -1251,7 +1251,7 @@ vm_bind_job_lock_objects(struct msm_vm_bind_job *job, struct drm_exec *exec)
 			case MSM_VM_BIND_OP_UNMAP:
 				ret = drm_gpuvm_sm_unmap_exec_lock(job->vm, exec,
 							      op->iova,
-							      op->obj_offset);
+							      op->range);
 				break;
 			case MSM_VM_BIND_OP_MAP:
 			case MSM_VM_BIND_OP_MAP_NULL: {
-- 
2.53.0

Re: [PATCH 2/2] drm/msm: Fix VM_BIND UNMAP locking

[Rob Clark]

On Tue, Mar 24, 2026 at 2:40 PM Rob Clark <robin.clark@oss.qualcomm.com> wrote:
>
> Wrong argument meant that the objs involved in UNMAP ops were not always
> getting locked.
>
> Since _NO_SHARE objs share a common resv with the VM (which is always
> locked) this would only show up with non-_NO_SHARE BOs.
>

Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/94
Fixes: 2e6a8a1fe2b2 ("drm/msm: Add VM_BIND ioctl")

> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/msm_gem_vma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index 68a8f8f592fb..40892e206dbd 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -1251,7 +1251,7 @@ vm_bind_job_lock_objects(struct msm_vm_bind_job *job, struct drm_exec *exec)
>                         case MSM_VM_BIND_OP_UNMAP:
>                                 ret = drm_gpuvm_sm_unmap_exec_lock(job->vm, exec,
>                                                               op->iova,
> -                                                             op->obj_offset);
> +                                                             op->range);
>                                 break;
>                         case MSM_VM_BIND_OP_MAP:
>                         case MSM_VM_BIND_OP_MAP_NULL: {
> --
> 2.53.0
>

[PATCH 1/2] drm/msm: Disallow foreign mapping of _NO_SHARE

[Rob Clark]

This restriction applies to mapping of _NO_SHARE objs in the kms vm as
well as importing/exporting BOs.  Since the DPU has it's own VM, scanout
counts as "exporting" a BO from outside of it's host VM.

Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
v2: Fix issue with MAP_NULL

 drivers/gpu/drm/msm/msm_gem_vma.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
index 1c6b0920c0d8..43d61e0919bd 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -373,6 +373,12 @@ msm_gem_vma_new(struct drm_gpuvm *gpuvm, struct drm_gem_object *obj,
 	struct msm_gem_vma *vma;
 	int ret;
 
+	/* _NO_SHARE objs cannot be mapped outside of their "host" vm: */
+	if (obj && (to_msm_bo(obj)->flags & MSM_BO_NO_SHARE) &&
+	    GEM_WARN_ON(obj->resv != drm_gpuvm_resv(gpuvm))) {
+		return ERR_PTR(-EINVAL);
+	}
+
 	drm_gpuvm_resv_assert_held(&vm->base);
 
 	vma = kzalloc(sizeof(*vma), GFP_KERNEL);
-- 
2.53.0

[PATCH 2/2] drm/msm: Fix VM_BIND UNMAP locking

[Rob Clark]

Wrong argument meant that the objs involved in UNMAP ops were not always
getting locked.

Since _NO_SHARE objs share a common resv with the VM (which is always
locked) this would only show up with non-_NO_SHARE BOs.

Reported-by: Victoria Brekenfeld <victoria@system76.com>
Fixes: 2e6a8a1fe2b2 ("drm/msm: Add VM_BIND ioctl")
Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/94
Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
v2: Add tags

 drivers/gpu/drm/msm/msm_gem_vma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
index 43d61e0919bd..953a2403f598 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -1251,7 +1251,7 @@ vm_bind_job_lock_objects(struct msm_vm_bind_job *job, struct drm_exec *exec)
 			case MSM_VM_BIND_OP_UNMAP:
 				ret = drm_gpuvm_sm_unmap_exec_lock(job->vm, exec,
 							      op->iova,
-							      op->obj_offset);
+							      op->range);
 				break;
 			case MSM_VM_BIND_OP_MAP:
 			case MSM_VM_BIND_OP_MAP_NULL: {
-- 
2.53.0

Re: [PATCH 1/2] drm/msm: Disallow foreign mapping of _NO_SHARE

[Dmitry Baryshkov]

On Tue, Mar 24, 2026 at 03:05:17PM -0700, Rob Clark wrote:
> This restriction applies to mapping of _NO_SHARE objs in the kms vm as
> well as importing/exporting BOs.  Since the DPU has it's own VM, scanout
> counts as "exporting" a BO from outside of it's host VM.
> 
> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> ---
> v2: Fix issue with MAP_NULL
> 
>  drivers/gpu/drm/msm/msm_gem_vma.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index 1c6b0920c0d8..43d61e0919bd 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -373,6 +373,12 @@ msm_gem_vma_new(struct drm_gpuvm *gpuvm, struct drm_gem_object *obj,
>  	struct msm_gem_vma *vma;
>  	int ret;
>  
> +	/* _NO_SHARE objs cannot be mapped outside of their "host" vm: */
> +	if (obj && (to_msm_bo(obj)->flags & MSM_BO_NO_SHARE) &&
> +	    GEM_WARN_ON(obj->resv != drm_gpuvm_resv(gpuvm))) {

Can this be used to spam the logs?

> +		return ERR_PTR(-EINVAL);
> +	}
> +
>  	drm_gpuvm_resv_assert_held(&vm->base);
>  
>  	vma = kzalloc(sizeof(*vma), GFP_KERNEL);
> -- 
> 2.53.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] drm/msm: Disallow foreign mapping of _NO_SHARE

[Rob Clark]

On Tue, Mar 24, 2026 at 3:20 PM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Tue, Mar 24, 2026 at 03:05:17PM -0700, Rob Clark wrote:
> > This restriction applies to mapping of _NO_SHARE objs in the kms vm as
> > well as importing/exporting BOs.  Since the DPU has it's own VM, scanout
> > counts as "exporting" a BO from outside of it's host VM.
> >
> > Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> > ---
> > v2: Fix issue with MAP_NULL
> >
> >  drivers/gpu/drm/msm/msm_gem_vma.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> > index 1c6b0920c0d8..43d61e0919bd 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> > @@ -373,6 +373,12 @@ msm_gem_vma_new(struct drm_gpuvm *gpuvm, struct drm_gem_object *obj,
> >       struct msm_gem_vma *vma;
> >       int ret;
> >
> > +     /* _NO_SHARE objs cannot be mapped outside of their "host" vm: */
> > +     if (obj && (to_msm_bo(obj)->flags & MSM_BO_NO_SHARE) &&
> > +         GEM_WARN_ON(obj->resv != drm_gpuvm_resv(gpuvm))) {
>
> Can this be used to spam the logs?

yeah, perhaps it should use UERR() instead

>
> > +             return ERR_PTR(-EINVAL);
> > +     }
> > +
> >       drm_gpuvm_resv_assert_held(&vm->base);
> >
> >       vma = kzalloc(sizeof(*vma), GFP_KERNEL);
> > --
> > 2.53.0
> >
>
> --
> With best wishes
> Dmitry

Re: [PATCH 1/2] drm/msm: Disallow foreign mapping of _NO_SHARE

[Rob Clark]

On Wed, Mar 25, 2026 at 8:17 AM Rob Clark <rob.clark@oss.qualcomm.com> wrote:
>
> On Tue, Mar 24, 2026 at 3:20 PM Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com> wrote:
> >
> > On Tue, Mar 24, 2026 at 03:05:17PM -0700, Rob Clark wrote:
> > > This restriction applies to mapping of _NO_SHARE objs in the kms vm as
> > > well as importing/exporting BOs.  Since the DPU has it's own VM, scanout
> > > counts as "exporting" a BO from outside of it's host VM.
> > >
> > > Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> > > ---
> > > v2: Fix issue with MAP_NULL
> > >
> > >  drivers/gpu/drm/msm/msm_gem_vma.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> > > index 1c6b0920c0d8..43d61e0919bd 100644
> > > --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> > > +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> > > @@ -373,6 +373,12 @@ msm_gem_vma_new(struct drm_gpuvm *gpuvm, struct drm_gem_object *obj,
> > >       struct msm_gem_vma *vma;
> > >       int ret;
> > >
> > > +     /* _NO_SHARE objs cannot be mapped outside of their "host" vm: */
> > > +     if (obj && (to_msm_bo(obj)->flags & MSM_BO_NO_SHARE) &&
> > > +         GEM_WARN_ON(obj->resv != drm_gpuvm_resv(gpuvm))) {
> >
> > Can this be used to spam the logs?
>
> yeah, perhaps it should use UERR() instead

So, for the case that I was concerned about, we should actually just
reject creating a drm fb from _NO_SHARE BO(s) (and report error w/
UERR()).  I'll send a patch for that.  With that, I think it is safe
to keep this WARN_ON(), since the only path to hit it should be a
driver bug.

BR,
-R

> >
> > > +             return ERR_PTR(-EINVAL);
> > > +     }
> > > +
> > >       drm_gpuvm_resv_assert_held(&vm->base);
> > >
> > >       vma = kzalloc(sizeof(*vma), GFP_KERNEL);
> > > --
> > > 2.53.0
> > >
> >
> > --
> > With best wishes
> > Dmitry