[PATCH V2 0/4] mailbox: zynqmp-ipi: Improve resource cleanup on driver unbind

[PATCH V2 0/4] mailbox: zynqmp-ipi: Improve resource cleanup on driver unbind

[Harini T]

1. Remove redundant mbox_controller_unregister() call, as
device_unregister() already handles controller cleanup.
2. Remove ipi_mbox->dev.parent check in zynqmp_ipi_free_mboxes() as
device_is_registered() is the canonical and robust method to verify the
device registration status.
3. Fix the mailbox cleanup loop to prevent out-of-bounds array access.
4. Fix SGI resource cleanup by using an explicit irq_type field for
reliable SGI detection, replacing the unreliable IRQ number check.

---
Changes in V2:
- Address review comments from V1.
- Patch 1/3 from V1 has been split into two patches in which the first
  focuses on removing the redundant mbox_controller_unregister() call
  and the second focuses on removing ipi_mbox->dev.parent check.
- Avoid using i-- as the 1st param in for loop.
- Avoid using variable name in the commit message for patch 3/4.
- Add Fixes tag for all the patches in the series.

V1 link: https://lore.kernel.org/all/20250822045732.1068103-1-harini.t@amd.com/
---

Harini T (4):
  mailbox: zynqmp-ipi: Remove redundant mbox_controller_unregister()
    call
  mailbox: zynqmp-ipi: Remove dev.parent check in zynqmp_ipi_free_mboxes
  mailbox: zynqmp-ipi: Fix out-of-bounds access in mailbox cleanup loop
  mailbox: zynqmp-ipi: Fix SGI cleanup on unbind

 drivers/mailbox/zynqmp-ipi-mailbox.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

-- 
2.43.0

[PATCH V2 1/4] mailbox: zynqmp-ipi: Remove redundant mbox_controller_unregister() call

[Harini T]

The controller is registered using the device-managed function
'devm_mbox_controller_register()'. As documented in mailbox.c, this
ensures the devres framework automatically calls
mbox_controller_unregister() when device_unregister() is invoked, making
the explicit call unnecessary.

Remove redundant mbox_controller_unregister() call as
device_unregister() handles controller cleanup.

Fixes: 4981b82ba2ff ("mailbox: ZynqMP IPI mailbox controller")
Signed-off-by: Harini T <harini.t@amd.com>
---
 drivers/mailbox/zynqmp-ipi-mailbox.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
index 0c143beaafda..263a3413a8c7 100644
--- a/drivers/mailbox/zynqmp-ipi-mailbox.c
+++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
@@ -894,7 +894,6 @@ static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata)
 	for (; i >= 0; i--) {
 		ipi_mbox = &pdata->ipi_mboxes[i];
 		if (ipi_mbox->dev.parent) {
-			mbox_controller_unregister(&ipi_mbox->mbox);
 			if (device_is_registered(&ipi_mbox->dev))
 				device_unregister(&ipi_mbox->dev);
 		}
-- 
2.43.0

Re: [PATCH V2 1/4] mailbox: zynqmp-ipi: Remove redundant mbox_controller_unregister() call

[Peng Fan]

On Mon, Sep 29, 2025 at 01:07:20PM +0530, Harini T wrote:
>The controller is registered using the device-managed function
>'devm_mbox_controller_register()'. As documented in mailbox.c, this
>ensures the devres framework automatically calls
>mbox_controller_unregister() when device_unregister() is invoked, making
>the explicit call unnecessary.
>
>Remove redundant mbox_controller_unregister() call as
>device_unregister() handles controller cleanup.
>
>Fixes: 4981b82ba2ff ("mailbox: ZynqMP IPI mailbox controller")
>Signed-off-by: Harini T <harini.t@amd.com>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

[PATCH V2 2/4] mailbox: zynqmp-ipi: Remove dev.parent check in zynqmp_ipi_free_mboxes

[Harini T]

The ipi_mbox->dev.parent check is unreliable proxy for registration
status as it fails to protect against probe failures that occur after
the parent is assigned but before device_register() completes.

device_is_registered() is the canonical and robust method to verify the
registration status.

Remove ipi_mbox->dev.parent check in zynqmp_ipi_free_mboxes().

Fixes: 4981b82ba2ff ("mailbox: ZynqMP IPI mailbox controller")
Signed-off-by: Harini T <harini.t@amd.com>
---
 drivers/mailbox/zynqmp-ipi-mailbox.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
index 263a3413a8c7..bdcc6937ee30 100644
--- a/drivers/mailbox/zynqmp-ipi-mailbox.c
+++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
@@ -893,10 +893,8 @@ static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata)
 	i = pdata->num_mboxes;
 	for (; i >= 0; i--) {
 		ipi_mbox = &pdata->ipi_mboxes[i];
-		if (ipi_mbox->dev.parent) {
-			if (device_is_registered(&ipi_mbox->dev))
-				device_unregister(&ipi_mbox->dev);
-		}
+		if (device_is_registered(&ipi_mbox->dev))
+			device_unregister(&ipi_mbox->dev);
 	}
 }
 
-- 
2.43.0

Re: [PATCH V2 2/4] mailbox: zynqmp-ipi: Remove dev.parent check in zynqmp_ipi_free_mboxes

[Peng Fan]

On Mon, Sep 29, 2025 at 01:07:21PM +0530, Harini T wrote:
>The ipi_mbox->dev.parent check is unreliable proxy for registration
>status as it fails to protect against probe failures that occur after
>the parent is assigned but before device_register() completes.
>
>device_is_registered() is the canonical and robust method to verify the
>registration status.
>
>Remove ipi_mbox->dev.parent check in zynqmp_ipi_free_mboxes().
>
>Fixes: 4981b82ba2ff ("mailbox: ZynqMP IPI mailbox controller")
>Signed-off-by: Harini T <harini.t@amd.com>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

[PATCH V2 3/4] mailbox: zynqmp-ipi: Fix out-of-bounds access in mailbox cleanup loop

[Harini T]

The cleanup loop was starting at the wrong array index, causing
out-of-bounds access.
Start the loop at the correct index for zero-indexed arrays to prevent
accessing memory beyond the allocated array bounds.

Fixes: 4981b82ba2ff ("mailbox: ZynqMP IPI mailbox controller")
Signed-off-by: Harini T <harini.t@amd.com>
---
 drivers/mailbox/zynqmp-ipi-mailbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
index bdcc6937ee30..dddbef6b2cb8 100644
--- a/drivers/mailbox/zynqmp-ipi-mailbox.c
+++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
@@ -890,7 +890,7 @@ static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata)
 	if (pdata->irq < MAX_SGI)
 		xlnx_mbox_cleanup_sgi(pdata);
 
-	i = pdata->num_mboxes;
+	i = pdata->num_mboxes - 1;
 	for (; i >= 0; i--) {
 		ipi_mbox = &pdata->ipi_mboxes[i];
 		if (device_is_registered(&ipi_mbox->dev))
-- 
2.43.0

Re: [PATCH V2 3/4] mailbox: zynqmp-ipi: Fix out-of-bounds access in mailbox cleanup loop

[Peng Fan]

On Mon, Sep 29, 2025 at 01:07:22PM +0530, Harini T wrote:
>The cleanup loop was starting at the wrong array index, causing
>out-of-bounds access.
>Start the loop at the correct index for zero-indexed arrays to prevent
>accessing memory beyond the allocated array bounds.
>
>Fixes: 4981b82ba2ff ("mailbox: ZynqMP IPI mailbox controller")
>Signed-off-by: Harini T <harini.t@amd.com>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

[PATCH V2 4/4] mailbox: zynqmp-ipi: Fix SGI cleanup on unbind

[Harini T]

The driver incorrectly determines SGI vs SPI interrupts by checking IRQ
number < 16, which fails with dynamic IRQ allocation. During unbind,
this causes improper SGI cleanup leading to kernel crash.

Add explicit irq_type field to pdata for reliable identification of SGI
interrupts (type-2) and only clean up SGI resources when appropriate.

Fixes: 6ffb1635341b ("mailbox: zynqmp: handle SGI for shared IPI")
Signed-off-by: Harini T <harini.t@amd.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mailbox/zynqmp-ipi-mailbox.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
index dddbef6b2cb8..967967b2b8a9 100644
--- a/drivers/mailbox/zynqmp-ipi-mailbox.c
+++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
@@ -62,7 +62,8 @@
 #define DST_BIT_POS	9U
 #define SRC_BITMASK	GENMASK(11, 8)
 
-#define MAX_SGI 16
+/* Macro to represent SGI type for IPI IRQs */
+#define IPI_IRQ_TYPE_SGI	2
 
 /*
  * Module parameters
@@ -121,6 +122,7 @@ struct zynqmp_ipi_mbox {
  * @dev:                  device pointer corresponding to the Xilinx ZynqMP
  *                        IPI agent
  * @irq:                  IPI agent interrupt ID
+ * @irq_type:             IPI SGI or SPI IRQ type
  * @method:               IPI SMC or HVC is going to be used
  * @local_id:             local IPI agent ID
  * @virq_sgi:             IRQ number mapped to SGI
@@ -130,6 +132,7 @@ struct zynqmp_ipi_mbox {
 struct zynqmp_ipi_pdata {
 	struct device *dev;
 	int irq;
+	unsigned int irq_type;
 	unsigned int method;
 	u32 local_id;
 	int virq_sgi;
@@ -887,7 +890,7 @@ static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata)
 	struct zynqmp_ipi_mbox *ipi_mbox;
 	int i;
 
-	if (pdata->irq < MAX_SGI)
+	if (pdata->irq_type == IPI_IRQ_TYPE_SGI)
 		xlnx_mbox_cleanup_sgi(pdata);
 
 	i = pdata->num_mboxes - 1;
@@ -956,14 +959,16 @@ static int zynqmp_ipi_probe(struct platform_device *pdev)
 		dev_err(dev, "failed to parse interrupts\n");
 		goto free_mbox_dev;
 	}
-	ret = out_irq.args[1];
+
+	/* Use interrupt type to distinguish SGI and SPI interrupts */
+	pdata->irq_type = out_irq.args[0];
 
 	/*
 	 * If Interrupt number is in SGI range, then request SGI else request
 	 * IPI system IRQ.
 	 */
-	if (ret < MAX_SGI) {
-		pdata->irq = ret;
+	if (pdata->irq_type == IPI_IRQ_TYPE_SGI) {
+		pdata->irq = out_irq.args[1];
 		ret = xlnx_mbox_init_sgi(pdev, pdata->irq, pdata);
 		if (ret)
 			goto free_mbox_dev;
-- 
2.43.0