Re: [PATCH] xen/drivers/char: fix SCIF IRQ registration failure propagation

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

On 08/04/2026 19:03, Oleksii Moisieiev wrote:
> In scif_uart_init_postirq(), when setup_irq() returns an error the
> failure was only logged via dprintk() and execution continued,
> unconditionally writing TIE|RIE|REIE into the Serial Control Register
> (SCSCR). This armed all three hardware interrupt lines (TX FIFO empty,
> RX data ready, receive error) with no handler registered 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.
> 
> The fix adds an early return inside the error branch. The
> interrupt-enable write to SCSCR is skipped entirely when no handler is
> registered.
> 
> SCIF TX continues to operate correctly after this change. The Xen
> serial framework never calls serial_async_transmit() for SCIF, so
> port->txbuf is always NULL. This causes __serial_putc() to take the
> synchronous finite-capacity path, which polls the SCFSR_TDFE hardware
> flag directly and does not depend on the interrupt mechanism.
NIT: It would be nice to at least mention that there will be no serial RX
without interrupts.

> 
> As a secondary clean-up, the hardware error-flag clearing sequence is
> moved to before the setup_irq() call so that error bits accumulated
> since init_preirq() are cleared unconditionally, regardless of whether
> IRQ registration succeeds.
> 
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@xxxxxxxx>
That's a good catch. I can see most of our drivers already use that.
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

That said, I can see that we have exactly the same issue for pl011, cadence and
exynos. I can either take your patch as is (no more work for you) and submit the
patch fixing remaining drivers or you can send a v2 fixing all at once. It's up
to you.

~Michal

> ---
> 
>  xen/drivers/char/scif-uart.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
> index 888821a3b8..673a2d3800 100644
> --- a/xen/drivers/char/scif-uart.c
> +++ b/xen/drivers/char/scif-uart.c
> @@ -187,16 +187,24 @@ static void __init scif_uart_init_postirq(struct 
> serial_port *port)
>      uart->irqaction.name    = "scif_uart";
>      uart->irqaction.dev_id  = port;
>  
> -    if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
> -        dprintk(XENLOG_ERR, "Failed to allocated scif_uart IRQ %d\n",
> -                uart->irq);
> -
>      /* Clear all errors */
>      if ( scif_readw(uart, params->status_reg) & params->error_mask )
>          scif_writew(uart, params->status_reg, ~params->error_mask);
>      if ( scif_readw(uart, params->overrun_reg) & params->overrun_mask )
>          scif_writew(uart, params->overrun_reg, ~params->overrun_mask);
>  
> +    if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
> +    {
> +        dprintk(XENLOG_ERR, "Failed to allocated scif_uart IRQ %d\n",
> +                uart->irq);
> +        /*
> +         * If the IRQ handler could not be installed (setup_irq failed),
> +         * do not enable TX/RX or error interrupts. Serial transmit will
> +         * fall back to polling mode.
> +         */
> +        return;
> +    }
> +
>      /* Enable TX/RX and Error Interrupts  */
>      scif_writew(uart, SCIF_SCSCR, scif_readw(uart, SCIF_SCSCR) |
>                  params->irq_flags);