Hi Leonid,
Leonid Komarianskyi <leonid_komarians...@epam.com> writes:
> 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_komarians...@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babc...@epam.com>
>
> ---
> Changes in V7:
> - fixed the condition in the is_espi assert for non-eSPI builds: the
> previous condition was mistakenly added with a wrong check and led to
> triggering asserts for all non-eSPI INTIDs, when it should be triggered
> in this case in the other way around
> - minor: used is_espi() in the espi_intid_to_idx() ASSERT, as is_espi
> performs the same verification
>
> Changes in V6:
> - added an assert in is_espi() when CONFIG_GICV3_ESPI=n to ensure that
> out-of-range array resources are not accessed, e.g., in __irq_to_desc()
> - removed unnecessary parentheses in is_espi()
> - converted helper macro to inline functions and added sanity checks
> with ASSERTs to them
> - defined espi_to_desc for non-eSPI builds as a prototype
> - updates the comments
> - used the IS_ENABLED(CONFIG_GICV3_ESPI) macro to initialize nr_irqs
>
> Changes in V5:
> - no functional changes introduced by this version compared with V4, only
> minor fixes and removal of ifdefs for macroses
> - added TODO comment, suggested by Oleksandr Tyshchenko
> - changed int to unsigned int for irqs
> - removed ifdefs for eSPI-specific defines and macros to reduce the
> number of ifdefs and code duplication in further changes
> - removed reviewed-by as moving defines from ifdefs requires additional
> confirmation from reviewers
>
> 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
>
> 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 | 37 ++++++++++++++++++++++++
> xen/arch/arm/irq.c | 53 ++++++++++++++++++++++++++++++++--
> 3 files changed, 96 insertions(+), 2 deletions(-)
>
> 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..2ff2d07d6d 100644
> --- a/xen/arch/arm/include/asm/irq.h
> +++ b/xen/arch/arm/include/asm/irq.h
> @@ -32,6 +32,10 @@ struct arch_irq_desc {
> #define SPI_MAX_INTID 1019
> #define LPI_OFFSET 8192
>
> +#define ESPI_BASE_INTID 4096
> +#define ESPI_MAX_INTID 5119
> +#define NR_ESPI_IRQS 1024
> +
> /* LPIs are always numbered starting at 8192, so 0 is a good invalid case. */
> #define INVALID_LPI 0
>
> @@ -39,7 +43,12 @@ struct arch_irq_desc {
> #define INVALID_IRQ 1023
>
> extern const unsigned int nr_irqs;
> +#ifdef CONFIG_GICV3_ESPI
> +/* This will cover the eSPI range, to allow asignmant of eSPIs to domains. */
> +#define nr_static_irqs (ESPI_MAX_INTID + 1)
> +#else
> #define nr_static_irqs NR_IRQS
> +#endif
>
> struct irq_desc;
> struct irqaction;
> @@ -55,6 +64,34 @@ 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
> + /*
> + * The function should not be called for eSPIs when CONFIG_GICV3_ESPI is
> + * disabled. Returning false allows the compiler to optimize the code
> + * when the config is disabled, while the assert ensures that
> out-of-range
> + * array resources are not accessed, e.g., in __irq_to_desc().
> + */
> + ASSERT(!(irq >= ESPI_BASE_INTID && irq <= ESPI_MAX_INTID));
> + return false;
> +#endif
> +}
> +
> +static inline unsigned int espi_intid_to_idx(unsigned int intid)
> +{
> + ASSERT(is_espi(intid));
> + return intid - ESPI_BASE_INTID;
> +}
> +
> +static inline unsigned int espi_idx_to_intid(unsigned int idx)
> +{
> + ASSERT(idx <= NR_ESPI_IRQS);
> + return idx + ESPI_BASE_INTID;
> +}
> +
> #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..c934d39bf6 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -19,7 +19,9 @@
> #include <asm/gic.h>
> #include <asm/vgic.h>
>
> -const unsigned int nr_irqs = NR_IRQS;
> +const unsigned int nr_irqs = IS_ENABLED(CONFIG_GICV3_ESPI) ?
> + (ESPI_MAX_INTID + 1) :
> + NR_IRQS;
>
> static unsigned int local_irqs_type[NR_LOCAL_IRQS];
> static DEFINE_SPINLOCK(local_irqs_type_lock);
> @@ -46,6 +48,50 @@ void irq_end_none(struct irq_desc *irq)
> }
>
> static irq_desc_t irq_desc[NR_IRQS - NR_LOCAL_IRQS];
> +#ifdef CONFIG_GICV3_ESPI
> +/* TODO: Consider allocating an array dynamically */
> +static irq_desc_t espi_desc[NR_ESPI_IRQS];
> +
> +static struct irq_desc *espi_to_desc(unsigned int irq)
> +{
> + return &espi_desc[espi_intid_to_idx(irq)];
> +}
> +
> +static int __init init_espi_data(void)
> +{
> + unsigned int irq;
> +
> + 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;
> +}
> +#else
> +/*
> + * Defined as a prototype as it should not be called if CONFIG_GICV3_ESPI=n.
> + * Without CONFIG_GICV3_ESPI, the additional 1024 IRQ descriptors will not
> + * be defined, and thus, they cannot be used. Unless INTIDs from the eSPI
> + * range are mistakenly defined in Xen DTS when the appropriate config is
> + * disabled, this function will not be reached because is_espi will return
> + * false for non-eSPI INTIDs.
> + */
> +struct irq_desc *espi_to_desc(unsigned int irq);
> +
> +static int __init init_espi_data(void)
> +{
> + return 0;
> +}
> +#endif
> +
> static DEFINE_PER_CPU(irq_desc_t[NR_LOCAL_IRQS], local_irq_desc);
>
> struct irq_desc *__irq_to_desc(unsigned int irq)
> @@ -53,6 +99,9 @@ struct irq_desc *__irq_to_desc(unsigned int irq)
> if ( irq < NR_LOCAL_IRQS )
> return &this_cpu(local_irq_desc)[irq];
>
> + if ( is_espi(irq) )
> + return espi_to_desc(irq);
> +
> return &irq_desc[irq-NR_LOCAL_IRQS];
> }
>
> @@ -79,7 +128,7 @@ static int __init init_irq_data(void)
> desc->action = NULL;
> }
>
> - return 0;
> + return init_espi_data();
> }
>
> static int init_local_irq_data(unsigned int cpu)
--
WBR, Volodymyr
