From 89ff45359abbf9d8d3c4aa3f5a57ed0be82b5a12 Mon Sep 17 00:00:00 2001 From: Lizhi Hou Date: Thu, 26 Feb 2026 16:48:41 -0800 Subject: accel/amdxdna: Fill invalid payload for failed command Newer userspace applications may read the payload of a failed command to obtain detailed error information. However, the driver and old firmware versions may not support returning advanced error information. In this case, initialize the command payload with an invalid value so userspace can detect that no detailed error information is available. Fixes: aac243092b70 ("accel/amdxdna: Add command execution") Reviewed-by: Mario Limonciello (AMD) Signed-off-by: Lizhi Hou Link: https://patch.msgid.link/20260227004841.3080241-1-lizhi.hou@amd.com --- drivers/accel/amdxdna/aie2_ctx.c | 23 ++++++++--------------- drivers/accel/amdxdna/amdxdna_ctx.c | 27 +++++++++++++++++++++++++++ drivers/accel/amdxdna/amdxdna_ctx.h | 3 +++ 3 files changed, 38 insertions(+), 15 deletions(-) (limited to 'drivers/accel') diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c index 25845bd5e507..afee5e667f77 100644 --- a/drivers/accel/amdxdna/aie2_ctx.c +++ b/drivers/accel/amdxdna/aie2_ctx.c @@ -186,13 +186,13 @@ aie2_sched_resp_handler(void *handle, void __iomem *data, size_t size) cmd_abo = job->cmd_bo; if (unlikely(job->job_timeout)) { - amdxdna_cmd_set_state(cmd_abo, ERT_CMD_STATE_TIMEOUT); + amdxdna_cmd_set_error(cmd_abo, job, 0, ERT_CMD_STATE_TIMEOUT); ret = -EINVAL; goto out; } if (unlikely(!data) || unlikely(size != sizeof(u32))) { - amdxdna_cmd_set_state(cmd_abo, ERT_CMD_STATE_ABORT); + amdxdna_cmd_set_error(cmd_abo, job, 0, ERT_CMD_STATE_ABORT); ret = -EINVAL; goto out; } @@ -202,7 +202,7 @@ aie2_sched_resp_handler(void *handle, void __iomem *data, size_t size) if (status == AIE2_STATUS_SUCCESS) amdxdna_cmd_set_state(cmd_abo, ERT_CMD_STATE_COMPLETED); else - amdxdna_cmd_set_state(cmd_abo, ERT_CMD_STATE_ERROR); + amdxdna_cmd_set_error(cmd_abo, job, 0, ERT_CMD_STATE_ERROR); out: aie2_sched_notify(job); @@ -244,13 +244,13 @@ aie2_sched_cmdlist_resp_handler(void *handle, void __iomem *data, size_t size) cmd_abo = job->cmd_bo; if (unlikely(job->job_timeout)) { - amdxdna_cmd_set_state(cmd_abo, ERT_CMD_STATE_TIMEOUT); + amdxdna_cmd_set_error(cmd_abo, job, 0, ERT_CMD_STATE_TIMEOUT); ret = -EINVAL; goto out; } if (unlikely(!data) || unlikely(size != sizeof(u32) * 3)) { - amdxdna_cmd_set_state(cmd_abo, ERT_CMD_STATE_ABORT); + amdxdna_cmd_set_error(cmd_abo, job, 0, ERT_CMD_STATE_ABORT); ret = -EINVAL; goto out; } @@ -270,19 +270,12 @@ aie2_sched_cmdlist_resp_handler(void *handle, void __iomem *data, size_t size) fail_cmd_idx, fail_cmd_status); if (fail_cmd_status == AIE2_STATUS_SUCCESS) { - amdxdna_cmd_set_state(cmd_abo, ERT_CMD_STATE_ABORT); + amdxdna_cmd_set_error(cmd_abo, job, fail_cmd_idx, ERT_CMD_STATE_ABORT); ret = -EINVAL; - goto out; + } else { + amdxdna_cmd_set_error(cmd_abo, job, fail_cmd_idx, ERT_CMD_STATE_ERROR); } - amdxdna_cmd_set_state(cmd_abo, ERT_CMD_STATE_ERROR); - if (amdxdna_cmd_get_op(cmd_abo) == ERT_CMD_CHAIN) { - struct amdxdna_cmd_chain *cc = amdxdna_cmd_get_payload(cmd_abo, NULL); - - cc->error_index = fail_cmd_idx; - if (cc->error_index >= cc->command_count) - cc->error_index = 0; - } out: aie2_sched_notify(job); return ret; diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c index 263d36072540..666dfd7b2a80 100644 --- a/drivers/accel/amdxdna/amdxdna_ctx.c +++ b/drivers/accel/amdxdna/amdxdna_ctx.c @@ -135,6 +135,33 @@ u32 amdxdna_cmd_get_cu_idx(struct amdxdna_gem_obj *abo) return INVALID_CU_IDX; } +int amdxdna_cmd_set_error(struct amdxdna_gem_obj *abo, + struct amdxdna_sched_job *job, u32 cmd_idx, + enum ert_cmd_state error_state) +{ + struct amdxdna_client *client = job->hwctx->client; + struct amdxdna_cmd *cmd = abo->mem.kva; + struct amdxdna_cmd_chain *cc = NULL; + + cmd->header &= ~AMDXDNA_CMD_STATE; + cmd->header |= FIELD_PREP(AMDXDNA_CMD_STATE, error_state); + + if (amdxdna_cmd_get_op(abo) == ERT_CMD_CHAIN) { + cc = amdxdna_cmd_get_payload(abo, NULL); + cc->error_index = (cmd_idx < cc->command_count) ? cmd_idx : 0; + abo = amdxdna_gem_get_obj(client, cc->data[0], AMDXDNA_BO_CMD); + if (!abo) + return -EINVAL; + cmd = abo->mem.kva; + } + + memset(cmd->data, 0xff, abo->mem.size - sizeof(*cmd)); + if (cc) + amdxdna_gem_put_obj(abo); + + return 0; +} + /* * This should be called in close() and remove(). DO NOT call in other syscalls. * This guarantee that when hwctx and resources will be released, if user diff --git a/drivers/accel/amdxdna/amdxdna_ctx.h b/drivers/accel/amdxdna/amdxdna_ctx.h index 16c85f08f03c..fbdf9d000871 100644 --- a/drivers/accel/amdxdna/amdxdna_ctx.h +++ b/drivers/accel/amdxdna/amdxdna_ctx.h @@ -167,6 +167,9 @@ amdxdna_cmd_get_state(struct amdxdna_gem_obj *abo) void *amdxdna_cmd_get_payload(struct amdxdna_gem_obj *abo, u32 *size); u32 amdxdna_cmd_get_cu_idx(struct amdxdna_gem_obj *abo); +int amdxdna_cmd_set_error(struct amdxdna_gem_obj *abo, + struct amdxdna_sched_job *job, u32 cmd_idx, + enum ert_cmd_state error_state); void amdxdna_sched_job_cleanup(struct amdxdna_sched_job *job); void amdxdna_hwctx_remove_all(struct amdxdna_client *client); -- cgit v1.2.3 From 6270ee26e1edd862ea17e3eba148ca8fb2c99dc9 Mon Sep 17 00:00:00 2001 From: Lizhi Hou Date: Thu, 26 Feb 2026 13:38:57 -0800 Subject: accel/amdxdna: Fix NULL pointer dereference of mgmt_chann mgmt_chann may be set to NULL if the firmware returns an unexpected error in aie2_send_mgmt_msg_wait(). This can later lead to a NULL pointer dereference in aie2_hw_stop(). Fix this by introducing a dedicated helper to destroy mgmt_chann and by adding proper NULL checks before accessing it. Fixes: b87f920b9344 ("accel/amdxdna: Support hardware mailbox") Reviewed-by: Mario Limonciello (AMD) Signed-off-by: Lizhi Hou Link: https://patch.msgid.link/20260226213857.3068474-1-lizhi.hou@amd.com --- drivers/accel/amdxdna/aie2_message.c | 21 ++++++++++++++++----- drivers/accel/amdxdna/aie2_pci.c | 7 ++----- drivers/accel/amdxdna/aie2_pci.h | 1 + 3 files changed, 19 insertions(+), 10 deletions(-) (limited to 'drivers/accel') diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c index 277a27bce850..22e1a85a7ae0 100644 --- a/drivers/accel/amdxdna/aie2_message.c +++ b/drivers/accel/amdxdna/aie2_message.c @@ -40,11 +40,8 @@ static int aie2_send_mgmt_msg_wait(struct amdxdna_dev_hdl *ndev, return -ENODEV; ret = xdna_send_msg_wait(xdna, ndev->mgmt_chann, msg); - if (ret == -ETIME) { - xdna_mailbox_stop_channel(ndev->mgmt_chann); - xdna_mailbox_destroy_channel(ndev->mgmt_chann); - ndev->mgmt_chann = NULL; - } + if (ret == -ETIME) + aie2_destroy_mgmt_chann(ndev); if (!ret && *hdl->status != AIE2_STATUS_SUCCESS) { XDNA_ERR(xdna, "command opcode 0x%x failed, status 0x%x", @@ -914,6 +911,20 @@ void aie2_msg_init(struct amdxdna_dev_hdl *ndev) ndev->exec_msg_ops = &legacy_exec_message_ops; } +void aie2_destroy_mgmt_chann(struct amdxdna_dev_hdl *ndev) +{ + struct amdxdna_dev *xdna = ndev->xdna; + + drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock)); + + if (!ndev->mgmt_chann) + return; + + xdna_mailbox_stop_channel(ndev->mgmt_chann); + xdna_mailbox_destroy_channel(ndev->mgmt_chann); + ndev->mgmt_chann = NULL; +} + static inline struct amdxdna_gem_obj * aie2_cmdlist_get_cmd_buf(struct amdxdna_sched_job *job) { diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c index 85079b6fc5d9..977ce21eaf9f 100644 --- a/drivers/accel/amdxdna/aie2_pci.c +++ b/drivers/accel/amdxdna/aie2_pci.c @@ -330,9 +330,7 @@ static void aie2_hw_stop(struct amdxdna_dev *xdna) aie2_runtime_cfg(ndev, AIE2_RT_CFG_CLK_GATING, NULL); aie2_mgmt_fw_fini(ndev); - xdna_mailbox_stop_channel(ndev->mgmt_chann); - xdna_mailbox_destroy_channel(ndev->mgmt_chann); - ndev->mgmt_chann = NULL; + aie2_destroy_mgmt_chann(ndev); drmm_kfree(&xdna->ddev, ndev->mbox); ndev->mbox = NULL; aie2_psp_stop(ndev->psp_hdl); @@ -441,8 +439,7 @@ static int aie2_hw_start(struct amdxdna_dev *xdna) return 0; destroy_mgmt_chann: - xdna_mailbox_stop_channel(ndev->mgmt_chann); - xdna_mailbox_destroy_channel(ndev->mgmt_chann); + aie2_destroy_mgmt_chann(ndev); stop_psp: aie2_psp_stop(ndev->psp_hdl); fini_smu: diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h index b20a3661078c..e72311c77996 100644 --- a/drivers/accel/amdxdna/aie2_pci.h +++ b/drivers/accel/amdxdna/aie2_pci.h @@ -303,6 +303,7 @@ int aie2_get_array_async_error(struct amdxdna_dev_hdl *ndev, /* aie2_message.c */ void aie2_msg_init(struct amdxdna_dev_hdl *ndev); +void aie2_destroy_mgmt_chann(struct amdxdna_dev_hdl *ndev); int aie2_suspend_fw(struct amdxdna_dev_hdl *ndev); int aie2_resume_fw(struct amdxdna_dev_hdl *ndev); int aie2_set_runtime_cfg(struct amdxdna_dev_hdl *ndev, u32 type, u64 value); -- cgit v1.2.3 From f82859c84a9862e242c904e72fb0625fef9b24e7 Mon Sep 17 00:00:00 2001 From: Lizhi Hou Date: Wed, 4 Mar 2026 11:50:12 -0800 Subject: accel/amdxdna: Fix major version check on NPU1 platform Add the missing major number in npu1_fw_feature_table. Without the major version specified, the firmware feature check fails, preventing new firmware commands from being enabled on the NPU1 platform. With the correct major version populated, the driver properly detects firmware support and enables the new command. Fixes: f1eac46fe5f7 ("accel/amdxdna: Update firmware version check for latest firmware") Reviewed-by: Mario Limonciello (AMD) Signed-off-by: Lizhi Hou Link: https://patch.msgid.link/20260304195012.3616908-1-lizhi.hou@amd.com --- drivers/accel/amdxdna/npu1_regs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/accel') diff --git a/drivers/accel/amdxdna/npu1_regs.c b/drivers/accel/amdxdna/npu1_regs.c index 6e3d3ca69c04..1320e924e548 100644 --- a/drivers/accel/amdxdna/npu1_regs.c +++ b/drivers/accel/amdxdna/npu1_regs.c @@ -67,7 +67,7 @@ const struct dpm_clk_freq npu1_dpm_clk_table[] = { static const struct aie2_fw_feature_tbl npu1_fw_feature_table[] = { { .major = 5, .min_minor = 7 }, - { .features = BIT_U64(AIE2_NPU_COMMAND), .min_minor = 8 }, + { .features = BIT_U64(AIE2_NPU_COMMAND), .major = 5, .min_minor = 8 }, { 0 } }; -- cgit v1.2.3 From d5b8b0347fa8470b751a506fb801797e271d7548 Mon Sep 17 00:00:00 2001 From: Lizhi Hou Date: Wed, 4 Mar 2026 22:20:41 -0800 Subject: accel/amdxdna: Split mailbox channel create function The management channel used for firmware control command submission is currently created after the firmware is started. If channel creation fails (for example, due to memory allocation failure or workqueue creation interruption), the firmware remains in a pending state and is unable to receive any control commands. To avoid leaving the firmware in this inconsistent state, split xdna_mailbox_create_channel() into two separate functions so that resource allocation can be completed before interacting with the hardware. xdna_mailbox_alloc_channel() Allocates memory and initializes the workqueue. This can be called earlier, before interacting with the hardware. xdna_mailbox_start_channel() Performs the hardware interaction required to start the channel. Rename xdna_mailbox_destroy_channel() to xdna_mailbox_free_channel(). Ensure that xdna_mailbox_stop_channel() and xdna_mailbox_free_channel() properly unwind the corresponding start and allocation steps, respectively. Fixes: b87f920b9344 ("accel/amdxdna: Support hardware mailbox") Reviewed-by: Mario Limonciello (AMD) Signed-off-by: Lizhi Hou Link: https://patch.msgid.link/20260305062041.3954024-1-lizhi.hou@amd.com --- drivers/accel/amdxdna/aie2_message.c | 17 ++++-- drivers/accel/amdxdna/aie2_pci.c | 63 +++++++++++++---------- drivers/accel/amdxdna/amdxdna_mailbox.c | 91 ++++++++++++++++----------------- drivers/accel/amdxdna/amdxdna_mailbox.h | 31 ++++++----- 4 files changed, 112 insertions(+), 90 deletions(-) (limited to 'drivers/accel') diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c index 22e1a85a7ae0..ffcf3be79e23 100644 --- a/drivers/accel/amdxdna/aie2_message.c +++ b/drivers/accel/amdxdna/aie2_message.c @@ -293,13 +293,20 @@ int aie2_create_context(struct amdxdna_dev_hdl *ndev, struct amdxdna_hwctx *hwct } intr_reg = i2x.mb_head_ptr_reg + 4; - hwctx->priv->mbox_chann = xdna_mailbox_create_channel(ndev->mbox, &x2i, &i2x, - intr_reg, ret); + hwctx->priv->mbox_chann = xdna_mailbox_alloc_channel(ndev->mbox); if (!hwctx->priv->mbox_chann) { XDNA_ERR(xdna, "Not able to create channel"); ret = -EINVAL; goto del_ctx_req; } + + ret = xdna_mailbox_start_channel(hwctx->priv->mbox_chann, &x2i, &i2x, + intr_reg, ret); + if (ret) { + XDNA_ERR(xdna, "Not able to create channel"); + ret = -EINVAL; + goto free_channel; + } ndev->hwctx_num++; XDNA_DBG(xdna, "Mailbox channel irq: %d, msix_id: %d", ret, resp.msix_id); @@ -307,6 +314,8 @@ int aie2_create_context(struct amdxdna_dev_hdl *ndev, struct amdxdna_hwctx *hwct return 0; +free_channel: + xdna_mailbox_free_channel(hwctx->priv->mbox_chann); del_ctx_req: aie2_destroy_context_req(ndev, hwctx->fw_ctx_id); return ret; @@ -322,7 +331,7 @@ int aie2_destroy_context(struct amdxdna_dev_hdl *ndev, struct amdxdna_hwctx *hwc xdna_mailbox_stop_channel(hwctx->priv->mbox_chann); ret = aie2_destroy_context_req(ndev, hwctx->fw_ctx_id); - xdna_mailbox_destroy_channel(hwctx->priv->mbox_chann); + xdna_mailbox_free_channel(hwctx->priv->mbox_chann); XDNA_DBG(xdna, "Destroyed fw ctx %d", hwctx->fw_ctx_id); hwctx->priv->mbox_chann = NULL; hwctx->fw_ctx_id = -1; @@ -921,7 +930,7 @@ void aie2_destroy_mgmt_chann(struct amdxdna_dev_hdl *ndev) return; xdna_mailbox_stop_channel(ndev->mgmt_chann); - xdna_mailbox_destroy_channel(ndev->mgmt_chann); + xdna_mailbox_free_channel(ndev->mgmt_chann); ndev->mgmt_chann = NULL; } diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c index 977ce21eaf9f..4924a9da55b6 100644 --- a/drivers/accel/amdxdna/aie2_pci.c +++ b/drivers/accel/amdxdna/aie2_pci.c @@ -361,10 +361,29 @@ static int aie2_hw_start(struct amdxdna_dev *xdna) } pci_set_master(pdev); + mbox_res.ringbuf_base = ndev->sram_base; + mbox_res.ringbuf_size = pci_resource_len(pdev, xdna->dev_info->sram_bar); + mbox_res.mbox_base = ndev->mbox_base; + mbox_res.mbox_size = MBOX_SIZE(ndev); + mbox_res.name = "xdna_mailbox"; + ndev->mbox = xdnam_mailbox_create(&xdna->ddev, &mbox_res); + if (!ndev->mbox) { + XDNA_ERR(xdna, "failed to create mailbox device"); + ret = -ENODEV; + goto disable_dev; + } + + ndev->mgmt_chann = xdna_mailbox_alloc_channel(ndev->mbox); + if (!ndev->mgmt_chann) { + XDNA_ERR(xdna, "failed to alloc channel"); + ret = -ENODEV; + goto disable_dev; + } + ret = aie2_smu_init(ndev); if (ret) { XDNA_ERR(xdna, "failed to init smu, ret %d", ret); - goto disable_dev; + goto free_channel; } ret = aie2_psp_start(ndev->psp_hdl); @@ -379,18 +398,6 @@ static int aie2_hw_start(struct amdxdna_dev *xdna) goto stop_psp; } - mbox_res.ringbuf_base = ndev->sram_base; - mbox_res.ringbuf_size = pci_resource_len(pdev, xdna->dev_info->sram_bar); - mbox_res.mbox_base = ndev->mbox_base; - mbox_res.mbox_size = MBOX_SIZE(ndev); - mbox_res.name = "xdna_mailbox"; - ndev->mbox = xdnam_mailbox_create(&xdna->ddev, &mbox_res); - if (!ndev->mbox) { - XDNA_ERR(xdna, "failed to create mailbox device"); - ret = -ENODEV; - goto stop_psp; - } - mgmt_mb_irq = pci_irq_vector(pdev, ndev->mgmt_chan_idx); if (mgmt_mb_irq < 0) { ret = mgmt_mb_irq; @@ -399,13 +406,13 @@ static int aie2_hw_start(struct amdxdna_dev *xdna) } xdna_mailbox_intr_reg = ndev->mgmt_i2x.mb_head_ptr_reg + 4; - ndev->mgmt_chann = xdna_mailbox_create_channel(ndev->mbox, - &ndev->mgmt_x2i, - &ndev->mgmt_i2x, - xdna_mailbox_intr_reg, - mgmt_mb_irq); - if (!ndev->mgmt_chann) { - XDNA_ERR(xdna, "failed to create management mailbox channel"); + ret = xdna_mailbox_start_channel(ndev->mgmt_chann, + &ndev->mgmt_x2i, + &ndev->mgmt_i2x, + xdna_mailbox_intr_reg, + mgmt_mb_irq); + if (ret) { + XDNA_ERR(xdna, "failed to start management mailbox channel"); ret = -EINVAL; goto stop_psp; } @@ -413,37 +420,41 @@ static int aie2_hw_start(struct amdxdna_dev *xdna) ret = aie2_mgmt_fw_init(ndev); if (ret) { XDNA_ERR(xdna, "initial mgmt firmware failed, ret %d", ret); - goto destroy_mgmt_chann; + goto stop_fw; } ret = aie2_pm_init(ndev); if (ret) { XDNA_ERR(xdna, "failed to init pm, ret %d", ret); - goto destroy_mgmt_chann; + goto stop_fw; } ret = aie2_mgmt_fw_query(ndev); if (ret) { XDNA_ERR(xdna, "failed to query fw, ret %d", ret); - goto destroy_mgmt_chann; + goto stop_fw; } ret = aie2_error_async_events_alloc(ndev); if (ret) { XDNA_ERR(xdna, "Allocate async events failed, ret %d", ret); - goto destroy_mgmt_chann; + goto stop_fw; } ndev->dev_status = AIE2_DEV_START; return 0; -destroy_mgmt_chann: - aie2_destroy_mgmt_chann(ndev); +stop_fw: + aie2_suspend_fw(ndev); + xdna_mailbox_stop_channel(ndev->mgmt_chann); stop_psp: aie2_psp_stop(ndev->psp_hdl); fini_smu: aie2_smu_fini(ndev); +free_channel: + xdna_mailbox_free_channel(ndev->mgmt_chann); + ndev->mgmt_chann = NULL; disable_dev: pci_disable_device(pdev); diff --git a/drivers/accel/amdxdna/amdxdna_mailbox.c b/drivers/accel/amdxdna/amdxdna_mailbox.c index 235a94047530..46d844a73a94 100644 --- a/drivers/accel/amdxdna/amdxdna_mailbox.c +++ b/drivers/accel/amdxdna/amdxdna_mailbox.c @@ -460,26 +460,49 @@ msg_id_failed: return ret; } -struct mailbox_channel * -xdna_mailbox_create_channel(struct mailbox *mb, - const struct xdna_mailbox_chann_res *x2i, - const struct xdna_mailbox_chann_res *i2x, - u32 iohub_int_addr, - int mb_irq) +struct mailbox_channel *xdna_mailbox_alloc_channel(struct mailbox *mb) { struct mailbox_channel *mb_chann; - int ret; - - if (!is_power_of_2(x2i->rb_size) || !is_power_of_2(i2x->rb_size)) { - pr_err("Ring buf size must be power of 2"); - return NULL; - } mb_chann = kzalloc_obj(*mb_chann); if (!mb_chann) return NULL; + INIT_WORK(&mb_chann->rx_work, mailbox_rx_worker); + mb_chann->work_q = create_singlethread_workqueue(MAILBOX_NAME); + if (!mb_chann->work_q) { + MB_ERR(mb_chann, "Create workqueue failed"); + goto free_chann; + } mb_chann->mb = mb; + + return mb_chann; + +free_chann: + kfree(mb_chann); + return NULL; +} + +void xdna_mailbox_free_channel(struct mailbox_channel *mb_chann) +{ + destroy_workqueue(mb_chann->work_q); + kfree(mb_chann); +} + +int +xdna_mailbox_start_channel(struct mailbox_channel *mb_chann, + const struct xdna_mailbox_chann_res *x2i, + const struct xdna_mailbox_chann_res *i2x, + u32 iohub_int_addr, + int mb_irq) +{ + int ret; + + if (!is_power_of_2(x2i->rb_size) || !is_power_of_2(i2x->rb_size)) { + pr_err("Ring buf size must be power of 2"); + return -EINVAL; + } + mb_chann->msix_irq = mb_irq; mb_chann->iohub_int_addr = iohub_int_addr; memcpy(&mb_chann->res[CHAN_RES_X2I], x2i, sizeof(*x2i)); @@ -489,61 +512,37 @@ xdna_mailbox_create_channel(struct mailbox *mb, mb_chann->x2i_tail = mailbox_get_tailptr(mb_chann, CHAN_RES_X2I); mb_chann->i2x_head = mailbox_get_headptr(mb_chann, CHAN_RES_I2X); - INIT_WORK(&mb_chann->rx_work, mailbox_rx_worker); - mb_chann->work_q = create_singlethread_workqueue(MAILBOX_NAME); - if (!mb_chann->work_q) { - MB_ERR(mb_chann, "Create workqueue failed"); - goto free_and_out; - } - /* Everything look good. Time to enable irq handler */ ret = request_irq(mb_irq, mailbox_irq_handler, 0, MAILBOX_NAME, mb_chann); if (ret) { MB_ERR(mb_chann, "Failed to request irq %d ret %d", mb_irq, ret); - goto destroy_wq; + return ret; } mb_chann->bad_state = false; mailbox_reg_write(mb_chann, mb_chann->iohub_int_addr, 0); - MB_DBG(mb_chann, "Mailbox channel created (irq: %d)", mb_chann->msix_irq); - return mb_chann; - -destroy_wq: - destroy_workqueue(mb_chann->work_q); -free_and_out: - kfree(mb_chann); - return NULL; + MB_DBG(mb_chann, "Mailbox channel started (irq: %d)", mb_chann->msix_irq); + return 0; } -int xdna_mailbox_destroy_channel(struct mailbox_channel *mb_chann) +void xdna_mailbox_stop_channel(struct mailbox_channel *mb_chann) { struct mailbox_msg *mb_msg; unsigned long msg_id; - MB_DBG(mb_chann, "IRQ disabled and RX work cancelled"); + /* Disable an irq and wait. This might sleep. */ free_irq(mb_chann->msix_irq, mb_chann); - destroy_workqueue(mb_chann->work_q); - /* We can clean up and release resources */ + /* Cancel RX work and wait for it to finish */ + drain_workqueue(mb_chann->work_q); + + /* We can clean up and release resources */ xa_for_each(&mb_chann->chan_xa, msg_id, mb_msg) mailbox_release_msg(mb_chann, mb_msg); - xa_destroy(&mb_chann->chan_xa); - MB_DBG(mb_chann, "Mailbox channel destroyed, irq: %d", mb_chann->msix_irq); - kfree(mb_chann); - return 0; -} - -void xdna_mailbox_stop_channel(struct mailbox_channel *mb_chann) -{ - /* Disable an irq and wait. This might sleep. */ - disable_irq(mb_chann->msix_irq); - - /* Cancel RX work and wait for it to finish */ - cancel_work_sync(&mb_chann->rx_work); - MB_DBG(mb_chann, "IRQ disabled and RX work cancelled"); + MB_DBG(mb_chann, "Mailbox channel stopped, irq: %d", mb_chann->msix_irq); } struct mailbox *xdnam_mailbox_create(struct drm_device *ddev, diff --git a/drivers/accel/amdxdna/amdxdna_mailbox.h b/drivers/accel/amdxdna/amdxdna_mailbox.h index ea367f2fb738..8b1e00945da4 100644 --- a/drivers/accel/amdxdna/amdxdna_mailbox.h +++ b/drivers/accel/amdxdna/amdxdna_mailbox.h @@ -74,9 +74,16 @@ struct mailbox *xdnam_mailbox_create(struct drm_device *ddev, const struct xdna_mailbox_res *res); /* - * xdna_mailbox_create_channel() -- Create a mailbox channel instance + * xdna_mailbox_alloc_channel() -- alloc a mailbox channel * - * @mailbox: the handle return from xdna_mailbox_create() + * @mb: mailbox handle + */ +struct mailbox_channel *xdna_mailbox_alloc_channel(struct mailbox *mb); + +/* + * xdna_mailbox_start_channel() -- start a mailbox channel instance + * + * @mb_chann: the handle return from xdna_mailbox_alloc_channel() * @x2i: host to firmware mailbox resources * @i2x: firmware to host mailbox resources * @xdna_mailbox_intr_reg: register addr of MSI-X interrupt @@ -84,28 +91,24 @@ struct mailbox *xdnam_mailbox_create(struct drm_device *ddev, * * Return: If success, return a handle of mailbox channel. Otherwise, return NULL. */ -struct mailbox_channel * -xdna_mailbox_create_channel(struct mailbox *mailbox, - const struct xdna_mailbox_chann_res *x2i, - const struct xdna_mailbox_chann_res *i2x, - u32 xdna_mailbox_intr_reg, - int mb_irq); +int +xdna_mailbox_start_channel(struct mailbox_channel *mb_chann, + const struct xdna_mailbox_chann_res *x2i, + const struct xdna_mailbox_chann_res *i2x, + u32 xdna_mailbox_intr_reg, + int mb_irq); /* - * xdna_mailbox_destroy_channel() -- destroy mailbox channel + * xdna_mailbox_free_channel() -- free mailbox channel * * @mailbox_chann: the handle return from xdna_mailbox_create_channel() - * - * Return: if success, return 0. otherwise return error code */ -int xdna_mailbox_destroy_channel(struct mailbox_channel *mailbox_chann); +void xdna_mailbox_free_channel(struct mailbox_channel *mailbox_chann); /* * xdna_mailbox_stop_channel() -- stop mailbox channel * * @mailbox_chann: the handle return from xdna_mailbox_create_channel() - * - * Return: if success, return 0. otherwise return error code */ void xdna_mailbox_stop_channel(struct mailbox_channel *mailbox_chann); -- cgit v1.2.3 From 150bceb3e0a4a30950279d91ea0e8cc69a736742 Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Wed, 18 Feb 2026 16:21:55 -0600 Subject: accel: ethosu: Fix job submit error clean-up refcount underflows If the job submit fails before adding the job to the scheduler queue such as when the GEM buffer bounds checks fail, then doing a ethosu_job_put() results in a pm_runtime_put_autosuspend() without the corresponding pm_runtime_resume_and_get(). The dma_fence_put()'s are also unnecessary, but seem to be harmless. Split the ethosu_job_cleanup() function into 2 parts for the before and after the job is queued. Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver") Reviewed-and-Tested-by: Anders Roxell Link: https://patch.msgid.link/20260218-ethos-fixes-v1-1-be3fa3ea9a30@kernel.org Signed-off-by: Rob Herring (Arm) --- drivers/accel/ethosu/ethosu_job.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) (limited to 'drivers/accel') diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c index 8598a3634340..ec85f4156744 100644 --- a/drivers/accel/ethosu/ethosu_job.c +++ b/drivers/accel/ethosu/ethosu_job.c @@ -143,23 +143,29 @@ out: return ret; } +static void ethosu_job_err_cleanup(struct ethosu_job *job) +{ + unsigned int i; + + for (i = 0; i < job->region_cnt; i++) + drm_gem_object_put(job->region_bo[i]); + + drm_gem_object_put(job->cmd_bo); + + kfree(job); +} + static void ethosu_job_cleanup(struct kref *ref) { struct ethosu_job *job = container_of(ref, struct ethosu_job, refcount); - unsigned int i; pm_runtime_put_autosuspend(job->dev->base.dev); dma_fence_put(job->done_fence); dma_fence_put(job->inference_done_fence); - for (i = 0; i < job->region_cnt; i++) - drm_gem_object_put(job->region_bo[i]); - - drm_gem_object_put(job->cmd_bo); - - kfree(job); + ethosu_job_err_cleanup(job); } static void ethosu_job_put(struct ethosu_job *job) @@ -454,12 +460,16 @@ static int ethosu_ioctl_submit_job(struct drm_device *dev, struct drm_file *file } } ret = ethosu_job_push(ejob); + if (!ret) { + ethosu_job_put(ejob); + return 0; + } out_cleanup_job: if (ret) drm_sched_job_cleanup(&ejob->base); out_put_job: - ethosu_job_put(ejob); + ethosu_job_err_cleanup(ejob); return ret; } -- cgit v1.2.3 From 838ae99f9a77a5724ee6d4e7b7b1eb079147f888 Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Wed, 18 Feb 2026 16:21:56 -0600 Subject: accel: ethosu: Fix NPU_OP_ELEMENTWISE validation with scalar The NPU_OP_ELEMENTWISE instruction uses a scalar value for IFM2 if the IFM2_BROADCAST "scalar" mode is set. It is a bit (7) on the u65 and part of a field (bits 3:0) on the u85. The driver was hardcoded to the u85. Fixes: 5a5e9c0228e6 ("accel: Add Arm Ethos-U NPU driver") Reviewed-and-Tested-by: Anders Roxell Link: https://patch.msgid.link/20260218-ethos-fixes-v1-2-be3fa3ea9a30@kernel.org Signed-off-by: Rob Herring (Arm) --- drivers/accel/ethosu/ethosu_gem.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/accel') diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c index 668c71d5ff45..ff5e58944138 100644 --- a/drivers/accel/ethosu/ethosu_gem.c +++ b/drivers/accel/ethosu/ethosu_gem.c @@ -417,7 +417,10 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev, return ret; break; case NPU_OP_ELEMENTWISE: - use_ifm2 = !((st.ifm2.broadcast == 8) || (param == 5) || + use_scale = ethosu_is_u65(edev) ? + (st.ifm2.broadcast & 0x80) : + (st.ifm2.broadcast == 8); + use_ifm2 = !(use_scale || (param == 5) || (param == 6) || (param == 7) || (param == 0x24)); use_ifm = st.ifm.broadcast != 8; ret = calc_sizes_elemwise(ddev, info, cmd, &st, use_ifm, use_ifm2); -- cgit v1.2.3 From 021f1b77f70d62351bba67b050b8d784641d817f Mon Sep 17 00:00:00 2001 From: "Rob Herring (Arm)" Date: Wed, 18 Feb 2026 16:21:57 -0600 Subject: accel: ethosu: Handle possible underflow in IFM size calculations If the command stream has larger padding sizes than the IFM and OFM diminsions, then the calculations will underflow to a negative value. The result is a very large region bounds which is caught on submit, but it's better to catch it earlier. Current mesa ethosu driver has a signedness bug which resulted in padding of 127 (the max) and triggers this issue. Reviewed-and-Tested-by: Anders Roxell Link: https://patch.msgid.link/20260218-ethos-fixes-v1-3-be3fa3ea9a30@kernel.org Signed-off-by: Rob Herring (Arm) --- drivers/accel/ethosu/ethosu_gem.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/accel') diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c index ff5e58944138..7994e7073903 100644 --- a/drivers/accel/ethosu/ethosu_gem.c +++ b/drivers/accel/ethosu/ethosu_gem.c @@ -245,11 +245,14 @@ static int calc_sizes(struct drm_device *ddev, ((st->ifm.stride_kernel >> 1) & 0x1) + 1; u32 stride_x = ((st->ifm.stride_kernel >> 5) & 0x2) + (st->ifm.stride_kernel & 0x1) + 1; - u32 ifm_height = st->ofm.height[2] * stride_y + + s32 ifm_height = st->ofm.height[2] * stride_y + st->ifm.height[2] - (st->ifm.pad_top + st->ifm.pad_bottom); - u32 ifm_width = st->ofm.width * stride_x + + s32 ifm_width = st->ofm.width * stride_x + st->ifm.width - (st->ifm.pad_left + st->ifm.pad_right); + if (ifm_height < 0 || ifm_width < 0) + return -EINVAL; + len = feat_matrix_length(info, &st->ifm, ifm_width, ifm_height, st->ifm.depth); dev_dbg(ddev->dev, "op %d: IFM:%d:0x%llx-0x%llx\n", -- cgit v1.2.3