[PATCH 9/9] drm/amdgpu: validate userq activity status for GEM_VA unmap

[Alex Deucher]

On Fri, May 30, 2025 at 4:05 AM Prike Liang <Prike.Liang at amd.com> wrote:
>
> The userq VA unmap requires validating queue status before unamapping
> it, if user tries to unmap an active userq by GEM VA IOCTL then the
> driver should report an error for this illegal usage.
>
> Signed-off-by: Prike Liang <Prike.Liang at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 16 +++++++++++++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    |  5 +++++
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index e43a61f64755..e2275280554d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -234,7 +234,7 @@ amdgpu_userq_map_helper(struct amdgpu_userq_mgr *uq_mgr,
>         return r;
>  }
>
> -static void
> +static int
>  amdgpu_userq_wait_for_last_fence(struct amdgpu_userq_mgr *uq_mgr,
>                                  struct amdgpu_usermode_queue *queue)
>  {
> @@ -243,10 +243,14 @@ amdgpu_userq_wait_for_last_fence(struct amdgpu_userq_mgr *uq_mgr,
>
>         if (f && !dma_fence_is_signaled(f)) {
>                 ret = dma_fence_wait_timeout(f, true, msecs_to_jiffies(100));
> -               if (ret <= 0)
> +               if (ret <= 0) {
>                         drm_file_err(uq_mgr->file, "Timed out waiting for fence=%llu:%llu\n",
>                                      f->context, f->seqno);
> +                       return -ETIMEDOUT;
> +               }
>         }
> +
> +       return 0;
>  }
>
>  static void
> @@ -464,7 +468,13 @@ amdgpu_userq_destroy(struct drm_file *filp, int queue_id)
>                 mutex_unlock(&uq_mgr->userq_mutex);
>                 return -EINVAL;
>         }
> -       amdgpu_userq_wait_for_last_fence(uq_mgr, queue);
> +
> +       if (amdgpu_userq_wait_for_last_fence(uq_mgr, queue)) {
> +               drm_warn(adev_to_drm(uq_mgr->adev), "Don't destroy a busy userq\n");
> +               mutex_unlock(&uq_mgr->userq_mutex);
> +               return -EINVAL;
> +       }
> +
>         r = amdgpu_bo_reserve(queue->db_obj.obj, true);
>         if (!r) {
>                 amdgpu_bo_unpin(queue->db_obj.obj);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 5e075e8f0ca3..168fc3835aaf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1930,6 +1930,11 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
>         struct amdgpu_vm *vm = bo_va->base.vm;
>         bool valid = true;
>
> +       if (amdgpu_userq_gem_va_unmap_wait(vm)) {
> +               dev_warn(adev->dev, "shouldn't unmap the VA for an active userq\n");
> +               return -EINVAL;
> +       }

There's not need to wait for anything.  Just return an error if the
userq has not been destroyed yet.

Alex

> +
>         saddr /= AMDGPU_GPU_PAGE_SIZE;
>
>         list_for_each_entry(mapping, &bo_va->valids, list) {
> --
> 2.34.1
>