[PATCH V3] accel/amdxdna: Prevent PM resume deadlock in hwctx_sync_debug_bo()

[PATCH V3] accel/amdxdna: Prevent PM resume deadlock in hwctx_sync_debug_bo()

[Lizhi Hou]

amdxdna_hwctx_sync_debug_bo() invokes the hardware hwctx_sync_debug_bo()
callback while holding xdna->dev_lock.

The callback may call amdxdna_cmd_submit(), which in turn calls
amdxdna_pm_resume_get(). If the device is suspended,
amdxdna_pm_resume_get() may synchronously execute amdxdna_pm_resume(),
which also acquires xdna->dev_lock, resulting in a deadlock.

Avoid the deadlock by calling amdxdna_pm_resume_get() before holding
xdna->dev_lock in both amdxdna_hwctx_sync_debug_bo() and
amdxdna_drm_config_hwctx_ioctl()

Fixes: 7ea046838021 ("accel/amdxdna: Support firmware debug buffer")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
V2:
  Fix jumping forward over the guard(mutex) declaration.
V3:
  Fix the same in amdxdna_drm_config_hwctx_ioctl().

 drivers/accel/amdxdna/amdxdna_ctx.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c
index ff6c3e8e5a15..2ab4443e7094 100644
--- a/drivers/accel/amdxdna/amdxdna_ctx.c
+++ b/drivers/accel/amdxdna/amdxdna_ctx.c
@@ -356,16 +356,25 @@ int amdxdna_drm_config_hwctx_ioctl(struct drm_device *dev, void *data, struct dr
 		return -EINVAL;
 	}
 
-	guard(mutex)(&xdna->dev_lock);
+	ret = amdxdna_pm_resume_get(xdna);
+	if (ret) {
+		XDNA_ERR(xdna, "Resume failed, ret %d", ret);
+		goto free_buf;
+	}
+
+	mutex_lock(&xdna->dev_lock);
 	hwctx = xa_load(&client->hwctx_xa, args->handle);
 	if (!hwctx) {
 		XDNA_DBG(xdna, "PID %d failed to get hwctx %d", client->pid, args->handle);
 		ret = -EINVAL;
-		goto free_buf;
+		goto unlock;
 	}
 
 	ret = xdna->dev_info->ops->hwctx_config(hwctx, args->param_type, val, buf, buf_size);
 
+unlock:
+	mutex_unlock(&xdna->dev_lock);
+	amdxdna_pm_suspend_put(xdna);
 free_buf:
 	kfree(buf);
 	return ret;
@@ -386,16 +395,25 @@ int amdxdna_hwctx_sync_debug_bo(struct amdxdna_client *client, u32 debug_bo_hdl)
 	if (!gobj)
 		return -EINVAL;
 
+	ret = amdxdna_pm_resume_get(xdna);
+	if (ret) {
+		XDNA_ERR(xdna, "Resume failed, ret %d", ret);
+		goto put_obj;
+	}
+
 	abo = to_xdna_obj(gobj);
-	guard(mutex)(&xdna->dev_lock);
+	mutex_lock(&xdna->dev_lock);
 	hwctx = xa_load(&client->hwctx_xa, abo->assigned_hwctx);
 	if (!hwctx) {
 		ret = -EINVAL;
-		goto put_obj;
+		goto unlock;
 	}
 
 	ret = xdna->dev_info->ops->hwctx_sync_debug_bo(hwctx, debug_bo_hdl);
 
+unlock:
+	mutex_unlock(&xdna->dev_lock);
+	amdxdna_pm_suspend_put(xdna);
 put_obj:
 	drm_gem_object_put(gobj);
 	return ret;
-- 
2.34.1

Re: [PATCH V3] accel/amdxdna: Prevent PM resume deadlock in hwctx_sync_debug_bo()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Concurrent destruction of hwctx causes a Use-After-Free because `dev_lock` is temporarily dropped during PM runtime resume without holding the SRCU read lock.
--

commit 3f02e0b8320e3b9382db4505bfac1b4c92265f29
Author: Lizhi Hou <lizhi.hou@amd.com>

accel/amdxdna: Prevent PM resume deadlock in hwctx_sync_debug_bo()

This commit addresses a potential deadlock by calling amdxdna_pm_resume_get()
before acquiring xdna->dev_lock in both amdxdna_hwctx_sync_debug_bo() and
amdxdna_drm_config_hwctx_ioctl().

> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c
> index 855da8c79a1cd..67a2abcf173ee 100644
> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c
> @@ -382,16 +382,25 @@ int amdxdna_drm_config_hwctx_ioctl(struct drm_device *dev, void *data, struct dr
[ ... ]
> +	mutex_lock(&xdna->dev_lock);
>  	hwctx = xa_load(&client->hwctx_xa, args->handle);
>  	if (!hwctx) {
>  		XDNA_DBG(xdna, "PID %d failed to get hwctx %d", client->pid, args->handle);
>  		ret = -EINVAL;
> -		goto free_buf;
> +		goto unlock;
>  	}
>  
>  	ret = xdna->dev_info->ops->hwctx_config(hwctx, args->param_type, val, buf, buf_size);

[Severity: High]
This isn't a bug introduced by this patch, but does dropping the lock inside
this callback expose hwctx to a use-after-free?

If hwctx_config() (for example, aie2_hwctx_cu_config()) calls
amdxdna_pm_resume_get_locked(xdna), which temporarily unlocks and re-locks
xdna->dev_lock:

	mutex_unlock(&xdna->dev_lock);
	ret = amdxdna_pm_resume_get(xdna);
	mutex_lock(&xdna->dev_lock);

During this window, could a concurrent amdxdna_drm_destroy_hwctx_ioctl()
acquire dev_lock, erase hwctx from the xarray, and call
amdxdna_hwctx_destroy_rcu()?

Because the SRCU read lock (client->hwctx_srcu) is not held here,
synchronize_srcu() wouldn't block, and hwctx could be freed while still
in use after re-acquiring dev_lock.

It appears a similar issue might also exist in amdxdna_update_heap(), which
iterates over hwctx_xa without the SRCU read lock and drops both mm_lock
and dev_lock inside the loop body via amdxdna_hwctx_expand_heap().

>  
> +unlock:
> +	mutex_unlock(&xdna->dev_lock);
> +	amdxdna_pm_suspend_put(xdna);
>  free_buf:
>  	kfree(buf);
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616212429.3620645-1-lizhi.hou@amd.com?part=1

Re: [PATCH V3] accel/amdxdna: Prevent PM resume deadlock in hwctx_sync_debug_bo()

[Max Zhen]

On 6/16/2026 Tue 14:24, Lizhi Hou wrote:
> amdxdna_hwctx_sync_debug_bo() invokes the hardware hwctx_sync_debug_bo()
> callback while holding xdna->dev_lock.
> 
> The callback may call amdxdna_cmd_submit(), which in turn calls
> amdxdna_pm_resume_get(). If the device is suspended,
> amdxdna_pm_resume_get() may synchronously execute amdxdna_pm_resume(),
> which also acquires xdna->dev_lock, resulting in a deadlock.
> 
> Avoid the deadlock by calling amdxdna_pm_resume_get() before holding
> xdna->dev_lock in both amdxdna_hwctx_sync_debug_bo() and
> amdxdna_drm_config_hwctx_ioctl()
> 
> Fixes: 7ea046838021 ("accel/amdxdna: Support firmware debug buffer")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>

Reviewed-by: Max Zhen <max.zhen@amd.com>

> ---
> V2:
>    Fix jumping forward over the guard(mutex) declaration.
> V3:
>    Fix the same in amdxdna_drm_config_hwctx_ioctl().
> 
>   drivers/accel/amdxdna/amdxdna_ctx.c | 26 ++++++++++++++++++++++----
>   1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c
> index ff6c3e8e5a15..2ab4443e7094 100644
> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c
> @@ -356,16 +356,25 @@ int amdxdna_drm_config_hwctx_ioctl(struct drm_device *dev, void *data, struct dr
>   		return -EINVAL;
>   	}
>   
> -	guard(mutex)(&xdna->dev_lock);
> +	ret = amdxdna_pm_resume_get(xdna);
> +	if (ret) {
> +		XDNA_ERR(xdna, "Resume failed, ret %d", ret);
> +		goto free_buf;
> +	}
> +
> +	mutex_lock(&xdna->dev_lock);
>   	hwctx = xa_load(&client->hwctx_xa, args->handle);
>   	if (!hwctx) {
>   		XDNA_DBG(xdna, "PID %d failed to get hwctx %d", client->pid, args->handle);
>   		ret = -EINVAL;
> -		goto free_buf;
> +		goto unlock;
>   	}
>   
>   	ret = xdna->dev_info->ops->hwctx_config(hwctx, args->param_type, val, buf, buf_size);
>   
> +unlock:
> +	mutex_unlock(&xdna->dev_lock);
> +	amdxdna_pm_suspend_put(xdna);
>   free_buf:
>   	kfree(buf);
>   	return ret;
> @@ -386,16 +395,25 @@ int amdxdna_hwctx_sync_debug_bo(struct amdxdna_client *client, u32 debug_bo_hdl)
>   	if (!gobj)
>   		return -EINVAL;
>   
> +	ret = amdxdna_pm_resume_get(xdna);
> +	if (ret) {
> +		XDNA_ERR(xdna, "Resume failed, ret %d", ret);
> +		goto put_obj;
> +	}
> +
>   	abo = to_xdna_obj(gobj);
> -	guard(mutex)(&xdna->dev_lock);
> +	mutex_lock(&xdna->dev_lock);
>   	hwctx = xa_load(&client->hwctx_xa, abo->assigned_hwctx);
>   	if (!hwctx) {
>   		ret = -EINVAL;
> -		goto put_obj;
> +		goto unlock;
>   	}
>   
>   	ret = xdna->dev_info->ops->hwctx_sync_debug_bo(hwctx, debug_bo_hdl);
>   
> +unlock:
> +	mutex_unlock(&xdna->dev_lock);
> +	amdxdna_pm_suspend_put(xdna);
>   put_obj:
>   	drm_gem_object_put(gobj);
>   	return ret;

Re: [PATCH V3] accel/amdxdna: Prevent PM resume deadlock in hwctx_sync_debug_bo()

[Lizhi Hou]

Applied to drm-misc-fixes

On 7/6/26 15:34, Max Zhen wrote:
>
>
> On 6/16/2026 Tue 14:24, Lizhi Hou wrote:
>> amdxdna_hwctx_sync_debug_bo() invokes the hardware hwctx_sync_debug_bo()
>> callback while holding xdna->dev_lock.
>>
>> The callback may call amdxdna_cmd_submit(), which in turn calls
>> amdxdna_pm_resume_get(). If the device is suspended,
>> amdxdna_pm_resume_get() may synchronously execute amdxdna_pm_resume(),
>> which also acquires xdna->dev_lock, resulting in a deadlock.
>>
>> Avoid the deadlock by calling amdxdna_pm_resume_get() before holding
>> xdna->dev_lock in both amdxdna_hwctx_sync_debug_bo() and
>> amdxdna_drm_config_hwctx_ioctl()
>>
>> Fixes: 7ea046838021 ("accel/amdxdna: Support firmware debug buffer")
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>
> Reviewed-by: Max Zhen <max.zhen@amd.com>
>
>> ---
>> V2:
>>    Fix jumping forward over the guard(mutex) declaration.
>> V3:
>>    Fix the same in amdxdna_drm_config_hwctx_ioctl().
>>
>>   drivers/accel/amdxdna/amdxdna_ctx.c | 26 ++++++++++++++++++++++----
>>   1 file changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c 
>> b/drivers/accel/amdxdna/amdxdna_ctx.c
>> index ff6c3e8e5a15..2ab4443e7094 100644
>> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
>> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c
>> @@ -356,16 +356,25 @@ int amdxdna_drm_config_hwctx_ioctl(struct 
>> drm_device *dev, void *data, struct dr
>>           return -EINVAL;
>>       }
>>   -    guard(mutex)(&xdna->dev_lock);
>> +    ret = amdxdna_pm_resume_get(xdna);
>> +    if (ret) {
>> +        XDNA_ERR(xdna, "Resume failed, ret %d", ret);
>> +        goto free_buf;
>> +    }
>> +
>> +    mutex_lock(&xdna->dev_lock);
>>       hwctx = xa_load(&client->hwctx_xa, args->handle);
>>       if (!hwctx) {
>>           XDNA_DBG(xdna, "PID %d failed to get hwctx %d", 
>> client->pid, args->handle);
>>           ret = -EINVAL;
>> -        goto free_buf;
>> +        goto unlock;
>>       }
>>         ret = xdna->dev_info->ops->hwctx_config(hwctx, 
>> args->param_type, val, buf, buf_size);
>>   +unlock:
>> +    mutex_unlock(&xdna->dev_lock);
>> +    amdxdna_pm_suspend_put(xdna);
>>   free_buf:
>>       kfree(buf);
>>       return ret;
>> @@ -386,16 +395,25 @@ int amdxdna_hwctx_sync_debug_bo(struct 
>> amdxdna_client *client, u32 debug_bo_hdl)
>>       if (!gobj)
>>           return -EINVAL;
>>   +    ret = amdxdna_pm_resume_get(xdna);
>> +    if (ret) {
>> +        XDNA_ERR(xdna, "Resume failed, ret %d", ret);
>> +        goto put_obj;
>> +    }
>> +
>>       abo = to_xdna_obj(gobj);
>> -    guard(mutex)(&xdna->dev_lock);
>> +    mutex_lock(&xdna->dev_lock);
>>       hwctx = xa_load(&client->hwctx_xa, abo->assigned_hwctx);
>>       if (!hwctx) {
>>           ret = -EINVAL;
>> -        goto put_obj;
>> +        goto unlock;
>>       }
>>         ret = xdna->dev_info->ops->hwctx_sync_debug_bo(hwctx, 
>> debug_bo_hdl);
>>   +unlock:
>> +    mutex_unlock(&xdna->dev_lock);
>> +    amdxdna_pm_suspend_put(xdna);
>>   put_obj:
>>       drm_gem_object_put(gobj);
>>       return ret;
>