Re: [PATCH] accel/amdxdna: make the debug BO command wait interruptible

[sashiko-bot@kernel.org]

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

New issues:
- [High] Interruptible wait causes software state to desynchronize from hardware, potentially leading to IOMMU faults or memory corruption.

Pre-existing issues:
- [High] Unresolved ABBA deadlock between `xdna->dev_lock` and the DRM TDR (Timeout Detection and Recovery) handler permanently wedges the device on a hardware hang.
--

commit ce5b5167020e9399e64c273e9bded5901cfbc67d
Author: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>

accel/amdxdna: make the debug BO command wait interruptible

This commit changes the aie2_cmd_wait() function to wait for debug BO
commands interruptibly instead of uninterruptibly. It propagates the return
code up to the callers to allow userspace processes blocked on a hardware
hang to be killed.

> diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
> index 54486960cbf5e..1f910ee4941c9 100644
> --- a/drivers/accel/amdxdna/aie2_ctx.c
> +++ b/drivers/accel/amdxdna/aie2_ctx.c
> @@ -897,17 +897,20 @@ static int aie2_hwctx_cu_config(struct amdxdna_hwctx *hwctx, void *buf, u32 size
>  	return ret;
>  }
>  
> -static void aie2_cmd_wait(struct amdxdna_hwctx *hwctx, u64 seq)
> +static int aie2_cmd_wait(struct amdxdna_hwctx *hwctx, u64 seq)
>  {
>  	struct dma_fence *out_fence = aie2_cmd_get_out_fence(hwctx, seq);
> +	long ret;
>  
>  	if (!out_fence) {
>  		XDNA_ERR(hwctx->client->xdna, "Failed to get fence");
> -		return;
> +		return -EINVAL;
>  	}
>  
> -	dma_fence_wait_timeout(out_fence, false, MAX_SCHEDULE_TIMEOUT);
> +	ret = dma_fence_wait_timeout(out_fence, true, MAX_SCHEDULE_TIMEOUT);
>  	dma_fence_put(out_fence);
> +
> +	return ret < 0 ? ret : 0;
>  }

[Severity: High]
This is a pre-existing issue, but does making the wait interruptible fully
resolve the underlying deadlock with the timeout handler?

While this prevents an unkillable task, the ioctl still holds xdna->dev_lock
across the wait:

amdxdna_drm_config_hwctx_ioctl()
   guard(mutex)(&xdna->dev_lock);
   ...
   aie2_hwctx_cfg_debug_bo()
      aie2_cmd_wait()
         dma_fence_wait_timeout()

If the firmware hangs, the DRM scheduler invokes aie2_sched_job_timedout()
to recover. However, aie2_sched_job_timedout() also requires xdna->dev_lock.
This deadlocks against the waiting ioctl thread, preventing automatic TDR
recovery unless the waiting process is manually killed.

>  static int aie2_hwctx_cfg_debug_bo(struct amdxdna_hwctx *hwctx, u32 bo_hdl,
> @@ -954,7 +957,10 @@ static int aie2_hwctx_cfg_debug_bo(struct amdxdna_hwctx *hwctx, u32 bo_hdl,
>  		goto put_cmd;
>  	}
>  
> -	aie2_cmd_wait(hwctx, seq);
> +	ret = aie2_cmd_wait(hwctx, seq);
> +	if (ret)
> +		goto put_cmd;
> +

[Severity: High]
If the wait is interrupted and returns an error, could this early return
cause the software state to desynchronize from the hardware?

By taking the goto put_cmd path, the function skips the state update later
in the function:

aie2_hwctx_cfg_debug_bo() {
    ...
    if (attach)
        abo->assigned_hwctx = hwctx->id;
    else
        abo->assigned_hwctx = AMDXDNA_INVALID_CTX_HANDLE;
    ...
}

If the hardware successfully processes the command but the software update
is skipped due to the interrupted wait, the driver will consider the BO
unattached.

Can this allow userspace to prematurely free the BO while the firmware still
has it mapped, leading to IOMMU faults or memory corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826150726.126296-1-taimuraz@kaitmazov.com?part=1