From: David Zhang <yidong.zhang@amd.com>
To: <quic_jhugo@quicinc.com>, <karol.wachowski@linux.intel.com>,
<max.zhen@amd.com>, <lizhi.hou@amd.com>, <ogabbay@kernel.org>,
<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>
Cc: David Zhang <yidong.zhang@amd.com>, <sonal.santan@amd.com>,
<mario.limonciello@amd.com>, Wendy Liang <wendy.liang@amd.com>
Subject: [PATCH V0 15/21] accel/amdxdna: Prepare for AIE4 command submission
Date: Fri, 25 Sep 2026 18:34:42 -0700 [thread overview]
Message-ID: <20260926013448.3840921-16-yidong.zhang@amd.com> (raw)
In-Reply-To: <20260926013448.3840921-1-yidong.zhang@amd.com>
Prepare the data structures and completion wait helpers required for
AIE4 command submission:
- Define struct amdxdna_cmd_start_dpu in amdxdna_ctx.h for the
ERT_START_DPU payload.
- Extend union amdxdna_job_priv with an aie4 member for queue list
linkage and job state tracking.
- Implement smp_rmb() ordering and non-sleeping retry in get_read_index(),
returning the cached last_read_index on read tearing to prevent false
timeouts.
- Update check_cmd_done() and aie4_cmd_wait() to detect asynchronous
device disconnect and reset via check_cert_comp_linked().
Co-developed-by: Max Zhen <max.zhen@amd.com>
Signed-off-by: Max Zhen <max.zhen@amd.com>
Co-developed-by: Wendy Liang <wendy.liang@amd.com>
Signed-off-by: Wendy Liang <wendy.liang@amd.com>
Signed-off-by: David Zhang <yidong.zhang@amd.com>
---
drivers/accel/amdxdna/aie4_ctx.c | 89 ++++++++++++++++++++++++-----
drivers/accel/amdxdna/amdxdna_ctx.h | 20 +++++++
2 files changed, 95 insertions(+), 14 deletions(-)
diff --git a/drivers/accel/amdxdna/aie4_ctx.c b/drivers/accel/amdxdna/aie4_ctx.c
index af52a1ea45e8..ad124b6d4a02 100644
--- a/drivers/accel/amdxdna/aie4_ctx.c
+++ b/drivers/accel/amdxdna/aie4_ctx.c
@@ -421,34 +421,92 @@ static inline bool valid_queue_index(u64 read, u64 write, u32 capacity)
static u64 get_read_index(struct amdxdna_hwctx *hwctx)
{
- u64 wi = READ_ONCE(*hwctx->priv->umq_write_index);
- u64 ri = READ_ONCE(*hwctx->priv->umq_read_index);
+ struct amdxdna_hwctx_priv *priv = hwctx->priv;
struct amdxdna_dev *xdna = hwctx->client->xdna;
+ u64 ri, wi;
+
+ /*
+ * Sample read_index (written by CERT) before write_index. CERT can
+ * never complete more than has been published, so a write_index sampled
+ * after read_index always satisfies wi >= ri; sampling write_index
+ * first races the submit path / CERT and yields a bogus ri > wi.
+ *
+ * In kernel-mode submission write_index is the driver's host-owned copy
+ * in coherent kernel memory (always >= the value mirrored into the UMQ,
+ * and the device never writes it).
+ *
+ * Security: read_index lives in the umq_bo, which the owning process can
+ * map. Under PASID/SVA the device reaches the queue through that process's
+ * own page tables, preventing access to other contexts' unshared memory.
+ * A forged read_index only completes the process's own command early and
+ * corrupts or hangs itself; however, if BOs are exported via dma-buf,
+ * premature fence signaling can cause an importing process or device to
+ * observe DMA completion before CERT has finished processing.
+ */
+ ri = READ_ONCE(*priv->umq_read_index);
+ /* Order the read_index sample before the write_index sample. */
+ smp_rmb();
+ wi = READ_ONCE(priv->write_index);
/*
* CERT cannot update read index as uint64 atomically. Driver may read
- * half-updated read index when it has bits in high 32bit. In case read
- * index is not valid, wait for some time and retry once. It should
- * allow CERT to complete the read index update.
+ * a half-updated read index when it has bits in the high 32 bits. If it
+ * looks invalid, re-sample once -- WITHOUT sleeping, since this can run as
+ * a wait_event() condition. If still invalid, report not-advanced; the
+ * waiter re-checks on the next completion wake or timeout.
*/
if (!valid_queue_index(ri, wi, CTX_MAX_CMDS)) {
- XDNA_WARN(xdna, "Invalid index, ri %llu, wi %llu", ri, wi);
- usleep_range(100, 200);
- ri = READ_ONCE(*hwctx->priv->umq_read_index);
+ ri = READ_ONCE(*priv->umq_read_index);
+ /* Order the read_index sample before the write_index sample. */
+ smp_rmb();
+ wi = READ_ONCE(priv->write_index);
if (!valid_queue_index(ri, wi, CTX_MAX_CMDS)) {
- XDNA_ERR(xdna, "Invalid index after retry, ri %llu, wi %llu", ri, wi);
- ri = 0;
+ /*
+ * Still invalid (torn 64-bit read, or a transient
+ * accounting skew). Return the last valid read_index
+ * instead of 0: read_index only advances, so the cached
+ * value is a safe lower bound -- it never reports a
+ * command complete that isn't, and never regresses the
+ * worker into falsely timing out a finished job.
+ */
+ XDNA_DBG(xdna, "Invalid index, ri %llu, wi %llu", ri, wi);
+ return READ_ONCE(priv->last_read_index);
}
}
+ WRITE_ONCE(priv->last_read_index, ri);
return ri;
}
-static inline bool check_cmd_done(struct amdxdna_hwctx *hwctx, u64 seq)
+/*
+ * The ctx is "connected" as long as @comp is still the cert_comp linked to it.
+ * A disconnect (teardown/reset) unlinks (and may re-link a fresh) cert_comp, so
+ * a changed pointer means the caller must retry (-EAGAIN). This runs as a
+ * wait_event() condition on the completion hot path (cert_comp->waitq is shared
+ * per MSI-X), so keep it lockless: the caller pins @comp with a kref, making
+ * this a pure pointer-identity compare - never a dereference, ABA-safe - and
+ * READ_ONCE pairs with the WRITE_ONCE in aie4_hwctx_create()/
+ * aie4_hwctx_destroy().
+ */
+static bool check_cert_comp_linked(struct amdxdna_hwctx *hwctx, struct cert_comp *comp)
{
- u64 read_idx = get_read_index(hwctx);
+ /* READ_ONCE pairs with the link/unlink WRITE_ONCE. */
+ return comp == READ_ONCE(hwctx->priv->cert_comp);
+}
+
+static inline bool check_cmd_done(struct amdxdna_hwctx *hwctx, u64 seq, struct cert_comp *comp)
+{
+ /*
+ * Runs as a wait_event() condition, so it must not sleep.
+ * check_cert_comp_linked() is lockless (a READ_ONCE pointer compare); a
+ * disconnect (teardown/reset) unlinks @comp and breaks the wait, and the
+ * caller then confirms real completion by re-reading read_index, so a
+ * disconnect wake is not mistaken for success.
+ */
+ if (!check_cert_comp_linked(hwctx, comp))
+ return true;
- return read_idx > seq;
+ return get_read_index(hwctx) > seq;
}
int aie4_cmd_wait(struct amdxdna_hwctx *hwctx, u64 seq, u32 timeout)
@@ -464,11 +522,14 @@ int aie4_cmd_wait(struct amdxdna_hwctx *hwctx, u64 seq, u32 timeout)
wait_jifs = msecs_to_jiffies(timeout);
ret = wait_event_interruptible_timeout(cert_comp->waitq,
- (check_cmd_done(hwctx, seq)),
+ check_cmd_done(hwctx, seq, cert_comp),
wait_jifs);
if (!ret)
ret = -ETIME;
+ else if (ret > 0 && get_read_index(hwctx) <= seq)
+ /* Woke on disconnect/reset, not on real completion. */
+ ret = -EAGAIN;
aie4_put_cert_comp(cert_comp);
diff --git a/drivers/accel/amdxdna/amdxdna_ctx.h b/drivers/accel/amdxdna/amdxdna_ctx.h
index 1529e7507fed..7ff806297f8c 100644
--- a/drivers/accel/amdxdna/amdxdna_ctx.h
+++ b/drivers/accel/amdxdna/amdxdna_ctx.h
@@ -48,6 +48,18 @@ struct amdxdna_cmd_start_npu {
u32 prop_args[]; /* properties and regular kernel arguments */
};
+/*
+ * struct amdxdna_cmd_start_dpu - interpretation of data payload for
+ * ERT_START_DPU in amdxdna_cmd.
+ */
+struct amdxdna_cmd_start_dpu {
+ u64 dtrace_buffer; /* dtrace buffer address 2 words */
+ u64 instruction_buffer; /* buffer address 2 words */
+ u32 instruction_buffer_size; /* size of buffer in bytes */
+ u16 uc_index; /* microblaze controller index */
+ u16 chained; /* number of following amdxdna_cmd_start_dpu elements */
+};
+
/*
* Interpretation of the beginning of data payload for ERT_CMD_CHAIN in
* amdxdna_cmd. The rest of the payload in amdxdna_cmd is cmd BO handles.
@@ -138,8 +150,14 @@ struct amdxdna_drv_cmd {
};
struct app_health_report;
+
union amdxdna_job_priv {
struct app_health_report *aie2_health;
+ /* aie4 kernel submission: queue linkage + job state */
+ struct {
+ struct list_head list;
+ u32 state;
+ } aie4;
};
struct amdxdna_sched_job {
@@ -162,6 +180,8 @@ struct amdxdna_sched_job {
};
#define aie2_job_health priv.aie2_health
+#define aie4_job_list priv.aie4.list
+#define aie4_job_state priv.aie4.state
static inline u32
amdxdna_cmd_get_op(struct amdxdna_gem_obj *abo)
--
2.34.1
next prev parent reply other threads:[~2026-09-26 1:35 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-26 1:34 [PATCH V0 00/21] accel/amdxdna: Kernel submission and PM for AIE4 David Zhang
2026-09-26 1:34 ` [PATCH V0 01/21] accel/amdxdna: Rename NPU3 firmware files David Zhang
2026-09-26 1:34 ` [PATCH V0 02/21] accel/amdxdna: Remove mmap for doorbell David Zhang
2026-09-26 1:34 ` [PATCH V0 03/21] accel/amdxdna: Add CERT firmware version support David Zhang
2026-09-26 1:34 ` [PATCH V0 04/21] accel/amdxdna: Upgrade firmware version to 6.0 David Zhang
2026-09-28 17:08 ` Lizhi Hou
2026-09-26 1:34 ` [PATCH V0 05/21] accel/amdxdna: Add NPU3 classic device support David Zhang
2026-09-26 1:34 ` [PATCH V0 06/21] accel/amdxdna: Add AIE version query to aie4_get_info David Zhang
2026-09-26 1:34 ` [PATCH V0 07/21] accel/amdxdna: Add get and set power_mode for AIE4 David Zhang
2026-09-28 18:39 ` Lizhi Hou
2026-09-26 1:34 ` [PATCH V0 08/21] accel/amdxdna: Restore power mode override on AIE4 hardware start David Zhang
2026-09-28 18:48 ` Lizhi Hou
2026-09-26 1:34 ` [PATCH V0 09/21] accel/amdxdna: Add clock, DPM frequency, and resource info queries for AIE4 David Zhang
2026-09-26 1:34 ` [PATCH V0 10/21] accel/amdxdna: Add context switch hysteresis with debugfs control David Zhang
2026-09-26 1:34 ` [PATCH V0 11/21] accel/amdxdna: Refactor AIE4 hardware initialization sequence David Zhang
2026-09-26 1:34 ` [PATCH V0 12/21] accel/amdxdna: Decouple AIE4 doorbell and MSI-X notification transport hooks David Zhang
2026-09-28 20:16 ` Lizhi Hou
2026-09-26 1:34 ` [PATCH V0 13/21] accel/amdxdna: Implement AIE4 kernel queue lifecycle and memory layout David Zhang
2026-09-26 1:34 ` [PATCH V0 14/21] accel/amdxdna: Fix fence timeline name and context allocation David Zhang
2026-09-28 21:06 ` Lizhi Hou
2026-09-30 0:20 ` Zhang, Yidong (David)
2026-09-26 1:34 ` David Zhang [this message]
2026-09-26 1:34 ` [PATCH V0 16/21] accel/amdxdna: Implement AIE4 command packet building and submission David Zhang
2026-09-26 1:34 ` [PATCH V0 17/21] accel/amdxdna: Finalize runtime PM before acquiring dev_lock on removal David Zhang
2026-09-26 1:34 ` [PATCH V0 18/21] accel/amdxdna: Implement AIE4 suspend and resume David Zhang
2026-09-26 1:34 ` [PATCH V0 19/21] accel/amdxdna: Link SR-IOV VFs for power management sequencing David Zhang
2026-09-26 1:34 ` [PATCH V0 20/21] accel/amdxdna: Implement runtime suspend and resume support David Zhang
2026-09-26 1:34 ` [PATCH V0 21/21] accel/amdxdna: Add stub hwctx_config for AIE4 David Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260926013448.3840921-16-yidong.zhang@amd.com \
--to=yidong.zhang@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=karol.wachowski@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lizhi.hou@amd.com \
--cc=mario.limonciello@amd.com \
--cc=max.zhen@amd.com \
--cc=ogabbay@kernel.org \
--cc=quic_jhugo@quicinc.com \
--cc=sonal.santan@amd.com \
--cc=wendy.liang@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®