Re: [PATCH V1] accel/amdxdna: Support preemption requests
From: Mario Limonciello
Date: Tue Nov 04 2025 - 14:30:25 EST
On 11/4/25 1:28 PM, Lizhi Hou wrote:
Got it thanks. No other concerns.
On 11/4/25 10:58, Mario Limonciello wrote:
On 11/4/25 12:53 PM, Lizhi Hou wrote:
The driver checks the firmware version during initialization.If preemption
is supported, the driver configures preemption accordingly and handles
userspace preemption requests. Otherwise, the driver returns an error for
userspace preemption requests.
Signed-off-by: Lizhi Hou <lizhi.hou@xxxxxxx>
---
drivers/accel/amdxdna/aie2_message.c | 95 +++++++++++++++++++++++++
drivers/accel/amdxdna/aie2_msg_priv.h | 3 +
drivers/accel/amdxdna/aie2_pci.c | 63 ++++++++++++++++
drivers/accel/amdxdna/aie2_pci.h | 8 +++
drivers/accel/amdxdna/amdxdna_ctx.h | 17 +++++
drivers/accel/amdxdna/amdxdna_pci_drv.c | 3 +-
drivers/accel/amdxdna/npu4_regs.c | 4 ++
include/uapi/drm/amdxdna_accel.h | 16 ++++-
8 files changed, 207 insertions(+), 2 deletions(-)
diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/ amdxdna/aie2_message.c
index 69cdce9ff208..d493bb1c3360 100644
--- a/drivers/accel/amdxdna/aie2_message.c
+++ b/drivers/accel/amdxdna/aie2_message.c
@@ -210,6 +210,14 @@ int aie2_create_context(struct amdxdna_dev_hdl *ndev, struct amdxdna_hwctx *hwct
hwctx->fw_ctx_id = resp.context_id;
WARN_ONCE(hwctx->fw_ctx_id == -1, "Unexpected context id");
+ if (ndev->force_preempt_enabled) {
+ ret = aie2_runtime_cfg(ndev, AIE2_RT_CFG_FORCE_PREEMPT, &hwctx->fw_ctx_id);
+ if (ret) {
+ XDNA_ERR(xdna, "failed to enable force preempt %d", ret);
+ return ret;
+ }
+ }
+
cq_pair = &resp.cq_pair[0];
x2i.mb_head_ptr_reg = AIE2_MBOX_OFF(ndev, cq_pair- >x2i_q.head_addr);
x2i.mb_tail_ptr_reg = AIE2_MBOX_OFF(ndev, cq_pair- >x2i_q.tail_addr);
@@ -601,6 +609,11 @@ aie2_cmdlist_fill_dpu(struct amdxdna_gem_obj *cmd_bo, void *slot, size_t *size)
return 0;
}
+static int aie2_cmdlist_unsupp(struct amdxdna_gem_obj *cmd_bo, void *slot, size_t *size)
+{
+ return -EOPNOTSUPP;
+}
+
static u32 aie2_get_chain_msg_op(u32 cmd_op)
{
switch (cmd_op) {
@@ -621,6 +634,8 @@ static struct aie2_exec_msg_ops legacy_exec_message_ops = {
.init_chain_req = aie2_init_exec_chain_req,
.fill_cf_slot = aie2_cmdlist_fill_cf,
.fill_dpu_slot = aie2_cmdlist_fill_dpu,
+ .fill_preempt_slot = aie2_cmdlist_unsupp,
+ .fill_elf_slot = aie2_cmdlist_unsupp,
.get_chain_msg_op = aie2_get_chain_msg_op,
};
@@ -680,6 +695,74 @@ aie2_cmdlist_fill_npu_dpu(struct amdxdna_gem_obj *cmd_bo, void *slot, size_t *si
return 0;
}
+static int
+aie2_cmdlist_fill_npu_preempt(struct amdxdna_gem_obj *cmd_bo, void *slot, size_t *size)
+{
+ struct cmd_chain_slot_npu *npu_slot = slot;
+ struct amdxdna_cmd_preempt_data *pd;
+ u32 cmd_len;
+ u32 arg_sz;
+
+ pd = amdxdna_cmd_get_payload(cmd_bo, &cmd_len);
+ arg_sz = cmd_len - sizeof(*pd);
+ if (cmd_len < sizeof(*pd) || arg_sz > MAX_NPU_ARGS_SIZE)
+ return -EINVAL;
+
+ if (*size < sizeof(*npu_slot) + arg_sz)
+ return -EINVAL;
+
+ npu_slot->cu_idx = amdxdna_cmd_get_cu_idx(cmd_bo);
+ if (npu_slot->cu_idx == INVALID_CU_IDX)
+ return -EINVAL;
+
+ memset(npu_slot, 0, sizeof(*npu_slot));
+ npu_slot->type = EXEC_NPU_TYPE_PREEMPT;
+ npu_slot->inst_buf_addr = pd->inst_buf;
+ npu_slot->save_buf_addr = pd->save_buf;
+ npu_slot->restore_buf_addr = pd->restore_buf;
+ npu_slot->inst_size = pd->inst_size;
+ npu_slot->save_size = pd->save_size;
+ npu_slot->restore_size = pd->restore_size;
+ npu_slot->inst_prop_cnt = pd->inst_prop_cnt;
+ npu_slot->arg_cnt = arg_sz / sizeof(u32);
+ memcpy(npu_slot->args, pd->prop_args, arg_sz);
Am I following this right? I would think this should be:
memcpy(npu_slot->args, pd->prop_args, npu_slot->arg_cnt);
npu_slot->arg_cnt is the number of u32. So arg_sz is used for memcpy here.
Reviewed-by: Mario Limonciello (AMD) <superm1@xxxxxxxxxx>