Re: [PATCH 1/2] mailbox: zynqmp-ipi: Fix CPU hotplug state cleanup

["Pandey, Radhey Shyam" <radheys@amd.com>]

On 11/3/2026 11:50 AM, Prasanna Kumar T S M wrote:
> On driver removal, the CPUHP_AP_ONLINE_DYN CPU hotplug state is removed
> instead of the its actual state. This can leave the mailbox driver
> callbacks dangling. Use the actual CPU hotplug state during driver
> removal.
> 
> Hotplug registration failures are also ignored, allowing probe to
> succeed without the callbacks needed to enable interrupts on CPUs
> brought online later.
> 
> Fixes: 6ffb1635341be ("mailbox: zynqmp: handle SGI for shared IPI")

Nit - only first 12 characters of the SHA-1 ID are sufficient.

Also describe the failure signature and how the error is reproduced.


> Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
> ---
>   drivers/mailbox/zynqmp-ipi-mailbox.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
> index 42d2583e44ca..b0643ee75db0 100644
> --- a/drivers/mailbox/zynqmp-ipi-mailbox.c
> +++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
> @@ -136,6 +136,7 @@ struct zynqmp_ipi_pdata {
>   	unsigned int method;
>   	u32 local_id;
>   	int virq_sgi;
> +	enum cpuhp_state hp_state;

Warning: drivers/mailbox/zynqmp-ipi-mailbox.c:141 struct member 
'hp_state' not described in 'zynqmp_ipi_pdata'

>   	int num_mboxes;
>   	struct zynqmp_ipi_mbox ipi_mboxes[] __counted_by(num_mboxes);
>   };
> @@ -863,15 +864,22 @@ static int xlnx_mbox_init_sgi(struct platform_device *pdev,
>   	irq_set_status_flags(pdata->virq_sgi, IRQ_PER_CPU);
>   
>   	/* Setup function for the CPU hot-plug cases */
> -	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "mailbox/sgi:starting",
> -			  xlnx_mbox_cpuhp_start, xlnx_mbox_cpuhp_down);
> +	pdata->hp_state = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> +			  "mailbox/sgi:starting", xlnx_mbox_cpuhp_start,

what is :starting here ?


> +			  xlnx_mbox_cpuhp_down);
> +	if (pdata->hp_state < 0) {
> +		irq_clear_status_flags(pdata->virq_sgi, IRQ_PER_CPU);
> +		free_percpu_irq(pdata->virq_sgi, &per_cpu_pdata);
> +		irq_dispose_mapping(pdata->virq_sgi);
> +		return pdata->hp_state;

  1. The new error path double-frees and then hits BUG().
Please reconfirm.

zynqmp_ipi_probe() sets pdata->irq_type before calling 
xlnx_mbox_init_sgi(), and its only error label runs
   zynqmp_ipi_free_mboxes() → xlnx_mbox_cleanup_sgi() whenever irq_type 
== IPI_IRQ_TYPE_SGI. So the new unwind runs, then the caller's unwind 
repeats
   irq_clear_status_flags(), free_percpu_irq() and 
irq_dispose_mapping(). Before any of that, cleanup calls 
cpuhp_remove_state(pdata->hp_state) with the negative errno still in the 
field, and _cpuhp_remove_state_cpuslocked() opens with 
BUG_ON(cpuhp_cb_check(state)), which rejects anything <= CPUHP_OFFLINE. 
A failed hotplug registration now panics instead of failing probe.


> +	}
>   
>   	return ret;
>   }
>   
>   static void xlnx_mbox_cleanup_sgi(struct zynqmp_ipi_pdata *pdata)
>   {
> -	cpuhp_remove_state(CPUHP_AP_ONLINE_DYN);
> +	cpuhp_remove_state(pdata->hp_state);
>   
>   	on_each_cpu(xlnx_disable_percpu_irq, NULL, 1);
>