[RFC PATCH] remoteproc: xlnx: initialize mailbox work before requesting channels

[RFC PATCH] remoteproc: xlnx: initialize mailbox work before requesting channels

[Runyu Xiao]

zynqmp_r5_setup_mbox() installs zynqmp_r5_mb_rx_cb() as the mailbox RX
callback before requesting the mailbox channels, but initializes
ipi->mbox_work only after both channels have been requested. Once the RX
channel is active, a notification delivered before the late INIT_WORK()
would make the callback queue an uninitialized work item.

Initialize the work item and its owning R5 core pointer before requesting
channels. Also drain the work before freeing the mailbox state, after the
channels have been released so no new callbacks can queue it.

This issue was found by our static analysis tool and then confirmed by
manual review of the mailbox setup sequence. The callback is published
before the channel requests complete, so the work item and the state used
by the work handler should be ready before the mailbox provider can invoke
it.

A QEMU PoC modeled a mailbox notification delivered after the RX callback
became reachable but before the delayed INIT_WORK(). DEBUG_OBJECTS reported
queueing an uninitialized work item from the zynqmp_r5_setup_mbox() path.

This is sent as an RFC because the practical trigger depends on the ZynqMP
IPI mailbox provider and firmware delivery timing. If the provider cannot
invoke the RX callback until after setup returns, this is a defensive
lifecycle cleanup rather than a reachable race on current systems.

Fixes: 5dfb28c257b7 ("remoteproc: xilinx: Add mailbox channels for rpmsg")
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/remoteproc/xlnx_r5_remoteproc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c
index 0b7b173d0d26..1477fc542afb 100644
--- a/drivers/remoteproc/xlnx_r5_remoteproc.c
+++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
@@ -260,8 +260,9 @@ static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void *msg)
  * Function to setup mailboxes related properties
  * return : NULL if failed else pointer to mbox_info
  */
-static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev)
+static struct mbox_info *zynqmp_r5_setup_mbox(struct zynqmp_r5_core *r5_core)
 {
+	struct device *cdev = r5_core->dev;
 	struct mbox_client *mbox_cl;
 	struct mbox_info *ipi;
 
@@ -269,6 +270,9 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev)
 	if (!ipi)
 		return NULL;
 
+	ipi->r5_core = r5_core;
+	INIT_WORK(&ipi->mbox_work, handle_event_notified);
+
 	mbox_cl = &ipi->mbox_cl;
 	mbox_cl->rx_callback = zynqmp_r5_mb_rx_cb;
 	mbox_cl->tx_block = false;
@@ -295,8 +299,6 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev)
 		return NULL;
 	}
 
-	INIT_WORK(&ipi->mbox_work, handle_event_notified);
-
 	return ipi;
 }
 
@@ -315,6 +317,8 @@ static void zynqmp_r5_free_mbox(struct mbox_info *ipi)
 		ipi->rx_chan = NULL;
 	}
 
+	cancel_work_sync(&ipi->mbox_work);
+
 	kfree(ipi);
 }
 
@@ -1388,10 +1392,9 @@ static int zynqmp_r5_cluster_init(struct zynqmp_r5_cluster *cluster)
 		 * If mailbox nodes are disabled using "status" property then
 		 * setting up mailbox channels will fail.
 		 */
-		ipi = zynqmp_r5_setup_mbox(&child_pdev->dev);
+		ipi = zynqmp_r5_setup_mbox(r5_cores[i]);
 		if (ipi) {
 			r5_cores[i]->ipi = ipi;
-			ipi->r5_core = r5_cores[i];
 		}
 
 		/*
-- 
2.34.1

Re: [RFC PATCH] remoteproc: xlnx: initialize mailbox work before requesting channels

[Shah, Tanmay]

Hello,

Thank you for your patch. Please find my comments below:

On 6/19/2026 2:48 AM, Runyu Xiao wrote:
> zynqmp_r5_setup_mbox() installs zynqmp_r5_mb_rx_cb() as the mailbox RX
> callback before requesting the mailbox channels, but initializes
> ipi->mbox_work only after both channels have been requested. Once the RX
> channel is active, a notification delivered before the late INIT_WORK()
> would make the callback queue an uninitialized work item.
> 
> Initialize the work item and its owning R5 core pointer before requesting
> channels. Also drain the work before freeing the mailbox state, after the
> channels have been released so no new callbacks can queue it.
> 
> This issue was found by our static analysis tool and then confirmed by
> manual review of the mailbox setup sequence. The callback is published
> before the channel requests complete, so the work item and the state used
> by the work handler should be ready before the mailbox provider can invoke
> it.
> 
> A QEMU PoC modeled a mailbox notification delivered after the RX callback
> became reachable but before the delayed INIT_WORK(). DEBUG_OBJECTS reported
> queueing an uninitialized work item from the zynqmp_r5_setup_mbox() path.
> 
> This is sent as an RFC because the practical trigger depends on the ZynqMP
> IPI mailbox provider and firmware delivery timing. If the provider cannot
> invoke the RX callback until after setup returns, this is a defensive
> lifecycle cleanup rather than a reachable race on current systems.
> 
> Fixes: 5dfb28c257b7 ("remoteproc: xilinx: Add mailbox channels for rpmsg")
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
>  drivers/remoteproc/xlnx_r5_remoteproc.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c
> index 0b7b173d0d26..1477fc542afb 100644
> --- a/drivers/remoteproc/xlnx_r5_remoteproc.c
> +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
> @@ -260,8 +260,9 @@ static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void *msg)
>   * Function to setup mailboxes related properties
>   * return : NULL if failed else pointer to mbox_info
>   */
> -static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev)
> +static struct mbox_info *zynqmp_r5_setup_mbox(struct zynqmp_r5_core *r5_core)

Why this is needed in this patch?

I have another patch that I will post which moves mabilbox setup
operation to rproc::ops::prepare(). I think the interface should be
changed in that patch.

>  {
> +	struct device *cdev = r5_core->dev;
>  	struct mbox_client *mbox_cl;
>  	struct mbox_info *ipi;
>  
> @@ -269,6 +270,9 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev)
>  	if (!ipi)
>  		return NULL;
>  
> +	ipi->r5_core = r5_core;
> +	INIT_WORK(&ipi->mbox_work, handle_event_notified);
> +

I agree with this part. IMO This patch can simply move INIT_WORK before
requesting channels and cancel_work() in the zynqmp_r5_free_mbox().

Thank You,
Tanmay


>  	mbox_cl = &ipi->mbox_cl;
>  	mbox_cl->rx_callback = zynqmp_r5_mb_rx_cb;
>  	mbox_cl->tx_block = false;
> @@ -295,8 +299,6 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev)
>  		return NULL;
>  	}
>  
> -	INIT_WORK(&ipi->mbox_work, handle_event_notified);
> -
>  	return ipi;
>  }
>  
> @@ -315,6 +317,8 @@ static void zynqmp_r5_free_mbox(struct mbox_info *ipi)
>  		ipi->rx_chan = NULL;
>  	}
>  
> +	cancel_work_sync(&ipi->mbox_work);
> +
>  	kfree(ipi);
>  }
>  
> @@ -1388,10 +1392,9 @@ static int zynqmp_r5_cluster_init(struct zynqmp_r5_cluster *cluster)
>  		 * If mailbox nodes are disabled using "status" property then
>  		 * setting up mailbox channels will fail.
>  		 */
> -		ipi = zynqmp_r5_setup_mbox(&child_pdev->dev);
> +		ipi = zynqmp_r5_setup_mbox(r5_cores[i]);
>  		if (ipi) {
>  			r5_cores[i]->ipi = ipi;
> -			ipi->r5_core = r5_cores[i];
>  		}
>  
>  		/*

[RFC PATCH v2] remoteproc: xlnx: initialize mailbox work before requesting channels

[Runyu Xiao]

zynqmp_r5_setup_mbox() installs zynqmp_r5_mb_rx_cb() as the mailbox RX
callback before requesting the mailbox channels, but initializes
ipi->mbox_work only after both channels have been requested. Once the RX
channel is active, a notification delivered before the late INIT_WORK()
would make the callback queue an uninitialized work item.

Initialize the work item before requesting channels. Also drain the work
before freeing the mailbox state, after the channels have been released so
no new callbacks can queue it.

This issue was found by our static analysis tool and then confirmed by
manual review of the mailbox setup sequence. The callback is published
before the channel requests complete, so the work item should be ready
before the mailbox provider can invoke it.

A QEMU PoC modeled a mailbox notification delivered after the RX callback
became reachable but before the delayed INIT_WORK(). DEBUG_OBJECTS reported
queueing an uninitialized work item from the zynqmp_r5_setup_mbox() path.

This is sent as an RFC because the practical trigger depends on the ZynqMP
IPI mailbox provider and firmware delivery timing. If the provider cannot
invoke the RX callback until after setup returns, this is a defensive
lifecycle cleanup rather than a reachable race on current systems.

Fixes: 5dfb28c257b7 ("remoteproc: xilinx: Add mailbox channels for rpmsg")
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
Changes in v2:
- Follow Tanmay's suggestion and keep zynqmp_r5_setup_mbox() taking the
  child device pointer. Do not move the r5_core assignment in this patch.
- Only move INIT_WORK() before channel requests and drain the work in
  zynqmp_r5_free_mbox().

 drivers/remoteproc/xlnx_r5_remoteproc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c
index 3349d1877751..e36918b8d234 100644
--- a/drivers/remoteproc/xlnx_r5_remoteproc.c
+++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
@@ -279,6 +279,8 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev)
 	if (!ipi)
 		return NULL;
 
+	INIT_WORK(&ipi->mbox_work, handle_event_notified);
+
 	mbox_cl = &ipi->mbox_cl;
 	mbox_cl->rx_callback = zynqmp_r5_mb_rx_cb;
 	mbox_cl->tx_block = false;
@@ -305,8 +307,6 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev)
 		return NULL;
 	}
 
-	INIT_WORK(&ipi->mbox_work, handle_event_notified);
-
 	return ipi;
 }
 
@@ -325,6 +325,8 @@ static void zynqmp_r5_free_mbox(struct mbox_info *ipi)
 		ipi->rx_chan = NULL;
 	}
 
+	cancel_work_sync(&ipi->mbox_work);
+
 	kfree(ipi);
 }
 
-- 
2.34.1

Re: [RFC PATCH v2] remoteproc: xlnx: initialize mailbox work before requesting channels

[Shah, Tanmay]

Reviewed-by: Tanmay Shah <tanmay.shah@amd.com>

On 7/16/2026 9:31 PM, Runyu Xiao wrote:
> zynqmp_r5_setup_mbox() installs zynqmp_r5_mb_rx_cb() as the mailbox RX
> callback before requesting the mailbox channels, but initializes
> ipi->mbox_work only after both channels have been requested. Once the RX
> channel is active, a notification delivered before the late INIT_WORK()
> would make the callback queue an uninitialized work item.
> 
> Initialize the work item before requesting channels. Also drain the work
> before freeing th\x13\x01mailbox state, after the channels have been released so
> no new callbacks can queue it.
> 
> This issue was found by our static analysis tool and then confirmed by
> manual review of the mailbox setup sequence. The callback is published
> before the channel requests complete, so the work item should be ready
> before the mailbox provider can invoke it.
> 
> A QEMU PoC modeled a mailbox notification delivered after the RX callback
> became reachable but before the delayed INIT_WORK(). DEBUG_OBJECTS reported
> queueing an uninitialized work item from the zynqmp_r5_setup_mbox() path.
> 
> This is sent as an RFC because the practical trigger depends on the ZynqMP
> IPI mailbox provider and firmware delivery timing. If the provider cannot
> invoke the RX callback until after setup returns, this is a defensive
> lifecycle cleanup rather than a reachable race on current systems.
> 
> Fixes: 5dfb28c257b7 ("remoteproc: xilinx: Add mailbox channels for rpmsg")
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
> Changes in v2:
> - Follow Tanmay's suggestion and keep zynqmp_r5_setup_mbox() taking the
>   child device pointer. Do not move the r5_core assignment in this patch.
> - Only move INIT_WORK() before channel requests and drain the work in
>   zynqmp_r5_free_mbox().
> 
>  drivers/remoteproc/xlnx_r5_remoteproc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c
> index 3349d1877751..e36918b8d234 100644
> --- a/drivers/remoteproc/xlnx_r5_remoteproc.c
> +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
> @@ -279,6 +279,8 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev)
>  	if (!ipi)
>  		return NULL;
>  
> +	INIT_WORK(&ipi->mbox_work, handle_event_notified);
> +
>  	mbox_cl = &ipi->mbox_cl;
>  	mbox_cl->rx_callback = zynqmp_r5_mb_rx_cb;
>  	mbox_cl->tx_block = false;
> @@ -305,8 +307,6 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev)
>  		return NULL;
>  	}
>  
> -	INIT_WORK(&ipi->mbox_work, handle_event_notified);
> -
>  	return ipi;
>  }
>  
> @@ -325,6 +325,8 @@ static void zynqmp_r5_free_mbox(struct mbox_info *ipi)
>  		ipi->rx_chan = NULL;
>  	}
>  
> +	cancel_work_sync(&ipi->mbox_work);
> +
>  	kfree(ipi);
>  }
>