[PATCH v2 1/2] drm/amdgpu/userq: use drm_exec in amdgpu_userq_fence_read_wptr

[PATCH v2 1/2] drm/amdgpu/userq: use drm_exec in amdgpu_userq_fence_read_wptr

[Sunil Khatri]

To access the bo from vm mapping first lock the root bo and
then the object bo of the mapping to make sure both locks
are taken safely.

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   | 55 +++++++++----------
 1 file changed, 26 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 369914cab555..008330a0d852 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -370,51 +370,48 @@ static int amdgpu_userq_fence_read_wptr(struct amdgpu_device *adev,
 {
 	struct amdgpu_bo_va_mapping *mapping;
 	struct amdgpu_bo *bo;
+	struct drm_exec exec;
 	u64 addr, *ptr;
-	int r;
-
-	r = amdgpu_bo_reserve(queue->vm->root.bo, false);
-	if (r)
-		return r;
+	int ret;
 
 	addr = queue->userq_prop->wptr_gpu_addr;
 	addr &= AMDGPU_GMC_HOLE_MASK;
 
-	mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, addr >> PAGE_SHIFT);
-	if (!mapping) {
-		amdgpu_bo_unreserve(queue->vm->root.bo);
-		DRM_ERROR("Failed to lookup amdgpu_bo_va_mapping\n");
-		return -EINVAL;
-	}
+	drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES, 2);
+	drm_exec_until_all_locked(&exec) {
+		ret = amdgpu_vm_lock_pd(queue->vm, &exec, 1);
+		drm_exec_retry_on_contention(&exec);
+		if (unlikely(ret))
+			goto lock_error;
 
-	bo = amdgpu_bo_ref(mapping->bo_va->base.bo);
-	amdgpu_bo_unreserve(queue->vm->root.bo);
-	r = amdgpu_bo_reserve(bo, true);
-	if (r) {
-		amdgpu_bo_unref(&bo);
-		DRM_ERROR("Failed to reserve userqueue wptr bo");
-		return r;
+		mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, addr >> PAGE_SHIFT);
+		if (!mapping) {
+			ret = -EINVAL;
+			goto lock_error;
+		}
+
+		ret = drm_exec_lock_obj(&exec, &mapping->bo_va->base.bo->tbo.base);
+		drm_exec_retry_on_contention(&exec);
+		if (unlikely(ret))
+			goto lock_error;
 	}
 
-	r = amdgpu_bo_kmap(bo, (void **)&ptr);
-	if (r) {
+	bo = mapping->bo_va->base.bo;
+	ret = amdgpu_bo_kmap(bo, (void **)&ptr);
+	if (ret) {
 		DRM_ERROR("Failed mapping the userqueue wptr bo");
-		goto map_error;
+		goto lock_error;
 	}
 
 	*wptr = le64_to_cpu(*ptr);
 
 	amdgpu_bo_kunmap(bo);
-	amdgpu_bo_unreserve(bo);
-	amdgpu_bo_unref(&bo);
-
+	drm_exec_fini(&exec);
 	return 0;
 
-map_error:
-	amdgpu_bo_unreserve(bo);
-	amdgpu_bo_unref(&bo);
-
-	return r;
+lock_error:
+	drm_exec_fini(&exec);
+	return ret;
 }
 
 static void
-- 
2.34.1

[PATCH v2 2/2] drm/amdgpu/userq: pin mqd and fw object bo to avoid eviction

[Sunil Khatri]

mqd and fw objects are queue core objects which should remain
valid and never be unmapped and evicted for user queues to work
properly.

During eviction if these buffers are evicted the hw continue to
use the invalid addresses and caused page faults and system hung.

Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 06b7b4228065..813df2d87dc3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -504,16 +504,20 @@ int amdgpu_userq_create_object(struct amdgpu_userq_mgr *uq_mgr,
 		goto free_obj;
 	}
 
+	r = amdgpu_bo_pin(userq_obj->obj, AMDGPU_GEM_DOMAIN_GTT);
+	if (r)
+		goto unresv;
+
 	r = amdgpu_ttm_alloc_gart(&(userq_obj->obj)->tbo);
 	if (r) {
 		drm_file_err(uq_mgr->file, "Failed to alloc GART for userqueue object (%d)", r);
-		goto unresv;
+		goto unpin_bo;
 	}
 
 	r = amdgpu_bo_kmap(userq_obj->obj, &userq_obj->cpu_ptr);
 	if (r) {
 		drm_file_err(uq_mgr->file, "Failed to map BO for userqueue (%d)", r);
-		goto unresv;
+		goto unpin_bo;
 	}
 
 	userq_obj->gpu_addr = amdgpu_bo_gpu_offset(userq_obj->obj);
@@ -521,11 +525,13 @@ int amdgpu_userq_create_object(struct amdgpu_userq_mgr *uq_mgr,
 	memset(userq_obj->cpu_ptr, 0, size);
 	return 0;
 
+unpin_bo:
+	amdgpu_bo_unpin(userq_obj->obj);
 unresv:
 	amdgpu_bo_unreserve(userq_obj->obj);
-
 free_obj:
 	amdgpu_bo_unref(&userq_obj->obj);
+
 	return r;
 }
 
@@ -533,6 +539,7 @@ void amdgpu_userq_destroy_object(struct amdgpu_userq_mgr *uq_mgr,
 				 struct amdgpu_userq_obj *userq_obj)
 {
 	amdgpu_bo_kunmap(userq_obj->obj);
+	amdgpu_bo_unpin(userq_obj->obj);
 	amdgpu_bo_unref(&userq_obj->obj);
 }
 
-- 
2.34.1

Re: [PATCH v2 2/2] drm/amdgpu/userq: pin mqd and fw object bo to avoid eviction

[Christian König]

On 5/8/26 12:39, Sunil Khatri wrote:
> mqd and fw objects are queue core objects which should remain
> valid and never be unmapped and evicted for user queues to work
> properly.
> 
> During eviction if these buffers are evicted the hw continue to
> use the invalid addresses and caused page faults and system hung.
> 
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>

We should probably use the eviction fence instead of pinning, but that can come in a later patch set.

Reviewed-by: Christian König <christian.koenig@amd.com> for now.

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 06b7b4228065..813df2d87dc3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -504,16 +504,20 @@ int amdgpu_userq_create_object(struct amdgpu_userq_mgr *uq_mgr,
>  		goto free_obj;
>  	}
>  
> +	r = amdgpu_bo_pin(userq_obj->obj, AMDGPU_GEM_DOMAIN_GTT);
> +	if (r)
> +		goto unresv;
> +
>  	r = amdgpu_ttm_alloc_gart(&(userq_obj->obj)->tbo);
>  	if (r) {
>  		drm_file_err(uq_mgr->file, "Failed to alloc GART for userqueue object (%d)", r);
> -		goto unresv;
> +		goto unpin_bo;
>  	}
>  
>  	r = amdgpu_bo_kmap(userq_obj->obj, &userq_obj->cpu_ptr);
>  	if (r) {
>  		drm_file_err(uq_mgr->file, "Failed to map BO for userqueue (%d)", r);
> -		goto unresv;
> +		goto unpin_bo;
>  	}
>  
>  	userq_obj->gpu_addr = amdgpu_bo_gpu_offset(userq_obj->obj);
> @@ -521,11 +525,13 @@ int amdgpu_userq_create_object(struct amdgpu_userq_mgr *uq_mgr,
>  	memset(userq_obj->cpu_ptr, 0, size);
>  	return 0;
>  
> +unpin_bo:
> +	amdgpu_bo_unpin(userq_obj->obj);
>  unresv:
>  	amdgpu_bo_unreserve(userq_obj->obj);
> -
>  free_obj:
>  	amdgpu_bo_unref(&userq_obj->obj);
> +
>  	return r;
>  }
>  
> @@ -533,6 +539,7 @@ void amdgpu_userq_destroy_object(struct amdgpu_userq_mgr *uq_mgr,
>  				 struct amdgpu_userq_obj *userq_obj)
>  {
>  	amdgpu_bo_kunmap(userq_obj->obj);
> +	amdgpu_bo_unpin(userq_obj->obj);
>  	amdgpu_bo_unref(&userq_obj->obj);
>  }
>  

Re: [PATCH v2 2/2] drm/amdgpu/userq: pin mqd and fw object bo to avoid eviction

[Khatri, Sunil]

On 11-05-2026 06:57 pm, Christian König wrote:
> On 5/8/26 12:39, Sunil Khatri wrote:
>> mqd and fw objects are queue core objects which should remain
>> valid and never be unmapped and evicted for user queues to work
>> properly.
>>
>> During eviction if these buffers are evicted the hw continue to
>> use the invalid addresses and caused page faults and system hung.
>>
>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
> We should probably use the eviction fence instead of pinning, but that can come in a later patch set.

yes, that is todo but also on other hand these buffers need to stay 
valid till the queue stays valid and active so keeping these buffers 
alive is fine but yes associating with eviction fence sounds correct.

Regards
Sunil khatri

>
> Reviewed-by: Christian König <christian.koenig@amd.com> for now.
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> index 06b7b4228065..813df2d87dc3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>> @@ -504,16 +504,20 @@ int amdgpu_userq_create_object(struct amdgpu_userq_mgr *uq_mgr,
>>   		goto free_obj;
>>   	}
>>   
>> +	r = amdgpu_bo_pin(userq_obj->obj, AMDGPU_GEM_DOMAIN_GTT);
>> +	if (r)
>> +		goto unresv;
>> +
>>   	r = amdgpu_ttm_alloc_gart(&(userq_obj->obj)->tbo);
>>   	if (r) {
>>   		drm_file_err(uq_mgr->file, "Failed to alloc GART for userqueue object (%d)", r);
>> -		goto unresv;
>> +		goto unpin_bo;
>>   	}
>>   
>>   	r = amdgpu_bo_kmap(userq_obj->obj, &userq_obj->cpu_ptr);
>>   	if (r) {
>>   		drm_file_err(uq_mgr->file, "Failed to map BO for userqueue (%d)", r);
>> -		goto unresv;
>> +		goto unpin_bo;
>>   	}
>>   
>>   	userq_obj->gpu_addr = amdgpu_bo_gpu_offset(userq_obj->obj);
>> @@ -521,11 +525,13 @@ int amdgpu_userq_create_object(struct amdgpu_userq_mgr *uq_mgr,
>>   	memset(userq_obj->cpu_ptr, 0, size);
>>   	return 0;
>>   
>> +unpin_bo:
>> +	amdgpu_bo_unpin(userq_obj->obj);
>>   unresv:
>>   	amdgpu_bo_unreserve(userq_obj->obj);
>> -
>>   free_obj:
>>   	amdgpu_bo_unref(&userq_obj->obj);
>> +
>>   	return r;
>>   }
>>   
>> @@ -533,6 +539,7 @@ void amdgpu_userq_destroy_object(struct amdgpu_userq_mgr *uq_mgr,
>>   				 struct amdgpu_userq_obj *userq_obj)
>>   {
>>   	amdgpu_bo_kunmap(userq_obj->obj);
>> +	amdgpu_bo_unpin(userq_obj->obj);
>>   	amdgpu_bo_unref(&userq_obj->obj);
>>   }
>>   

Re: [PATCH v2 1/2] drm/amdgpu/userq: use drm_exec in amdgpu_userq_fence_read_wptr

[Christian König]

On 5/8/26 12:39, Sunil Khatri wrote:
> To access the bo from vm mapping first lock the root bo and
> then the object bo of the mapping to make sure both locks
> are taken safely.
> 
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   | 55 +++++++++----------
>  1 file changed, 26 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index 369914cab555..008330a0d852 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -370,51 +370,48 @@ static int amdgpu_userq_fence_read_wptr(struct amdgpu_device *adev,
>  {
>  	struct amdgpu_bo_va_mapping *mapping;
>  	struct amdgpu_bo *bo;
> +	struct drm_exec exec;
>  	u64 addr, *ptr;
> -	int r;
> -
> -	r = amdgpu_bo_reserve(queue->vm->root.bo, false);
> -	if (r)
> -		return r;
> +	int ret;
>  
>  	addr = queue->userq_prop->wptr_gpu_addr;
>  	addr &= AMDGPU_GMC_HOLE_MASK;
>  
> -	mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, addr >> PAGE_SHIFT);
> -	if (!mapping) {
> -		amdgpu_bo_unreserve(queue->vm->root.bo);
> -		DRM_ERROR("Failed to lookup amdgpu_bo_va_mapping\n");
> -		return -EINVAL;
> -	}
> +	drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES, 2);
> +	drm_exec_until_all_locked(&exec) {
> +		ret = amdgpu_vm_lock_pd(queue->vm, &exec, 1);
> +		drm_exec_retry_on_contention(&exec);
> +		if (unlikely(ret))
> +			goto lock_error;
>  
> -	bo = amdgpu_bo_ref(mapping->bo_va->base.bo);
> -	amdgpu_bo_unreserve(queue->vm->root.bo);
> -	r = amdgpu_bo_reserve(bo, true);
> -	if (r) {
> -		amdgpu_bo_unref(&bo);
> -		DRM_ERROR("Failed to reserve userqueue wptr bo");
> -		return r;
> +		mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, addr >> PAGE_SHIFT);
> +		if (!mapping) {
> +			ret = -EINVAL;
> +			goto lock_error;
> +		}
> +
> +		ret = drm_exec_lock_obj(&exec, &mapping->bo_va->base.bo->tbo.base);
> +		drm_exec_retry_on_contention(&exec);
> +		if (unlikely(ret))
> +			goto lock_error;
>  	}
>  
> -	r = amdgpu_bo_kmap(bo, (void **)&ptr);
> -	if (r) {
> +	bo = mapping->bo_va->base.bo;
> +	ret = amdgpu_bo_kmap(bo, (void **)&ptr);
> +	if (ret) {
>  		DRM_ERROR("Failed mapping the userqueue wptr bo");
> -		goto map_error;
> +		goto lock_error;
>  	}
>  
>  	*wptr = le64_to_cpu(*ptr);
>  
>  	amdgpu_bo_kunmap(bo);
> -	amdgpu_bo_unreserve(bo);
> -	amdgpu_bo_unref(&bo);
> -
> +	drm_exec_fini(&exec);
>  	return 0;
>  
> -map_error:
> -	amdgpu_bo_unreserve(bo);
> -	amdgpu_bo_unref(&bo);
> -
> -	return r;
> +lock_error:
> +	drm_exec_fini(&exec);
> +	return ret;
>  }
>  
>  static void

Re: [PATCH v2 1/2] drm/amdgpu/userq: use drm_exec in amdgpu_userq_fence_read_wptr

[Christian König]

On 5/8/26 12:39, Sunil Khatri wrote:
> To access the bo from vm mapping first lock the root bo and
> then the object bo of the mapping to make sure both locks
> are taken safely.
> 
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   | 55 +++++++++----------
>  1 file changed, 26 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index 369914cab555..008330a0d852 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -370,51 +370,48 @@ static int amdgpu_userq_fence_read_wptr(struct amdgpu_device *adev,
>  {
>  	struct amdgpu_bo_va_mapping *mapping;
>  	struct amdgpu_bo *bo;
> +	struct drm_exec exec;
>  	u64 addr, *ptr;
> -	int r;
> -
> -	r = amdgpu_bo_reserve(queue->vm->root.bo, false);
> -	if (r)
> -		return r;
> +	int ret;
>  
>  	addr = queue->userq_prop->wptr_gpu_addr;
>  	addr &= AMDGPU_GMC_HOLE_MASK;
>  
> -	mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, addr >> PAGE_SHIFT);
> -	if (!mapping) {
> -		amdgpu_bo_unreserve(queue->vm->root.bo);
> -		DRM_ERROR("Failed to lookup amdgpu_bo_va_mapping\n");
> -		return -EINVAL;
> -	}
> +	drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES, 2);
> +	drm_exec_until_all_locked(&exec) {
> +		ret = amdgpu_vm_lock_pd(queue->vm, &exec, 1);
> +		drm_exec_retry_on_contention(&exec);
> +		if (unlikely(ret))
> +			goto lock_error;
>  
> -	bo = amdgpu_bo_ref(mapping->bo_va->base.bo);
> -	amdgpu_bo_unreserve(queue->vm->root.bo);
> -	r = amdgpu_bo_reserve(bo, true);
> -	if (r) {
> -		amdgpu_bo_unref(&bo);
> -		DRM_ERROR("Failed to reserve userqueue wptr bo");
> -		return r;
> +		mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, addr >> PAGE_SHIFT);
> +		if (!mapping) {
> +			ret = -EINVAL;
> +			goto lock_error;
> +		}
> +
> +		ret = drm_exec_lock_obj(&exec, &mapping->bo_va->base.bo->tbo.base);
> +		drm_exec_retry_on_contention(&exec);
> +		if (unlikely(ret))
> +			goto lock_error;
>  	}
>  
> -	r = amdgpu_bo_kmap(bo, (void **)&ptr);
> -	if (r) {
> +	bo = mapping->bo_va->base.bo;
> +	ret = amdgpu_bo_kmap(bo, (void **)&ptr);
> +	if (ret) {
>  		DRM_ERROR("Failed mapping the userqueue wptr bo");
> -		goto map_error;
> +		goto lock_error;
>  	}
>  
>  	*wptr = le64_to_cpu(*ptr);
>  
>  	amdgpu_bo_kunmap(bo);
> -	amdgpu_bo_unreserve(bo);
> -	amdgpu_bo_unref(&bo);
> -
> +	drm_exec_fini(&exec);
>  	return 0;
>  
> -map_error:
> -	amdgpu_bo_unreserve(bo);
> -	amdgpu_bo_unref(&bo);
> -
> -	return r;
> +lock_error:
> +	drm_exec_fini(&exec);
> +	return ret;
>  }
>  
>  static void