accel/amdxdna: Fix command timeout race

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>
Reviewed-by: Max Zhen <max.zhen@amd.com>
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Link: https://patch.msgid.link/20260718083409.1825940-1-lizhi.hou@amd.com

diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
index 101f324ee1787ffa2c707fd2e341c3b65cb925a0..4b3a62aa87983a7cf17daf179202e9eabc033ca4 100644 (file)
--- 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 @@ out:
                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",
@@ -658,7 +666,9 @@ int aie2_hwctx_init(struct amdxdna_hwctx *hwctx)
        const struct drm_sched_init_args args = {
                .ops = &sched_ops,
                .credit_limit = HWCTX_MAX_CMDS,
-               .timeout = msecs_to_jiffies(tdr_timeout_ms),
+               .timeout = tdr_timeout_ms ?
+                       msecs_to_jiffies(tdr_timeout_ms) :
+                       MAX_SCHEDULE_TIMEOUT,
                .name = "amdxdna_js",
                .dev = xdna->ddev.dev,
        };

diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
index 22f66c7f534d42927d50ccae8f06ceb736c5d504..daec1f6b490797183fa4e732f25b8104f124d0a3 100644 (file)
--- 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 77648cc548b661a1233a138d915c8fd9c8cb4d90..ea1dac10640042eedfa0856524a2a8d06971877e 100644 (file)
--- 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 {