[PATCH 0/2] amdxdna: fixes for closing a process

[PATCH 0/2] amdxdna: fixes for closing a process

[Mario Limonciello]

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

[PATCH 1/2] accel/amdxdna: Fix NULL pointer dereference in mailbox channel cleanup

[Mario Limonciello]

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

[PATCH 2/2] accel/amdxdna: Reduce log noise during process termination

[Mario Limonciello]

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

Re: [PATCH 1/2] accel/amdxdna: Fix NULL pointer dereference in mailbox channel cleanup

[Lizhi Hou]

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);

Re: [PATCH 2/2] accel/amdxdna: Reduce log noise during process termination

[Lizhi Hou]

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;
>   }

Re: [PATCH 2/2] accel/amdxdna: Reduce log noise during process termination

[Falkowski, Maciej]

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;
>   }