Re: [PATCH v2 3/3] xen/arm: handle irq_set_type() failures

On 10/08/2026 7:38 pm, Mykola Kvach wrote:
> 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);

I know it was pre-existing, but this is an overly verbose way of writing:

rc = irq_set_type(gicv2_info.maintenance_irq,
          (processor->flags & ACPI_MADT_VGIC_IRQ_MODE)
          ? IRQ_TYPE_EDGE_BOTH
          : IRQ_TYPE_LEVEL_MASK);

I expect the optimiser can transform behind the scenes, but it's better
to make the C simpler for humans too.

~Andrew