In cuart_init_postirq(), two code paths could reach the
interrupt-enable write to IER 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 IER write, enabling the receive
  data interrupt with no handler installed.

- When setup_irq() returned an error, only an error message was
  printed and execution continued to the IER write, arming the
  receive hardware interrupt line with no handler to service it. On
  platforms where the GIC receives this asserted line, the result is
  either repeated spurious-interrupt warnings or an unhandled
  interrupt fault.

Restructure cuart_init_postirq() to use early returns in both error
paths.

Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---



 xen/drivers/char/cadence-uart.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/xen/drivers/char/cadence-uart.c b/xen/drivers/char/cadence-uart.c
index b2f379833f..a63dc4adb2 100644
--- a/xen/drivers/char/cadence-uart.c
+++ b/xen/drivers/char/cadence-uart.c
@@ -72,13 +72,18 @@ static void __init cuart_init_postirq(struct serial_port *port)
     struct cuart *uart = port->uart;
     int rc;
 
-    if ( uart->irq > 0 )
+    /* Don't unmask interrupts if no valid irq was provided */
+    if ( uart->irq <= 0 )
+        return;
+
+    uart->irqaction.handler = cuart_interrupt;
+    uart->irqaction.name    = "cadence-uart";
+    uart->irqaction.dev_id  = port;
+    if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
     {
-        uart->irqaction.handler = cuart_interrupt;
-        uart->irqaction.name    = "cadence-uart";
-        uart->irqaction.dev_id  = port;
-        if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
-            printk("ERROR: Failed to allocate cadence-uart IRQ %d\n", uart->irq);
+        printk("ERROR: Failed to allocate cadence-uart IRQ %d\n", uart->irq);
+        /* Do not unmask interrupts if irq handler wasn't set */
+        return;
     }
 
     /* Clear pending error interrupts */
-- 
2.43.0

On 4/9/26 16:50, Oleksii Moisieiev wrote:

Hello Oleksii


> In cuart_init_postirq(), two code paths could reach the
> interrupt-enable write to IER 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 IER write, enabling the receive
>    data interrupt with no handler installed.
> 
> - When setup_irq() returned an error, only an error message was
>    printed and execution continued to the IER write, arming the
>    receive hardware interrupt line with no handler to service it. On
>    platforms where the GIC receives this asserted line, the result is
>    either repeated spurious-interrupt warnings or an unhandled
>    interrupt fault.
> 
> Restructure cuart_init_postirq() to use early returns in both error
> paths.
> 
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> ---
> 
> 
> 
>   xen/drivers/char/cadence-uart.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/drivers/char/cadence-uart.c b/xen/drivers/char/cadence-uart.c
> index b2f379833f..a63dc4adb2 100644
> --- a/xen/drivers/char/cadence-uart.c
> +++ b/xen/drivers/char/cadence-uart.c
> @@ -72,13 +72,18 @@ static void __init cuart_init_postirq(struct serial_port *port)
>       struct cuart *uart = port->uart;
>       int rc;
>   
> -    if ( uart->irq > 0 )
> +    /* Don't unmask interrupts if no valid irq was provided */
> +    if ( uart->irq <= 0 )

But irq field is defined as an unsigned int. By definition, an unsigned 
int can never be less than zero.

> +        return;
> +
> +    uart->irqaction.handler = cuart_interrupt;
> +    uart->irqaction.name    = "cadence-uart";
> +    uart->irqaction.dev_id  = port;
> +    if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
>       {
> -        uart->irqaction.handler = cuart_interrupt;
> -        uart->irqaction.name    = "cadence-uart";
> -        uart->irqaction.dev_id  = port;
> -        if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
> -            printk("ERROR: Failed to allocate cadence-uart IRQ %d\n", uart->irq);
> +        printk("ERROR: Failed to allocate cadence-uart IRQ %d\n", uart->irq);

NIT: The format specifier should be %u

> +        /* Do not unmask interrupts if irq handler wasn't set */
> +        return;
>       }
>   
>       /* Clear pending error interrupts */


I notice that with this change, the block that clears pending error 
interrupts is now skipped if setup_irq() fails or if no valid IRQ is 
provided. Is this intentional / OK?
In "[PATCH v2 1/4] xen/drivers/char: fix SCIF IRQ registration failure 
propagation", the error status registers are always cleared, even on 
failure.