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.

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@epam.com>
---

 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);
-- 
2.43.0

base-commit: 599e3daf63ee2a83bb7096c452aa05b4aea84f2f
branch: amoi_dfmea_scif

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@epam.com>
That's a good catch. I can see most of our drivers already use that.
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

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);

Hi Michal,

I'll prepare v2 with the fixes.

--

Oleksii

On 09/04/2026 11:17, Orzel, Michal wrote:
>
> 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@epam.com>
> That's a good catch. I can see most of our drivers already use that.
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
>
> 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);