* [PATCH 0/2] amdxdna: fixes for closing a process @ 2026-02-10 16:42 Mario Limonciello 2026-02-10 16:42 ` [PATCH 1/2] accel/amdxdna: Fix NULL pointer dereference in mailbox channel cleanup Mario Limonciello 2026-02-10 16:42 ` [PATCH 2/2] accel/amdxdna: Reduce log noise during process termination Mario Limonciello 0 siblings, 2 replies; 6+ messages in thread From: Mario Limonciello @ 2026-02-10 16:42 UTC (permalink / raw) To: mario.limonciello, lizhi.hou, mamin506, ogabbay, superm1; +Cc: dri-devel I found that with drm-next when I close a process using amdxdna that I hit a GPF. After fixing the GPF I found that it was really noisy for standard cleanup from a closed process. Mario Limonciello (2): accel/amdxdna: Fix NULL pointer dereference in mailbox channel cleanup accel/amdxdna: Reduce log noise during process termination drivers/accel/amdxdna/aie2_ctx.c | 6 ++++-- drivers/accel/amdxdna/aie2_message.c | 18 ++++++++++++------ drivers/accel/amdxdna/aie2_pci.c | 14 +++++++++----- 3 files changed, 25 insertions(+), 13 deletions(-) -- 2.53.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] accel/amdxdna: Fix NULL pointer dereference in mailbox channel cleanup 2026-02-10 16:42 [PATCH 0/2] amdxdna: fixes for closing a process Mario Limonciello @ 2026-02-10 16:42 ` Mario Limonciello 2026-02-10 17:17 ` Lizhi Hou 2026-02-10 16:42 ` [PATCH 2/2] accel/amdxdna: Reduce log noise during process termination Mario Limonciello 1 sibling, 1 reply; 6+ messages in thread From: Mario Limonciello @ 2026-02-10 16:42 UTC (permalink / raw) To: mario.limonciello, lizhi.hou, mamin506, ogabbay, superm1; +Cc: dri-devel aie2_destroy_context() is called during various cleanup paths, including when context creation fails partially. If xdna_mailbox_create_channel() fails during aie2_create_context(), the hwctx->priv->mbox_chann pointer remains NULL. When cleanup occurs (e.g., during process termination via amdxdna_hwctx_remove_all), aie2_destroy_context() is invoked and attempts to stop and destroy the NULL mailbox channel, leading to a NULL pointer dereference. The issue was observed in the following call path: amdxdna_drm_close amdxdna_hwctx_remove_all aie2_hwctx_fini aie2_release_resource aie2_destroy_context xdna_mailbox_stop_channel <- NULL dereference Add NULL checks in aie2_destroy_context() before calling mailbox channel operations. Also add defensive NULL checks in aie2_hw_stop() for both mgmt_chann and mbox to prevent similar issues during device shutdown. Fixes: 97f27573837e ("accel/amdxdna: Fix potential NULL pointer dereference in context cleanup") Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/accel/amdxdna/aie2_message.c | 14 +++++++++----- drivers/accel/amdxdna/aie2_pci.c | 14 +++++++++----- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c index 7d7dcfeaf7942..77e3cdf18658b 100644 --- a/drivers/accel/amdxdna/aie2_message.c +++ b/drivers/accel/amdxdna/aie2_message.c @@ -318,11 +318,15 @@ int aie2_destroy_context(struct amdxdna_dev_hdl *ndev, struct amdxdna_hwctx *hwc struct amdxdna_dev *xdna = ndev->xdna; int ret; - 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_DBG(xdna, "Destroyed fw ctx %d", hwctx->fw_ctx_id); - hwctx->priv->mbox_chann = NULL; + if (hwctx->priv->mbox_chann) { + 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_DBG(xdna, "Destroyed fw ctx %d", hwctx->fw_ctx_id); + hwctx->priv->mbox_chann = NULL; + } else { + ret = aie2_destroy_context_req(ndev, hwctx->fw_ctx_id); + } hwctx->fw_ctx_id = -1; ndev->hwctx_num--; diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c index f70ccf0f3c019..9c2572706bf53 100644 --- a/drivers/accel/amdxdna/aie2_pci.c +++ b/drivers/accel/amdxdna/aie2_pci.c @@ -324,11 +324,15 @@ static void aie2_hw_stop(struct amdxdna_dev *xdna) } aie2_mgmt_fw_fini(ndev); - xdna_mailbox_stop_channel(ndev->mgmt_chann); - xdna_mailbox_destroy_channel(ndev->mgmt_chann); - ndev->mgmt_chann = NULL; - drmm_kfree(&xdna->ddev, ndev->mbox); - ndev->mbox = NULL; + if (ndev->mgmt_chann) { + xdna_mailbox_stop_channel(ndev->mgmt_chann); + xdna_mailbox_destroy_channel(ndev->mgmt_chann); + ndev->mgmt_chann = NULL; + } + if (ndev->mbox) { + drmm_kfree(&xdna->ddev, ndev->mbox); + ndev->mbox = NULL; + } aie2_psp_stop(ndev->psp_hdl); aie2_smu_fini(ndev); aie2_error_async_events_free(ndev); -- 2.53.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] accel/amdxdna: Fix NULL pointer dereference in mailbox channel cleanup 2026-02-10 16:42 ` [PATCH 1/2] accel/amdxdna: Fix NULL pointer dereference in mailbox channel cleanup Mario Limonciello @ 2026-02-10 17:17 ` Lizhi Hou 0 siblings, 0 replies; 6+ messages in thread From: Lizhi Hou @ 2026-02-10 17:17 UTC (permalink / raw) To: Mario Limonciello, mamin506, ogabbay, superm1; +Cc: dri-devel Hi Mario, I posted a fix for this: https://lore.kernel.org/dri-devel/20260206060306.4050531-1-lizhi.hou@amd.com/ I am not sure if it still a good time to merge to drm-misc-next-fixes for 6.20 kernel. And I plan to merge to drm-misc-fixes during 6.20 rc1 time. Thanks, Lizhi On 2/10/26 08:42, Mario Limonciello wrote: > aie2_destroy_context() is called during various cleanup paths, including > when context creation fails partially. If xdna_mailbox_create_channel() > fails during aie2_create_context(), the hwctx->priv->mbox_chann pointer > remains NULL. When cleanup occurs (e.g., during process termination via > amdxdna_hwctx_remove_all), aie2_destroy_context() is invoked and attempts > to stop and destroy the NULL mailbox channel, leading to a NULL pointer > dereference. > > The issue was observed in the following call path: > amdxdna_drm_close > amdxdna_hwctx_remove_all > aie2_hwctx_fini > aie2_release_resource > aie2_destroy_context > xdna_mailbox_stop_channel <- NULL dereference > > Add NULL checks in aie2_destroy_context() before calling mailbox channel > operations. Also add defensive NULL checks in aie2_hw_stop() for both > mgmt_chann and mbox to prevent similar issues during device shutdown. > > Fixes: 97f27573837e ("accel/amdxdna: Fix potential NULL pointer dereference in context cleanup") > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/accel/amdxdna/aie2_message.c | 14 +++++++++----- > drivers/accel/amdxdna/aie2_pci.c | 14 +++++++++----- > 2 files changed, 18 insertions(+), 10 deletions(-) > > diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c > index 7d7dcfeaf7942..77e3cdf18658b 100644 > --- a/drivers/accel/amdxdna/aie2_message.c > +++ b/drivers/accel/amdxdna/aie2_message.c > @@ -318,11 +318,15 @@ int aie2_destroy_context(struct amdxdna_dev_hdl *ndev, struct amdxdna_hwctx *hwc > struct amdxdna_dev *xdna = ndev->xdna; > int ret; > > - 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_DBG(xdna, "Destroyed fw ctx %d", hwctx->fw_ctx_id); > - hwctx->priv->mbox_chann = NULL; > + if (hwctx->priv->mbox_chann) { > + 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_DBG(xdna, "Destroyed fw ctx %d", hwctx->fw_ctx_id); > + hwctx->priv->mbox_chann = NULL; > + } else { > + ret = aie2_destroy_context_req(ndev, hwctx->fw_ctx_id); > + } > hwctx->fw_ctx_id = -1; > ndev->hwctx_num--; > > diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c > index f70ccf0f3c019..9c2572706bf53 100644 > --- a/drivers/accel/amdxdna/aie2_pci.c > +++ b/drivers/accel/amdxdna/aie2_pci.c > @@ -324,11 +324,15 @@ static void aie2_hw_stop(struct amdxdna_dev *xdna) > } > > aie2_mgmt_fw_fini(ndev); > - xdna_mailbox_stop_channel(ndev->mgmt_chann); > - xdna_mailbox_destroy_channel(ndev->mgmt_chann); > - ndev->mgmt_chann = NULL; > - drmm_kfree(&xdna->ddev, ndev->mbox); > - ndev->mbox = NULL; > + if (ndev->mgmt_chann) { > + xdna_mailbox_stop_channel(ndev->mgmt_chann); > + xdna_mailbox_destroy_channel(ndev->mgmt_chann); > + ndev->mgmt_chann = NULL; > + } > + if (ndev->mbox) { > + drmm_kfree(&xdna->ddev, ndev->mbox); > + ndev->mbox = NULL; > + } > aie2_psp_stop(ndev->psp_hdl); > aie2_smu_fini(ndev); > aie2_error_async_events_free(ndev); ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] accel/amdxdna: Reduce log noise during process termination 2026-02-10 16:42 [PATCH 0/2] amdxdna: fixes for closing a process Mario Limonciello 2026-02-10 16:42 ` [PATCH 1/2] accel/amdxdna: Fix NULL pointer dereference in mailbox channel cleanup Mario Limonciello @ 2026-02-10 16:42 ` Mario Limonciello 2026-02-10 17:20 ` Lizhi Hou 2026-02-25 18:19 ` Falkowski, Maciej 1 sibling, 2 replies; 6+ messages in thread From: Mario Limonciello @ 2026-02-10 16:42 UTC (permalink / raw) To: mario.limonciello, lizhi.hou, mamin506, ogabbay, superm1; +Cc: dri-devel During process termination, several error messages are logged that are not actual errors but expected conditions when a process is killed or interrupted. This creates unnecessary noise in the kernel log. The specific scenarios are: 1. HMM invalidation returns -ERESTARTSYS when the wait is interrupted by a signal during process cleanup. This is expected when a process is being terminated and should not be logged as an error. 2. Context destruction returns -ENODEV when the firmware or device has already stopped, which commonly occurs during cleanup if the device was already torn down. This is also an expected condition during orderly shutdown. Downgrade these expected error conditions from error level to debug level to reduce log noise while still keeping genuine errors visible. Fixes: 97f27573837e ("accel/amdxdna: Fix potential NULL pointer dereference in context cleanup") Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/accel/amdxdna/aie2_ctx.c | 6 ++++-- drivers/accel/amdxdna/aie2_message.c | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c index 37d05f2e986f9..79f6316655e73 100644 --- a/drivers/accel/amdxdna/aie2_ctx.c +++ b/drivers/accel/amdxdna/aie2_ctx.c @@ -497,7 +497,7 @@ static void aie2_release_resource(struct amdxdna_hwctx *hwctx) if (AIE2_FEATURE_ON(xdna->dev_handle, AIE2_TEMPORAL_ONLY)) { ret = aie2_destroy_context(xdna->dev_handle, hwctx); - if (ret) + if (ret && ret != -ENODEV) XDNA_ERR(xdna, "Destroy temporal only context failed, ret %d", ret); } else { ret = xrs_release_resource(xdna->xrs_hdl, (uintptr_t)hwctx); @@ -1070,6 +1070,8 @@ void aie2_hmm_invalidate(struct amdxdna_gem_obj *abo, ret = dma_resv_wait_timeout(gobj->resv, DMA_RESV_USAGE_BOOKKEEP, true, MAX_SCHEDULE_TIMEOUT); - if (!ret || ret == -ERESTARTSYS) + if (!ret) XDNA_ERR(xdna, "Failed to wait for bo, ret %ld", ret); + else if (ret == -ERESTARTSYS) + XDNA_DBG(xdna, "Wait for bo interrupted by signal"); } diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c index 77e3cdf18658b..5697c0c2dd43f 100644 --- a/drivers/accel/amdxdna/aie2_message.c +++ b/drivers/accel/amdxdna/aie2_message.c @@ -216,8 +216,10 @@ static int aie2_destroy_context_req(struct amdxdna_dev_hdl *ndev, u32 id) req.context_id = id; ret = aie2_send_mgmt_msg_wait(ndev, &msg); - if (ret) + if (ret && ret != -ENODEV) XDNA_WARN(xdna, "Destroy context failed, ret %d", ret); + else if (ret == -ENODEV) + XDNA_DBG(xdna, "Destroy context: device already stopped"); return ret; } -- 2.53.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] accel/amdxdna: Reduce log noise during process termination 2026-02-10 16:42 ` [PATCH 2/2] accel/amdxdna: Reduce log noise during process termination Mario Limonciello @ 2026-02-10 17:20 ` Lizhi Hou 2026-02-25 18:19 ` Falkowski, Maciej 1 sibling, 0 replies; 6+ messages in thread From: Lizhi Hou @ 2026-02-10 17:20 UTC (permalink / raw) To: Mario Limonciello, mamin506, ogabbay, superm1; +Cc: dri-devel On 2/10/26 08:42, Mario Limonciello wrote: > During process termination, several error messages are logged that are > not actual errors but expected conditions when a process is killed or > interrupted. This creates unnecessary noise in the kernel log. > > The specific scenarios are: > > 1. HMM invalidation returns -ERESTARTSYS when the wait is interrupted by > a signal during process cleanup. This is expected when a process is > being terminated and should not be logged as an error. > > 2. Context destruction returns -ENODEV when the firmware or device has > already stopped, which commonly occurs during cleanup if the device > was already torn down. This is also an expected condition during > orderly shutdown. > > Downgrade these expected error conditions from error level to debug level > to reduce log noise while still keeping genuine errors visible. > > Fixes: 97f27573837e ("accel/amdxdna: Fix potential NULL pointer dereference in context cleanup") > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/accel/amdxdna/aie2_ctx.c | 6 ++++-- > drivers/accel/amdxdna/aie2_message.c | 4 +++- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c > index 37d05f2e986f9..79f6316655e73 100644 > --- a/drivers/accel/amdxdna/aie2_ctx.c > +++ b/drivers/accel/amdxdna/aie2_ctx.c > @@ -497,7 +497,7 @@ static void aie2_release_resource(struct amdxdna_hwctx *hwctx) > > if (AIE2_FEATURE_ON(xdna->dev_handle, AIE2_TEMPORAL_ONLY)) { > ret = aie2_destroy_context(xdna->dev_handle, hwctx); > - if (ret) > + if (ret && ret != -ENODEV) > XDNA_ERR(xdna, "Destroy temporal only context failed, ret %d", ret); > } else { > ret = xrs_release_resource(xdna->xrs_hdl, (uintptr_t)hwctx); > @@ -1070,6 +1070,8 @@ void aie2_hmm_invalidate(struct amdxdna_gem_obj *abo, > > ret = dma_resv_wait_timeout(gobj->resv, DMA_RESV_USAGE_BOOKKEEP, > true, MAX_SCHEDULE_TIMEOUT); > - if (!ret || ret == -ERESTARTSYS) > + if (!ret) > XDNA_ERR(xdna, "Failed to wait for bo, ret %ld", ret); > + else if (ret == -ERESTARTSYS) > + XDNA_DBG(xdna, "Wait for bo interrupted by signal"); > } > diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c > index 77e3cdf18658b..5697c0c2dd43f 100644 > --- a/drivers/accel/amdxdna/aie2_message.c > +++ b/drivers/accel/amdxdna/aie2_message.c > @@ -216,8 +216,10 @@ static int aie2_destroy_context_req(struct amdxdna_dev_hdl *ndev, u32 id) > > req.context_id = id; > ret = aie2_send_mgmt_msg_wait(ndev, &msg); > - if (ret) > + if (ret && ret != -ENODEV) > XDNA_WARN(xdna, "Destroy context failed, ret %d", ret);Reviewed-by: > + else if (ret == -ENODEV) > + XDNA_DBG(xdna, "Destroy context: device already stopped"); Reviewed-by: Lizhi Hou <lizhi.hou@amd.com> > > return ret; > } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] accel/amdxdna: Reduce log noise during process termination 2026-02-10 16:42 ` [PATCH 2/2] accel/amdxdna: Reduce log noise during process termination Mario Limonciello 2026-02-10 17:20 ` Lizhi Hou @ 2026-02-25 18:19 ` Falkowski, Maciej 1 sibling, 0 replies; 6+ messages in thread From: Falkowski, Maciej @ 2026-02-25 18:19 UTC (permalink / raw) To: Mario Limonciello, lizhi.hou, mamin506, ogabbay, superm1; +Cc: dri-devel Reviewed-by: Maciej Falkowski <maciej.falkowski@linux.intel.com> On 2/10/2026 5:42 PM, Mario Limonciello wrote: > During process termination, several error messages are logged that are > not actual errors but expected conditions when a process is killed or > interrupted. This creates unnecessary noise in the kernel log. > > The specific scenarios are: > > 1. HMM invalidation returns -ERESTARTSYS when the wait is interrupted by > a signal during process cleanup. This is expected when a process is > being terminated and should not be logged as an error. > > 2. Context destruction returns -ENODEV when the firmware or device has > already stopped, which commonly occurs during cleanup if the device > was already torn down. This is also an expected condition during > orderly shutdown. > > Downgrade these expected error conditions from error level to debug level > to reduce log noise while still keeping genuine errors visible. > > Fixes: 97f27573837e ("accel/amdxdna: Fix potential NULL pointer dereference in context cleanup") > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/accel/amdxdna/aie2_ctx.c | 6 ++++-- > drivers/accel/amdxdna/aie2_message.c | 4 +++- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c > index 37d05f2e986f9..79f6316655e73 100644 > --- a/drivers/accel/amdxdna/aie2_ctx.c > +++ b/drivers/accel/amdxdna/aie2_ctx.c > @@ -497,7 +497,7 @@ static void aie2_release_resource(struct amdxdna_hwctx *hwctx) > > if (AIE2_FEATURE_ON(xdna->dev_handle, AIE2_TEMPORAL_ONLY)) { > ret = aie2_destroy_context(xdna->dev_handle, hwctx); > - if (ret) > + if (ret && ret != -ENODEV) > XDNA_ERR(xdna, "Destroy temporal only context failed, ret %d", ret); > } else { > ret = xrs_release_resource(xdna->xrs_hdl, (uintptr_t)hwctx); > @@ -1070,6 +1070,8 @@ void aie2_hmm_invalidate(struct amdxdna_gem_obj *abo, > > ret = dma_resv_wait_timeout(gobj->resv, DMA_RESV_USAGE_BOOKKEEP, > true, MAX_SCHEDULE_TIMEOUT); > - if (!ret || ret == -ERESTARTSYS) > + if (!ret) > XDNA_ERR(xdna, "Failed to wait for bo, ret %ld", ret); > + else if (ret == -ERESTARTSYS) > + XDNA_DBG(xdna, "Wait for bo interrupted by signal"); > } > diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c > index 77e3cdf18658b..5697c0c2dd43f 100644 > --- a/drivers/accel/amdxdna/aie2_message.c > +++ b/drivers/accel/amdxdna/aie2_message.c > @@ -216,8 +216,10 @@ static int aie2_destroy_context_req(struct amdxdna_dev_hdl *ndev, u32 id) > > req.context_id = id; > ret = aie2_send_mgmt_msg_wait(ndev, &msg); > - if (ret) > + if (ret && ret != -ENODEV) > XDNA_WARN(xdna, "Destroy context failed, ret %d", ret); > + else if (ret == -ENODEV) > + XDNA_DBG(xdna, "Destroy context: device already stopped"); > > return ret; > } ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-02-25 18:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-10 16:42 [PATCH 0/2] amdxdna: fixes for closing a process Mario Limonciello 2026-02-10 16:42 ` [PATCH 1/2] accel/amdxdna: Fix NULL pointer dereference in mailbox channel cleanup Mario Limonciello 2026-02-10 17:17 ` Lizhi Hou 2026-02-10 16:42 ` [PATCH 2/2] accel/amdxdna: Reduce log noise during process termination Mario Limonciello 2026-02-10 17:20 ` Lizhi Hou 2026-02-25 18:19 ` Falkowski, Maciej
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox