On 04.09.25 23:01, 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_komarians...@epam.com>
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
Initially I was slightly worried about ...
---
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);
... this assert.
The system defines that there are only 1024 (NR_ESPI_IRQS) eSPIs, which
map to indices 0 through 1023. An idx of 1024 is an invalid eSPI index.
The assert would allow idx = 1024 to pass, and the helper would then
return an invalid interrupt ID of 5120 (1024 + 4096). But, as I
understand, you chose "<=" instead of "<" to be able to also pass
NR_ESPI_IRQS/gic_number_espis() to calculate a "one-past-the-end"
interrupt ID, which can then be used as an exclusive upper bound for
range checks in the code.
By looking at how it is used within the series, I did not notice obvious
issues.
+ 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)
