[PATCH v2 3/3] xen/arm: handle irq_set_type() failures
Mykola Kvach <[email protected]>
| Newsgroups | gmane.comp.emulators.xen.devel |
|---|---|
| Message-ID | <d4087afce93cd4bb1779507393ac74c5dd5baea3.1786385827.git.mykola_kvach@epam.com> |
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. 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 UART or notification setup when trigger configuration fails. Signed-off-by: Mykola Kvach <[email protected]> --- Changes in v2: - new patch. --- xen/arch/arm/gic-v2.c | 8 ++++++-- xen/arch/arm/gic-v3.c | 8 ++++++-- xen/arch/arm/tee/ffa_notif.c | 11 ++++++++++- xen/arch/arm/time.c | 18 ++++++++++++++---- xen/drivers/char/ns16550.c | 5 ++++- xen/drivers/char/pl011.c | 4 +++- 6 files changed, 43 insertions(+), 11 deletions(-) diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c index 43a379fdda..b8dcbb0bb4 100644 --- a/xen/arch/arm/gic-v2.c +++ b/xen/arch/arm/gic-v2.c @@ -1157,6 +1157,7 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, const unsigned long end) { static int cpu_base_assigned = 0; + int rc; struct acpi_madt_generic_interrupt *processor = container_of(header, struct acpi_madt_generic_interrupt, header); @@ -1173,9 +1174,12 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, 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); + rc = irq_set_type(gicv2_info.maintenance_irq, IRQ_TYPE_EDGE_BOTH); else - irq_set_type(gicv2_info.maintenance_irq, IRQ_TYPE_LEVEL_MASK); + rc = irq_set_type(gicv2_info.maintenance_irq, IRQ_TYPE_LEVEL_MASK); + + if ( rc ) + return rc; cpu_base_assigned = 1; } diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index acdac22953..d92d0b9b3c 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -1734,6 +1734,7 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, const unsigned long end) { static int cpu_base_assigned = 0; + int rc; struct acpi_madt_generic_interrupt *processor = container_of(header, struct acpi_madt_generic_interrupt, header); @@ -1748,9 +1749,12 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, 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); + rc = irq_set_type(gicv3_info.maintenance_irq, IRQ_TYPE_EDGE_BOTH); else - irq_set_type(gicv3_info.maintenance_irq, IRQ_TYPE_LEVEL_MASK); + rc = irq_set_type(gicv3_info.maintenance_irq, IRQ_TYPE_LEVEL_MASK); + + if ( rc ) + return rc; cpu_base_assigned = 1; } diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c index 186e726412..d08d0a3366 100644 --- a/xen/arch/arm/tee/ffa_notif.c +++ b/xen/arch/arm/tee/ffa_notif.c @@ -407,7 +407,16 @@ 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 6955b2788f..39b5eabe7c 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 120ac09d23..4bfdcfebd7 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -1928,6 +1928,7 @@ static int __init ns16550_acpi_uart_init(const void *data) struct acpi_table_header *table; struct acpi_table_spcr *spcr; acpi_status status; + int rc; /* * Same as the DT part. * Only support one UART on ARM which happen to be ns16550_com[0]. @@ -1976,7 +1977,9 @@ 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); + rc = irq_set_type(spcr->interrupt, IRQ_TYPE_LEVEL_HIGH); + if ( rc ) + return rc; uart->irq = spcr->interrupt; uart->vuart.base_addr = uart->io_base; diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c index a336241033..97c53c11e0 100644 --- a/xen/drivers/char/pl011.c +++ b/xen/drivers/char/pl011.c @@ -363,7 +363,9 @@ 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); + res = irq_set_type(spcr->interrupt, IRQ_TYPE_LEVEL_HIGH); + if ( res ) + return res; /* TODO - mmio32 proper handling (for now set to true) */ res = pl011_uart_init(spcr->interrupt, spcr->serial_port.address, -- 2.43.0