Re: [PATCH v3 2/4] xen/drivers/char/pl011: fix IRQ registration failure propagation

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 22/04/2026 11:33, Oleksii Moisieiev wrote:
> In pl011_init_postirq(), two code paths could reach the
> interrupt-unmask write to IMSC without a handler being registered:
> 
> - When no valid IRQ number was provided (uart->irq <= 0), the original
>   positive-condition guard (if uart->irq > 0) skipped the irqaction
>   setup but still fell through to the IMSC write, unmasking
>   RTI|OEI|BEI|PEI|FEI|TXI|RXI with no handler installed.
> 
> - When setup_irq() returned an error, only an error message was
>   printed and execution continued to the IMSC write, arming all
>   hardware interrupt lines with no handler to service them. On
>   platforms where the GIC receives these asserted lines, the result
>   is either repeated spurious-interrupt warnings or an unhandled
>   interrupt fault.
> 
> Restructure pl011_init_postirq() to use early returns: return
> immediately when no valid IRQ is provided, and return after logging
> the error when setup_irq() fails. The interrupt-enable write to IMSC
> is only reached when IRQ registration succeeds.
> 
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@xxxxxxxx>
> ---
> 
> Changes in v3:
> - clear pending error interrupts before setup_irq for pl011
> 
>  xen/drivers/char/pl011.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
> index 5f9913367d..9e308f4936 100644
> --- a/xen/drivers/char/pl011.c
> +++ b/xen/drivers/char/pl011.c
> @@ -150,18 +150,24 @@ static void __init pl011_init_postirq(struct 
> serial_port *port)
>      struct pl011 *uart = port->uart;
>      int rc;
>  
> -    if ( uart->irq > 0 )
> -    {
> -        uart->irqaction.handler = pl011_interrupt;
> -        uart->irqaction.name    = "pl011";
> -        uart->irqaction.dev_id  = port;
> -        if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
> -            printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq);
> -    }
> +    /* Don't unmask interrupts if no valid irq was provided */
> +    if ( uart->irq <= 0 )
uart->irq is unsigned, so it should be compared to 0. Can be fixed on commit.

Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

~Michal