Re: [PATCH v4 04/12] xen/arm/irq: add handling for IRQs in the eSPI range

[Oleksandr Tyshchenko <olekstysh@gmail.com>]

On 27.08.25 21:24, Leonid Komarianskyi wrote:


Hello Leonid

> Currently, Xen does not support eSPI interrupts, leading
> to a data abort when such interrupts are defined in the DTS.
> 
> This patch introduces a separate array to initialize up to
> 1024 interrupt descriptors in the eSPI range and adds the
> necessary defines and helper function. These changes lay the
> groundwork for future implementation of full eSPI interrupt
> support. As this GICv3.1 feature is not required by all vendors,
> all changes are guarded by ifdefs, depending on the corresponding
> Kconfig option.
> 
> Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@epam.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> 
> ---
> Changes in V4:
> - removed redundant line with 'default n' in Kconfig, as it is disabled
>    by default, without explicit specification
> - added reviewed-by from Volodymyr Babchuk


only NITs below

Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

> 
> Changes in V3:
> - introduced a new define NR_ESPI_IRQS to avoid confusion, like in the
>    case of using NR_IRQS for espi_desc array
> - implemented helper functions espi_to_desc and init_espi_data to make
>    it possible to add stubs with the same name, and as a result, reduce
>    the number of #ifdefs
> - disable CONFIG_GICV3_ESPI default value to n
> 
> Changes in V2:
> - use (ESPI_MAX_INTID + 1) instead of (ESPI_BASE_INTID + NR_IRQS)
> - remove unnecessary comment for nr_irqs initialization
> ---
>   xen/arch/arm/Kconfig           |  8 ++++++
>   xen/arch/arm/include/asm/irq.h | 26 +++++++++++++++++
>   xen/arch/arm/irq.c             | 52 +++++++++++++++++++++++++++++++++-
>   3 files changed, 85 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 17df147b25..43b05533b1 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -135,6 +135,14 @@ config GICV3
>   	  Driver for the ARM Generic Interrupt Controller v3.
>   	  If unsure, use the default setting.
>   
> +config GICV3_ESPI
> +	bool "Extended SPI range support"
> +	depends on GICV3 && !NEW_VGIC
> +	help
> +	  Allow Xen and domains to use interrupt numbers from the extended SPI
> +	  range, from 4096 to 5119. This feature is introduced in GICv3.1
> +	  architecture.
> +
>   config HAS_ITS
>           bool "GICv3 ITS MSI controller support (UNSUPPORTED)" if UNSUPPORTED
>           depends on GICV3 && !NEW_VGIC && !ARM_32
> diff --git a/xen/arch/arm/include/asm/irq.h b/xen/arch/arm/include/asm/irq.h
> index 5bc6475eb4..221dbf23a2 100644
> --- a/xen/arch/arm/include/asm/irq.h
> +++ b/xen/arch/arm/include/asm/irq.h
> @@ -32,6 +32,15 @@ struct arch_irq_desc {
>   #define SPI_MAX_INTID   1019
>   #define LPI_OFFSET      8192
>   
> +#ifdef CONFIG_GICV3_ESPI
> +#define ESPI_BASE_INTID 4096
> +#define ESPI_MAX_INTID  5119
> +#define NR_ESPI_IRQS    1024
> +
> +#define ESPI_INTID2IDX(intid) ((intid) - ESPI_BASE_INTID)
> +#define ESPI_IDX2INTID(idx)   ((idx) + ESPI_BASE_INTID)
> +#endif
> +
>   /* LPIs are always numbered starting at 8192, so 0 is a good invalid case. */
>   #define INVALID_LPI     0
>   
> @@ -39,7 +48,15 @@ struct arch_irq_desc {
>   #define INVALID_IRQ     1023
>   
>   extern const unsigned int nr_irqs;
> +#ifdef CONFIG_GICV3_ESPI
> +/*
> + * This will also cover the eSPI range, as some critical devices
> + * for booting Xen (e.g., serial) may use this type of interrupts.
> + */
> +#define nr_static_irqs (ESPI_MAX_INTID + 1)
> +#else
>   #define nr_static_irqs NR_IRQS
> +#endif
>   
>   struct irq_desc;
>   struct irqaction;
> @@ -55,6 +72,15 @@ static inline bool is_lpi(unsigned int irq)
>       return irq >= LPI_OFFSET;
>   }
>   
> +static inline bool is_espi(unsigned int irq)
> +{
> +#ifdef CONFIG_GICV3_ESPI
> +    return (irq >= ESPI_BASE_INTID && irq <= ESPI_MAX_INTID);
> +#else
> +    return false;
> +#endif
> +}
> +
>   #define domain_pirq_to_irq(d, pirq) (pirq)
>   
>   bool is_assignable_irq(unsigned int irq);
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index b8eccfc924..adb5e49ea3 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -19,7 +19,11 @@
>   #include <asm/gic.h>
>   #include <asm/vgic.h>
>   
> +#ifdef CONFIG_GICV3_ESPI
> +const unsigned int nr_irqs = ESPI_MAX_INTID + 1;
> +#else
>   const unsigned int nr_irqs = NR_IRQS;
> +#endif
>   
>   static unsigned int local_irqs_type[NR_LOCAL_IRQS];
>   static DEFINE_SPINLOCK(local_irqs_type_lock);
> @@ -46,6 +50,49 @@ void irq_end_none(struct irq_desc *irq)
>   }
>   
>   static irq_desc_t irq_desc[NR_IRQS - NR_LOCAL_IRQS];
> +#ifdef CONFIG_GICV3_ESPI
> +static irq_desc_t espi_desc[NR_ESPI_IRQS];

NIT: I would write "TODO: Consider allocating an array dynamically if
there is a need to enable GICV3_ESPI by default". But it is up to you.

> +
> +static struct irq_desc *espi_to_desc(unsigned int irq)
> +{
> +    return &espi_desc[ESPI_INTID2IDX(irq)];
> +}
> +
> +static int __init init_espi_data(void)
> +{
> +    int irq;

NIT: please use unsigned int


> +
> +    for ( irq = ESPI_BASE_INTID; irq <= ESPI_MAX_INTID; irq++ )
> +    {
> +        struct irq_desc *desc = irq_to_desc(irq);
> +        int rc = init_one_irq_desc(desc);
> +
> +        if ( rc )
> +            return rc;
> +
> +        desc->irq = irq;
> +        desc->action  = NULL;
> +    }
> +
> +    return 0;
> +}

[snip]