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



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

diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 5f9913367d..918b9d4d4a 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -150,13 +150,18 @@ static void __init pl011_init_postirq(struct serial_port *port)
     struct pl011 *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 = pl011_interrupt;
+    uart->irqaction.name    = "pl011";
+    uart->irqaction.dev_id  = port;
+    if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 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);
+        printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq);
+        /* Do not unmask interrupts if irq handler wasn't set */
+        return;
     }
 
     /* Clear pending error interrupts */
-- 
2.43.0

On 09/04/2026 15:50, 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@epam.com>
> ---
> 
> 
> 
>  xen/drivers/char/pl011.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
> index 5f9913367d..918b9d4d4a 100644
> --- a/xen/drivers/char/pl011.c
> +++ b/xen/drivers/char/pl011.c
> @@ -150,13 +150,18 @@ static void __init pl011_init_postirq(struct serial_port *port)
>      struct pl011 *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 = pl011_interrupt;
> +    uart->irqaction.name    = "pl011";
> +    uart->irqaction.dev_id  = port;
> +    if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 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);
> +        printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq);
> +        /* Do not unmask interrupts if irq handler wasn't set */
> +        return;
>      }
>  
>      /* Clear pending error interrupts */
I think we should clear pending errors every time. Other than that, the patch is
ok. Provided the remark is addressed:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal