Re: [PATCH] irqchip/gic-v5: Install root IRQ handler last in gicv5_init_common()

[Marc Zyngier <maz@kernel.org>]

On Wed, 02 Sep 2026 15:32:55 +0100,
Jiangshan Yi <yijiangshan@kylinos.cn> wrote:
> 
> set_handle_irq() is irreversible: once installed, handle_arch_irq
> points at gicv5_handle_irq, which dereferences the PPI/SPI/LPI
> irqdomains.  If gicv5_irs_enable() fails afterwards, the error path
> calls gicv5_free_domains() while handle_arch_irq still references
> those domains.

I don't think that's a massive problem. If you can't enable the IRS,
you can't do much at all. Your system is already completely dead.

>
> Move set_handle_irq() after gicv5_irs_enable() so the handler is
> only installed once all fallible steps have succeeded.  Drop the
> no-op set_handle_irq(NULL) in the error path, which never worked
> because set_handle_irq() returns -EBUSY once a handler is set.

That is a reasonable cleanup.

>
> Fixes: 183750b276c2 ("irqchip/gic-v5: Fix gicv5_init_common() error paths")
> Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
> ---
>  drivers/irqchip/irq-gic-v5.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v5.c b/drivers/irqchip/irq-gic-v5.c
> index ac2d423b1723..37184942dc11 100644
> --- a/drivers/irqchip/irq-gic-v5.c
> +++ b/drivers/irqchip/irq-gic-v5.c
> @@ -1166,21 +1166,19 @@ static int __init gicv5_init_common(struct fwnode_handle *parent_domain)
>  	if (ret)
>  		goto out_int;
>  
> -	ret = set_handle_irq(gicv5_handle_irq);
> +	ret = gicv5_irs_enable();
>  	if (ret)
>  		goto out_int;
>  
> -	ret = gicv5_irs_enable();
> +	ret = set_handle_irq(gicv5_handle_irq);
>  	if (ret)
> -		goto out_handle;
> +		goto out_int;

Under which circumstance can this fail? There should be none. So if
this ever returns an error, we should immediately stop with a panic.
No need to clean anything up, just die on the spot.

>  
>  	gicv5_smp_init();
>  
>  	gicv5_irs_its_probe();
>  	return 0;
>  
> -out_handle:
> -	set_handle_irq(NULL);
>  out_int:
>  	gicv5_cpu_disable_interrupts();
>  	gicv5_free_domains();

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.