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

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

[Jiangshan Yi]

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.

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.

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;
 
 	gicv5_smp_init();
 
 	gicv5_irs_its_probe();
 	return 0;
 
-out_handle:
-	set_handle_irq(NULL);
 out_int:
 	gicv5_cpu_disable_interrupts();
 	gicv5_free_domains();
-- 
2.25.1

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

[Marc Zyngier]

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.

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

[Jiangshan Yi]

On Wed, 02 Sep 2026 16:54:41 +0100, Marc Zyngier wrote:
> 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.

Fair enough, I'll drop the Fixes tag and reword the commit log.

> 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.

OK, v2 will panic() on failure and drop the out_handle label along
with the no-op set_handle_irq(NULL).

Thanks for the review.

Jiangshan