[PATCH v6 1/4] xen/arm: handle irq_set_type() failures

[Mykola Kvach <mykola_kvach@xxxxxxxx>]

Several Arm firmware initialization paths discard irq_set_type()'s return
value, violating MISRA C Rule 17.7. If trigger configuration fails,
initialization continues with an IRQ that was not configured as requested.

GTDT and MADT retain rejected timer and maintenance INTIDs.
check_timer_irq_cfg() and release_irq() later perform unconditional
descriptor lookups on those values. Xen has no backing descriptors for
INTIDs 1024 through 4095, so retaining one can cause an out-of-bounds
access.

Check the return value in the GTDT, MADT, SPCR, and FF-A paths. Store timer
INTIDs only after successful trigger configuration, make GTDT parsing
failure fatal, and stop notification setup when trigger configuration
fails. Keep the NS16550 and PL011 consoles available by falling back to
polling, with a warning, if their IRQ trigger configuration fails.

Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
---
Changes in v6:
- Fall back to polling with a warning when ACPI IRQ configuration fails
  for NS16550 or PL011, preserving the console.

Changes in v3:
- Avoid partial state updates and simplify maintenance IRQ setup.

Changes in v2:
- New patch.
---
 xen/arch/arm/gic-v2.c        | 15 +++++++++------
 xen/arch/arm/gic-v3.c        | 15 +++++++++------
 xen/arch/arm/tee/ffa_notif.c | 10 +++++++++-
 xen/arch/arm/time.c          | 18 ++++++++++++++----
 xen/drivers/char/ns16550.c   |  8 +++++++-
 xen/drivers/char/pl011.c     | 12 ++++++++++--
 6 files changed, 58 insertions(+), 20 deletions(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index d05c574d88..87eda9d322 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -1166,17 +1166,20 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header 
*header,
     /* Read from APIC table and fill up the GIC variables */
     if ( cpu_base_assigned == 0 )
     {
+        int rc;
+
+        rc = irq_set_type(processor->vgic_interrupt,
+                          processor->flags & ACPI_MADT_VGIC_IRQ_MODE ?
+                          IRQ_TYPE_EDGE_BOTH : IRQ_TYPE_LEVEL_MASK);
+
+        if ( rc )
+            return rc;
+
         cbase = processor->base_address;
         csize = SZ_8K;
         hbase = processor->gich_base_address;
         vbase = processor->gicv_base_address;
         gicv2_info.maintenance_irq = processor->vgic_interrupt;
-
-        if ( processor->flags & ACPI_MADT_VGIC_IRQ_MODE )
-            irq_set_type(gicv2_info.maintenance_irq, IRQ_TYPE_EDGE_BOTH);
-        else
-            irq_set_type(gicv2_info.maintenance_irq, IRQ_TYPE_LEVEL_MASK);
-
         cpu_base_assigned = 1;
     }
     else
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index acdac22953..b32a9b5009 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1743,15 +1743,18 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header 
*header,
     /* Read from APIC table and fill up the GIC variables */
     if ( !cpu_base_assigned )
     {
+        int rc;
+
+        rc = irq_set_type(processor->vgic_interrupt,
+                          processor->flags & ACPI_MADT_VGIC_IRQ_MODE ?
+                          IRQ_TYPE_EDGE_BOTH : IRQ_TYPE_LEVEL_MASK);
+
+        if ( rc )
+            return rc;
+
         cbase = processor->base_address;
         vbase = processor->gicv_base_address;
         gicv3_info.maintenance_irq = processor->vgic_interrupt;
-
-        if ( processor->flags & ACPI_MADT_VGIC_IRQ_MODE )
-            irq_set_type(gicv3_info.maintenance_irq, IRQ_TYPE_EDGE_BOTH);
-        else
-            irq_set_type(gicv3_info.maintenance_irq, IRQ_TYPE_LEVEL_MASK);
-
         cpu_base_assigned = 1;
     }
     else
diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
index 186e726412..c64bc39f03 100644
--- a/xen/arch/arm/tee/ffa_notif.c
+++ b/xen/arch/arm/tee/ffa_notif.c
@@ -407,7 +407,15 @@ void ffa_notif_init(void)
         irq = resp.a2;
         notif_sri_irq = irq;
         if ( irq >= NR_GIC_SGI )
-            irq_set_type(irq, IRQ_TYPE_EDGE_RISING);
+        {
+            ret = irq_set_type(irq, IRQ_TYPE_EDGE_RISING);
+            if ( ret )
+            {
+                printk(XENLOG_ERR "ffa: irq_set_type irq %u failed: error 
%d\n",
+                       irq, ret);
+                return;
+            }
+        }
         ret = request_irq(irq, 0, notif_irq_handler, "FF-A notif", NULL);
         if ( ret )
         {
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index be54b87438..ccfb76e20f 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -60,20 +60,27 @@ static int __init arch_timer_acpi_init(struct 
acpi_table_header *header)
 {
     u32 irq_type;
     struct acpi_table_gtdt *gtdt;
+    int rc;
 
     gtdt = container_of(header, struct acpi_table_gtdt, header);
 
     /* Initialize all the generic timer IRQ variable from GTDT table */
     irq_type = acpi_get_timer_irq_type(gtdt->non_secure_el1_flags);
-    irq_set_type(gtdt->non_secure_el1_interrupt, irq_type);
+    rc = irq_set_type(gtdt->non_secure_el1_interrupt, irq_type);
+    if ( rc )
+        return rc;
     timer_irq[TIMER_PHYS_NONSECURE_PPI] = gtdt->non_secure_el1_interrupt;
 
     irq_type = acpi_get_timer_irq_type(gtdt->virtual_timer_flags);
-    irq_set_type(gtdt->virtual_timer_interrupt, irq_type);
+    rc = irq_set_type(gtdt->virtual_timer_interrupt, irq_type);
+    if ( rc )
+        return rc;
     timer_irq[TIMER_VIRT_PPI] = gtdt->virtual_timer_interrupt;
 
     irq_type = acpi_get_timer_irq_type(gtdt->non_secure_el2_flags);
-    irq_set_type(gtdt->non_secure_el2_interrupt, irq_type);
+    rc = irq_set_type(gtdt->non_secure_el2_interrupt, irq_type);
+    if ( rc )
+        return rc;
     timer_irq[TIMER_HYP_PPI] = gtdt->non_secure_el2_interrupt;
 
     return 0;
@@ -81,7 +88,10 @@ static int __init arch_timer_acpi_init(struct 
acpi_table_header *header)
 
 static void __init preinit_acpi_xen_time(void)
 {
-    acpi_table_parse(ACPI_SIG_GTDT, arch_timer_acpi_init);
+    int rc = acpi_table_parse(ACPI_SIG_GTDT, arch_timer_acpi_init);
+
+    if ( rc )
+        panic("Timer: Failed to configure interrupts from GTDT: %d\n", rc);
 }
 #else
 static void __init preinit_acpi_xen_time(void) { }
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 593d208483..cdda28879e 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1990,8 +1990,14 @@ static int __init ns16550_acpi_uart_init(const void 
*data)
     uart->reg_width = spcr->serial_port.access_width;
 
     /* The trigger/polarity information is not available in spcr. */
-    irq_set_type(spcr->interrupt, IRQ_TYPE_LEVEL_HIGH);
     uart->irq = spcr->interrupt;
+    if ( irq_set_type(spcr->interrupt, IRQ_TYPE_LEVEL_HIGH) )
+    {
+        printk(XENLOG_WARNING
+               "ns16550: unable to configure IRQ %u, using polling\n",
+               spcr->interrupt);
+        uart->irq = 0;
+    }
 
     uart->vuart.base_addr = uart->io_base;
     uart->vuart.size = uart->io_size;
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index a336241033..612d457fe0 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -347,6 +347,7 @@ static int __init pl011_acpi_uart_init(const void *data)
 {
     acpi_status status;
     struct acpi_table_spcr *spcr = NULL;
+    unsigned int irq;
     int res;
     bool sbsa;
 
@@ -363,10 +364,17 @@ static int __init pl011_acpi_uart_init(const void *data)
             spcr->interface_type == ACPI_DBG2_SBSA_32);
 
     /* trigger/polarity information is not available in spcr */
-    irq_set_type(spcr->interrupt, IRQ_TYPE_LEVEL_HIGH);
+    irq = spcr->interrupt;
+    if ( irq_set_type(irq, IRQ_TYPE_LEVEL_HIGH) )
+    {
+        printk(XENLOG_WARNING
+               "pl011: unable to configure IRQ %u, using polling\n",
+               irq);
+        irq = 0;
+    }
 
     /* TODO - mmio32 proper handling (for now set to true) */
-    res = pl011_uart_init(spcr->interrupt, spcr->serial_port.address,
+    res = pl011_uart_init(irq, spcr->serial_port.address,
                           PAGE_SIZE, sbsa, true);
     if ( res < 0 )
     {
-- 
2.53.0