Re: [PATCH V1] accel/amdxdna: Fix command timeout race

[Lizhi Hou <lizhi.hou@amd.com>]

Applied to drm-misc-fixes

On 7/19/26 15:29, Max Zhen wrote:
>
>
> On 7/18/2026 Sat 01:34, Lizhi Hou wrote:
>> From: Wendy Liang <wendy.liang@amd.com>
>>
>> When two commands enter aie2_sched_job_timedout() concurrently, both
>> check the timeout detection state. The first scheduler thread observes
>> tdr_status as SIGNALED and updates it to WAIT. The second thread then
>> observes the updated state instead of the original SIGNALED state, which
>> may cause the command timeout to be handled incorrectly.
>>
>> Replace tdr_status with last_signal_ts, which records the timestamp of
>> the last driver signal. Timeout detection now only reads
>> last_signal_ts and never modifies it, allowing multiple serialized
>> detect() calls under dev_lock to evaluate the same signal timestamp
>> independently. If there is not any new job scheduled or completed
>> within tdr_timeout_ms, the command will timeout.
>>
>> Fixes: 9022f010977f ("accel/amdxdna: Check for device hang on job 
>> timeout")
>> Signed-off-by: Wendy Liang <wendy.liang@amd.com>
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> Reviewed-by: Max Zhen <max.zhen@amd.com>
>> ---
>>   drivers/accel/amdxdna/aie2_ctx.c | 22 +++++++++++++++-------
>>   drivers/accel/amdxdna/aie2_pci.c |  1 +
>>   drivers/accel/amdxdna/aie2_pci.h |  7 +------
>>   3 files changed, 17 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/aie2_ctx.c 
>> b/drivers/accel/amdxdna/aie2_ctx.c
>> index 101f324ee178..94dfee7263bd 100644
>> --- a/drivers/accel/amdxdna/aie2_ctx.c
>> +++ b/drivers/accel/amdxdna/aie2_ctx.c
>> @@ -43,20 +43,22 @@ struct aie2_ctx_health {
>>     static inline void aie2_tdr_signal(struct amdxdna_dev *xdna)
>>   {
>> -    WRITE_ONCE(xdna->dev_handle->tdr_status, AIE2_TDR_SIGNALED);
>> +    WRITE_ONCE(xdna->dev_handle->last_signal_ts, jiffies);
>>   }
>>     static bool aie2_tdr_detect(struct amdxdna_dev *xdna)
>>   {
>>       struct amdxdna_dev_hdl *ndev = xdna->dev_handle;
>> +    unsigned long last = READ_ONCE(ndev->last_signal_ts);
>>   -    if (READ_ONCE(ndev->tdr_status) == AIE2_TDR_WAIT) {
>> -        XDNA_ERR(xdna, "TDR timeout detected");
>> -        return true;
>> -    }
>> +    if (!tdr_timeout_ms)
>> +        return false;
>> +
>> +    if (!time_after(jiffies, last + msecs_to_jiffies(tdr_timeout_ms)))
>> +        return false;
>>   -    WRITE_ONCE(ndev->tdr_status, AIE2_TDR_WAIT);
>> -    return false;
>> +    XDNA_ERR(xdna, "TDR timeout detected");
>> +    return true;
>>   }
>>     static void aie2_cmd_release(struct kref *ref)
>> @@ -434,6 +436,12 @@ aie2_sched_job_run(struct drm_sched_job *sched_job)
>>           mmput(job->mm);
>>           fence = ERR_PTR(ret);
>>       } else {
>> +        /*
>> +         * Command is successfully posted to hardware, update the
>> +         * tdr timestamp. The total pending commands are limited.
>> +         * So there will not be a case that driver keeps posting
>> +         * commands without getting any hardware respond.
>> +         */
>>           aie2_tdr_signal(hwctx->client->xdna);
>>       }
>>       trace_xdna_job(sched_job, hwctx->name, "sent to device",
>> diff --git a/drivers/accel/amdxdna/aie2_pci.c 
>> b/drivers/accel/amdxdna/aie2_pci.c
>> index 22f66c7f534d..daec1f6b4907 100644
>> --- a/drivers/accel/amdxdna/aie2_pci.c
>> +++ b/drivers/accel/amdxdna/aie2_pci.c
>> @@ -420,6 +420,7 @@ static int aie2_hw_start(struct amdxdna_dev *xdna)
>>           goto stop_fw;
>>       }
>>   +    WRITE_ONCE(ndev->last_signal_ts, jiffies);
>>       ndev->dev_status = AIE2_DEV_START;
>>         return 0;
>> diff --git a/drivers/accel/amdxdna/aie2_pci.h 
>> b/drivers/accel/amdxdna/aie2_pci.h
>> index 77648cc548b6..ea1dac106400 100644
>> --- a/drivers/accel/amdxdna/aie2_pci.h
>> +++ b/drivers/accel/amdxdna/aie2_pci.h
>> @@ -143,11 +143,6 @@ struct aie2_exec_msg_ops {
>>       u32 (*get_chain_msg_op)(u32 cmd_op);
>>   };
>>   -enum aie2_tdr_status {
>> -    AIE2_TDR_WAIT,
>> -    AIE2_TDR_SIGNALED,
>> -};
>> -
>>   struct amdxdna_dev_hdl {
>>       struct aie_device        aie;
>>       const struct amdxdna_dev_priv    *priv;
>> @@ -179,7 +174,7 @@ struct amdxdna_dev_hdl {
>>       u32                hwctx_num;
>>         struct amdxdna_async_error    last_async_err;
>> -    enum aie2_tdr_status        tdr_status;
>> +    unsigned long            last_signal_ts;
>>   };
>>     struct aie2_hw_ops {
>