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

[Lizhi Hou]

From: Wendy Liang <wendy.liang@xxxxxxx>

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@xxxxxxx>
Signed-off-by: Lizhi Hou <lizhi.hou@xxxxxxx>
---
 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 {
-- 
2.34.1