On Fri, Sep 19, 2025 at 4:19 AM Prike Liang <[email protected]> wrote:
>
> When a user unmaps a userq VA, the driver must ensure
> the queue has no in-flight jobs. If there is pending work,
> the kernel should wait for the attached eviction (bookkeeping)
> fence to signal before deleting the mapping.
>
> Suggested-by: Christian König <[email protected]>
> Signed-off-by: Prike Liang <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 31 +++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h |  3 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    | 12 +++++++++
>  3 files changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 402145d64714..224d09019997 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -1195,3 +1195,34 @@ int 
> amdgpu_userq_start_sched_for_enforce_isolation(struct amdgpu_device *adev,
>         mutex_unlock(&adev->userq_mutex);
>         return ret;
>  }
> +
> +int amdgpu_userq_gem_va_unmap_validate(struct amdgpu_device *adev,
> +                                      struct amdgpu_bo_va_mapping *mapping,
> +                                      uint64_t saddr)
> +{
> +       u32 ip_mask = amdgpu_userq_get_supported_ip_mask(adev);
> +       struct amdgpu_bo_va *bo_va = mapping->bo_va;
> +       struct dma_resv *resv = bo_va->base.bo->tbo.base.resv;
> +       int ret;
> +
> +       if (!ip_mask)
> +               return 0;
> +
> +       dev_warn_once(adev->dev, "now unmapping a vital queue va:%llx\n", 
> saddr);
> +       /**
> +        * The userq VA mapping reservation should include the eviction fence,
> +        * if the eviction fence can't signal successfully during unmapping,
> +        * then driver will warn to flag this improper unmap of the userq VA.
> +        * Note: The eviction fence may be attached to different BOs, and this
> +        * unmap is only for one kind of userq VAs, so at this point suppose
> +        * the eviction fence is always unsignaled.
> +        */
> +       if (!dma_resv_test_signaled(resv, DMA_RESV_USAGE_BOOKKEEP)) {
> +               ret = dma_resv_wait_timeout(resv, DMA_RESV_USAGE_BOOKKEEP, 
> true,
> +                                           MAX_SCHEDULE_TIMEOUT);
> +               if (ret <= 0)
> +                       return -EBUSY;
> +       }
> +
> +       return 0;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> index f19416feb7ef..7bbbb5988fc7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> @@ -146,4 +146,7 @@ int amdgpu_userq_start_sched_for_enforce_isolation(struct 
> amdgpu_device *adev,
>                                                    u32 idx);
>  int amdgpu_userq_input_va_validate(struct amdgpu_usermode_queue *queue,
>                                    u64 addr, u64 expected_size);
> +int amdgpu_userq_gem_va_unmap_validate(struct amdgpu_device *adev,
> +                                      struct amdgpu_bo_va_mapping *mapping,
> +                                      uint64_t saddr);
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 1ccd348bf48b..e12a314d27a1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1966,6 +1966,7 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
>         struct amdgpu_bo_va_mapping *mapping;
>         struct amdgpu_vm *vm = bo_va->base.vm;
>         bool valid = true;
> +       int r;
>
>         saddr /= AMDGPU_GPU_PAGE_SIZE;
>
> @@ -1986,6 +1987,17 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
>                         return -ENOENT;
>         }
>
> +       /* It's unlikely to happen that the mapping userq hasn't been idled
> +        * during user requests GEM unmap IOCTL except for forcing the unmap
> +        * from user space.
> +        */
> +       if (unlikely(atomic_read(&bo_va->userq_va_mapped) > 0)) {
> +               r = amdgpu_userq_gem_va_unmap_validate(adev, mapping, saddr);
> +               if (unlikely(r == -EBUSY))
> +                       dev_warn_once(adev->dev,
> +                                     "Here should be an improper unmap 
> request from user space\n");

Rephrase the error here.  Maybe something like:

"Attempt to unmap an active userq buffer\n"

With that fixed:
Reviewed-by: Alex Deucher <[email protected]>


> +       }
> +
>         list_del(&mapping->list);
>         amdgpu_vm_it_remove(mapping, &vm->va);
>         mapping->bo_va = NULL;
> --
> 2.34.1
>

Reply via email to