[PATCH v2] xen/arm: gicv3: initialize eSPI unconditionally

Leonid Komarianskyi posted 1 patch 1 day, 23 hours ago
xen/arch/arm/gic-v3.c                  | 32 ++++++++++++++------------
xen/arch/arm/include/asm/gic_v3_defs.h |  2 --
2 files changed, 17 insertions(+), 17 deletions(-)
[PATCH v2] xen/arm: gicv3: initialize eSPI unconditionally
Posted by Leonid Komarianskyi 1 day, 23 hours ago
Since the firmware may initialize eSPIs before Xen, and without
CONFIG_GICV3_ESPI enabled, Xen would not reinitialize them properly
during boot. In such cases, once the GIC is re-enabled in Xen,
interrupts may be received that cannot be handled.

To ensure proper operation on hardware with eSPI feature, even when the eSPI
config is disabled, gicv3_dist_espi_common_init() should be invoked
regardless of whether CONFIG_GICV3_ESPI is enabled or not. This will not
affect hardware without eSPI support, as the function checks if the
hardware supports eSPIs by reading the GICD_TYPER.ESPI field (using
GICD_TYPER_ESPIS_NUM macro), which indicates whether the extended SPI
range is supported. If the hardware does not support eSPI, the function
will not perform any actions.

There are no functional changes for setups where CONFIG_GICV3_ESPI=y.

Suggested-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@epam.com>
Acked-by: Julien Grall <jgrall@amazon.com>
---
Changes in v2:
- rebased on the current staging
- placed Suggested-by tag first to keep tags in chronological order
- added Acked-by from Julien Grall

This is a follow-up patch related to the discussion:
https://lore.kernel.org/xen-devel/820704d0-4047-4f02-a058-01daba2765f1@xen.org/

Sending v2 with the requested changes, as I only now noticed
that this patch has not been merged yet.
---
 xen/arch/arm/gic-v3.c                  | 32 ++++++++++++++------------
 xen/arch/arm/include/asm/gic_v3_defs.h |  2 --
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index acdac22953..463769d77b 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -703,17 +703,32 @@ unsigned int gic_number_espis(void)
     return gic_hw_ops->info->nr_espi;
 }
 
+static void __init gicv3_dist_espi_init_aff(uint64_t affinity)
+{
+    unsigned int i;
+
+    for ( i = 0; i < gicv3_info.nr_espi; i++ )
+        writeq_relaxed_non_atomic(affinity, GICD + GICD_IROUTERnE + i * 8);
+}
+#else
+
+static void __init gicv3_dist_espi_init_aff(uint64_t affinity) { }
+#endif
+
 static void __init gicv3_dist_espi_common_init(uint32_t type)
 {
     unsigned int espi_nr, i;
 
     espi_nr = min(1024U, GICD_TYPER_ESPIS_NUM(type));
+#ifdef CONFIG_GICV3_ESPI
     gicv3_info.nr_espi = espi_nr;
+#endif
     /* The GIC HW doesn't support eSPI, so we can leave from here */
-    if ( gicv3_info.nr_espi == 0 )
+    if ( espi_nr == 0 )
         return;
 
-    printk("GICv3: %u eSPI lines\n", gicv3_info.nr_espi);
+    if ( IS_ENABLED(CONFIG_GICV3_ESPI) )
+        printk("GICv3: %u eSPI lines\n", espi_nr);
 
     /* The configuration for eSPIs is similar to that for regular SPIs */
     for ( i = 0; i < espi_nr; i += 16 )
@@ -733,19 +748,6 @@ static void __init gicv3_dist_espi_common_init(uint32_t type)
         writel_relaxed(GENMASK(31, 0), GICD + GICD_IGROUPRnE + (i / 32) * 4);
 }
 
-static void __init gicv3_dist_espi_init_aff(uint64_t affinity)
-{
-    unsigned int i;
-
-    for ( i = 0; i < gicv3_info.nr_espi; i++ )
-        writeq_relaxed_non_atomic(affinity, GICD + GICD_IROUTERnE + i * 8);
-}
-#else
-static void __init gicv3_dist_espi_common_init(uint32_t type) { }
-
-static void __init gicv3_dist_espi_init_aff(uint64_t affinity) { }
-#endif
-
 static void __init gicv3_dist_init(void)
 {
     uint32_t type;
diff --git a/xen/arch/arm/include/asm/gic_v3_defs.h b/xen/arch/arm/include/asm/gic_v3_defs.h
index 3714cfeb7d..2081b2fe8f 100644
--- a/xen/arch/arm/include/asm/gic_v3_defs.h
+++ b/xen/arch/arm/include/asm/gic_v3_defs.h
@@ -63,7 +63,6 @@
 #define GICD_IROUTERnE               (0x8000)
 #define GICD_IROUTERnEN              (0x9FFC)
 
-#ifdef CONFIG_GICV3_ESPI
 #define GICD_TYPER_ESPI_SHIFT        8
 #define GICD_TYPER_ESPI_RANGE_SHIFT  27
 #define GICD_TYPER_ESPI_RANGE_MASK   (0x1F)
@@ -73,7 +72,6 @@
 #define GICD_TYPER_ESPIS_NUM(typer)    \
         (((typer) & GICD_TYPER_ESPI) ? \
         GICD_TYPER_ESPI_RANGE((typer) >> GICD_TYPER_ESPI_RANGE_SHIFT) : 0)
-#endif
 
 /* Common between GICD_PIDR2 and GICR_PIDR2 */
 #define GIC_PIDR2_ARCH_MASK         (0xf0)
-- 
2.34.1
Re: [PATCH v2] xen/arm: gicv3: initialize eSPI unconditionally
Posted by Mykola Kvach 1 day, 10 hours ago
Hi Leonid,

Thank you for the patch.

On Tue, Sep 22, 2026 at 9:59 PM Leonid Komarianskyi
<Leonid_Komarianskyi@epam.com> wrote:
>
> Since the firmware may initialize eSPIs before Xen, and without
> CONFIG_GICV3_ESPI enabled, Xen would not reinitialize them properly
> during boot. In such cases, once the GIC is re-enabled in Xen,
> interrupts may be received that cannot be handled.
>
> To ensure proper operation on hardware with eSPI feature, even when the eSPI
> config is disabled, gicv3_dist_espi_common_init() should be invoked
> regardless of whether CONFIG_GICV3_ESPI is enabled or not. This will not
> affect hardware without eSPI support, as the function checks if the
> hardware supports eSPIs by reading the GICD_TYPER.ESPI field (using
> GICD_TYPER_ESPIS_NUM macro), which indicates whether the extended SPI
> range is supported. If the hardware does not support eSPI, the function
> will not perform any actions.
>
> There are no functional changes for setups where CONFIG_GICV3_ESPI=y.
>
> Suggested-by: Julien Grall <jgrall@amazon.com>
> Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@epam.com>
> Acked-by: Julien Grall <jgrall@amazon.com>
> ---
> Changes in v2:
> - rebased on the current staging
> - placed Suggested-by tag first to keep tags in chronological order
> - added Acked-by from Julien Grall
>
> This is a follow-up patch related to the discussion:
> https://lore.kernel.org/xen-devel/820704d0-4047-4f02-a058-01daba2765f1@xen.org/
>
> Sending v2 with the requested changes, as I only now noticed
> that this patch has not been merged yet.
> ---
>  xen/arch/arm/gic-v3.c                  | 32 ++++++++++++++------------
>  xen/arch/arm/include/asm/gic_v3_defs.h |  2 --
>  2 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index acdac22953..463769d77b 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -703,17 +703,32 @@ unsigned int gic_number_espis(void)
>      return gic_hw_ops->info->nr_espi;
>  }
>
> +static void __init gicv3_dist_espi_init_aff(uint64_t affinity)
> +{
> +    unsigned int i;
> +
> +    for ( i = 0; i < gicv3_info.nr_espi; i++ )
> +        writeq_relaxed_non_atomic(affinity, GICD + GICD_IROUTERnE + i * 8);
> +}
> +#else
> +
> +static void __init gicv3_dist_espi_init_aff(uint64_t affinity) { }
> +#endif
> +
>  static void __init gicv3_dist_espi_common_init(uint32_t type)

I think the ordering in gicv3_dist_espi_common_init() needs to be
revisited now that this function is also called with
CONFIG_GICV3_ESPI=n.

The motivation for this patch is that firmware may have left an eSPI
enabled. However, we currently program GICD_ICFGRnE before clearing
the corresponding enable bit in GICD_ICENABLERnE.

The GIC architecture requires an interrupt to be individually disabled
before changing Int_config; otherwise the behavior is UNPREDICTABLE.
See Arm IHI 0069H.b, section 12.9.9 (GICD_ICFGR<n>).

We also rely on the same requirement in gic_set_irq_type().

So shouldn't we disable/deactivate all eSPIs before programming
GICD_ICFGRnE? Linux also initializes the extended SPI range in this
order: ICENABLERnE/ICACTIVERnE first, followed by IGROUPRnE,
ICFGRnE and IPRIORITYRnE.

This issue already seems to exist for the CONFIG_GICV3_ESPI=y path,
but this patch makes it relevant to the newly added CONFIG=n path,
where an eSPI left enabled by firmware is precisely the case we are
trying to handle.

Also, we could disable/deactivate eSPIs for all builds, while keeping
the rest of the eSPI configuration under CONFIG_GICV3_ESPI. This would
avoid accessing the other eSPI registers in builds without eSPI
support, unless there is a particular reason to initialize them there.

Best regards,
Mykola
Re: [PATCH v2] xen/arm: gicv3: initialize eSPI unconditionally
Posted by Leonid Komarianskyi 1 day ago
Hello Mykola,

Thank you for your review.

On 9/23/26 11:04, Mykola Kvach wrote:
> Hi Leonid,
>
> Thank you for the patch.
>
> On Tue, Sep 22, 2026 at 9:59 PM Leonid Komarianskyi
> <Leonid_Komarianskyi@epam.com> wrote:
>>
>> Since the firmware may initialize eSPIs before Xen, and without
>> CONFIG_GICV3_ESPI enabled, Xen would not reinitialize them properly
>> during boot. In such cases, once the GIC is re-enabled in Xen,
>> interrupts may be received that cannot be handled.
>>
>> To ensure proper operation on hardware with eSPI feature, even when the eSPI
>> config is disabled, gicv3_dist_espi_common_init() should be invoked
>> regardless of whether CONFIG_GICV3_ESPI is enabled or not. This will not
>> affect hardware without eSPI support, as the function checks if the
>> hardware supports eSPIs by reading the GICD_TYPER.ESPI field (using
>> GICD_TYPER_ESPIS_NUM macro), which indicates whether the extended SPI
>> range is supported. If the hardware does not support eSPI, the function
>> will not perform any actions.
>>
>> There are no functional changes for setups where CONFIG_GICV3_ESPI=y.
>>
>> Suggested-by: Julien Grall <jgrall@amazon.com>
>> Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@epam.com>
>> Acked-by: Julien Grall <jgrall@amazon.com>
>> ---
>> Changes in v2:
>> - rebased on the current staging
>> - placed Suggested-by tag first to keep tags in chronological order
>> - added Acked-by from Julien Grall
>>
>> This is a follow-up patch related to the discussion:
>> https://lore.kernel.org/xen-devel/820704d0-4047-4f02-a058-01daba2765f1@xen.org/
>>
>> Sending v2 with the requested changes, as I only now noticed
>> that this patch has not been merged yet.
>> ---
>>   xen/arch/arm/gic-v3.c                  | 32 ++++++++++++++------------
>>   xen/arch/arm/include/asm/gic_v3_defs.h |  2 --
>>   2 files changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index acdac22953..463769d77b 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -703,17 +703,32 @@ unsigned int gic_number_espis(void)
>>       return gic_hw_ops->info->nr_espi;
>>   }
>>
>> +static void __init gicv3_dist_espi_init_aff(uint64_t affinity)
>> +{
>> +    unsigned int i;
>> +
>> +    for ( i = 0; i < gicv3_info.nr_espi; i++ )
>> +        writeq_relaxed_non_atomic(affinity, GICD + GICD_IROUTERnE + i * 8);
>> +}
>> +#else
>> +
>> +static void __init gicv3_dist_espi_init_aff(uint64_t affinity) { }
>> +#endif
>> +
>>   static void __init gicv3_dist_espi_common_init(uint32_t type)
>
> I think the ordering in gicv3_dist_espi_common_init() needs to be
> revisited now that this function is also called with
> CONFIG_GICV3_ESPI=n.
>
> The motivation for this patch is that firmware may have left an eSPI
> enabled. However, we currently program GICD_ICFGRnE before clearing
> the corresponding enable bit in GICD_ICENABLERnE.
>
> The GIC architecture requires an interrupt to be individually disabled
> before changing Int_config; otherwise the behavior is UNPREDICTABLE.
> See Arm IHI 0069H.b, section 12.9.9 (GICD_ICFGR<n>).
>

Section 12.9.9 describes GICD_ICFGR<n>, i.e. the regular SPI range, and
it indeed contains this requirement. However, the code in
gicv3_dist_espi_common_init() programs GICD_ICFGR<n>E, which is
described in 12.9.10, and I could not find an equivalent requirement there.

The only related rule I found that also covers the extended SPI range is
in Arm IHI 0069H.b section 4.5:

"Changing the configuration of an interrupt from level-sensitive to
edge-triggered, or from edge-triggered to level-sensitive, when there is
a pending interrupt, leaves the interrupt in an UNKNOWN state."

That rule is about the pending state rather than the enable state, though.

Also, the current eSPI initialization sequence mirrors the one used for
regular SPIs in gicv3_dist_init(), where the requirement from 12.9.9
does apply. So if we decide to reorder the initialization, I think it
should be done for regular SPIs as well, ideally in a separate
preparatory patch.

> We also rely on the same requirement in gic_set_irq_type().
>
> So shouldn't we disable/deactivate all eSPIs before programming
> GICD_ICFGRnE? Linux also initializes the extended SPI range in this
> order: ICENABLERnE/ICACTIVERnE first, followed by IGROUPRnE,
> ICFGRnE and IPRIORITYRnE.
>
> This issue already seems to exist for the CONFIG_GICV3_ESPI=y path,
> but this patch makes it relevant to the newly added CONFIG=n path,
> where an eSPI left enabled by firmware is precisely the case we are
> trying to handle.
>
> Also, we could disable/deactivate eSPIs for all builds, while keeping
> the rest of the eSPI configuration under CONFIG_GICV3_ESPI. This would
> avoid accessing the other eSPI registers in builds without eSPI
> support, unless there is a particular reason to initialize them there.
>

This is a fair point, but I think it is better to clarify with the Arm
maintainers first whether the SPI/eSPI initialization order should be
changed in a separate preparatory patch, as the code for this patch
depends on the answer. As mentioned above, I could not find such a
restriction for eSPIs; it applies only to regular SPIs. If the
maintainers agree to change the sequence for both SPIs and eSPIs, I can
prepare a separate patch and update this patch accordingly.


Best regards,
Leonid.