[PATCH v7 05/12] xen/arm: gicv3: implement handling of GICv3.1 eSPI

Leonid Komarianskyi posted 12 patches 1 week, 2 days ago
There is a newer version of this series
[PATCH v7 05/12] xen/arm: gicv3: implement handling of GICv3.1 eSPI
Posted by Leonid Komarianskyi 1 week, 2 days ago
Introduced appropriate register definitions, helper macros,
and initialization of required GICv3.1 distributor registers
to support eSPI. This type of interrupt is handled in the
same way as regular SPI interrupts, with the following
differences:

1) eSPIs can have up to 1024 interrupts, starting from the
beginning of the range, whereas regular SPIs use INTIDs from
32 to 1019, totaling 988 interrupts;
2) eSPIs start at INTID 4096, necessitating additional interrupt
index conversion during register operations.

In case if appropriate config is disabled, or GIC HW doesn't
support eSPI, the existing functionality will remain the same.

Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@epam.com>
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

---
Changes in V7:
- no changes

Changes in V6:
- removed unnecessary parentheses in gic_is_valid_espi()
- updated gic_is_valid_line(): it now verifies the condition irq <
  gic_number_lines() first, as it is more likely that the irq number
  will be from the non-eSPI range
- minor change: changed the macros ESPI_INTID2IDX and ESPI_IDX2INTID
  into appropriate inline functions introduced in the previous patch
- added reviewed-by from Oleksandr Tyshchenko

Changes in V5:
- fixed minor nits, no functional changes: changed u32 to uint32_t and
  added a comment noting that the configuration for eSPIs is the same as
  for regular SPIs
- removed ifdefs for eSPI-specific offsets to reduce the number of
  ifdefs and code duplication in further changes
- removed reviewed-by as moving offset from ifdefs requires additional
  confirmation from reviewers

Changes in V4:
- added offsets for GICD_IGRPMODRnE and GICD_NSACRnE that are required
  for vGIC emulation
- added a log banner with eSPI information, similar to the one for
  regular SPI
- added newline after ifdef and before gic_is_valid_line
- added reviewed-by from Volodymyr Babchuk

Changes in V3:
- add __init attribute to gicv3_dist_espi_common_init
- change open-codded eSPI register initialization to the appropriate
  gen-mask macro
- fixed formatting for lines with more than 80 symbols
- introduced gicv3_dist_espi_init_aff to be able to use stubs in case of
  CONFIG_GICV3_ESPI disabled
- renamed parameter in the GICD_TYPER_ESPI_RANGE macro to espi_range
  (name was taken from GIC specification) to avoid confusion
- changed type for i variable to unsigned int since it cannot be
  negative

Changes in V2:
- move gic_number_espis function from
  [PATCH 08/10] xen/arm: vgic: add resource management for extended SPIs
  to use it in the newly introduced gic_is_valid_espi
- add gic_is_valid_espi which checks if IRQ number is in supported
  by HW eSPI range
- update gic_is_valid_irq conditions to allow operations with eSPIs
---
 xen/arch/arm/gic-v3.c                  | 83 ++++++++++++++++++++++++++
 xen/arch/arm/include/asm/gic.h         | 21 ++++++-
 xen/arch/arm/include/asm/gic_v3_defs.h | 38 ++++++++++++
 3 files changed, 141 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index a1e302fea2..a69263e461 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -485,6 +485,36 @@ static void __iomem *get_addr_by_offset(struct irq_desc *irqd, uint32_t offset)
         default:
             break;
         }
+#ifdef CONFIG_GICV3_ESPI
+    case ESPI_BASE_INTID ... ESPI_MAX_INTID:
+    {
+        uint32_t irq_index = espi_intid_to_idx(irqd->irq);
+
+        switch ( offset )
+        {
+        case GICD_ISENABLER:
+            return (GICD + GICD_ISENABLERnE + (irq_index / 32) * 4);
+        case GICD_ICENABLER:
+            return (GICD + GICD_ICENABLERnE + (irq_index / 32) * 4);
+        case GICD_ISPENDR:
+            return (GICD + GICD_ISPENDRnE + (irq_index / 32) * 4);
+        case GICD_ICPENDR:
+            return (GICD + GICD_ICPENDRnE + (irq_index / 32) * 4);
+        case GICD_ISACTIVER:
+            return (GICD + GICD_ISACTIVERnE + (irq_index / 32) * 4);
+        case GICD_ICACTIVER:
+            return (GICD + GICD_ICACTIVERnE + (irq_index / 32) * 4);
+        case GICD_ICFGR:
+            return (GICD + GICD_ICFGRnE + (irq_index / 16) * 4);
+        case GICD_IROUTER:
+            return (GICD + GICD_IROUTERnE + irq_index * 8);
+        case GICD_IPRIORITYR:
+            return (GICD + GICD_IPRIORITYRnE + irq_index);
+        default:
+            break;
+        }
+    }
+#endif
     default:
         break;
     }
@@ -655,6 +685,55 @@ static void gicv3_set_irq_priority(struct irq_desc *desc,
     spin_unlock(&gicv3.lock);
 }
 
+#ifdef CONFIG_GICV3_ESPI
+unsigned int gic_number_espis(void)
+{
+    return gic_hw_ops->info->nr_espi;
+}
+
+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));
+    gicv3_info.nr_espi = espi_nr;
+    /* The GIC HW doesn't support eSPI, so we can leave from here */
+    if ( gicv3_info.nr_espi == 0 )
+        return;
+
+    printk("GICv3: %d eSPI lines\n", gicv3_info.nr_espi);
+
+    /* The configuration for eSPIs is similar to that for regular SPIs */
+    for ( i = 0; i < espi_nr; i += 16 )
+        writel_relaxed(0, GICD + GICD_ICFGRnE + (i / 16) * 4);
+
+    for ( i = 0; i < espi_nr; i += 4 )
+        writel_relaxed(GIC_PRI_IRQ_ALL,
+                       GICD + GICD_IPRIORITYRnE + (i / 4) * 4);
+
+    for ( i = 0; i < espi_nr; i += 32 )
+    {
+        writel_relaxed(GENMASK(31, 0), GICD + GICD_ICENABLERnE + (i / 32) * 4);
+        writel_relaxed(GENMASK(31, 0), GICD + GICD_ICACTIVERnE + (i / 32) * 4);
+    }
+
+    for ( i = 0; i < espi_nr; i += 32 )
+        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;
@@ -700,6 +779,8 @@ static void __init gicv3_dist_init(void)
     for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 32 )
         writel_relaxed(GENMASK(31, 0), GICD + GICD_IGROUPR + (i / 32) * 4);
 
+    gicv3_dist_espi_common_init(type);
+
     gicv3_dist_wait_for_rwp();
 
     /* Turn on the distributor */
@@ -713,6 +794,8 @@ static void __init gicv3_dist_init(void)
 
     for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i++ )
         writeq_relaxed_non_atomic(affinity, GICD + GICD_IROUTER + i * 8);
+
+    gicv3_dist_espi_init_aff(affinity);
 }
 
 static int gicv3_enable_redist(void)
diff --git a/xen/arch/arm/include/asm/gic.h b/xen/arch/arm/include/asm/gic.h
index 3fcee42675..3947c8634d 100644
--- a/xen/arch/arm/include/asm/gic.h
+++ b/xen/arch/arm/include/asm/gic.h
@@ -306,9 +306,24 @@ extern void gic_dump_vgic_info(struct vcpu *v);
 
 /* Number of interrupt lines */
 extern unsigned int gic_number_lines(void);
+#ifdef CONFIG_GICV3_ESPI
+extern unsigned int gic_number_espis(void);
+
+static inline bool gic_is_valid_espi(unsigned int irq)
+{
+    return irq >= ESPI_BASE_INTID &&
+           irq < espi_idx_to_intid(gic_number_espis());
+}
+#else
+static inline bool gic_is_valid_espi(unsigned int irq)
+{
+    return false;
+}
+#endif
+
 static inline bool gic_is_valid_line(unsigned int irq)
 {
-    return irq < gic_number_lines();
+    return irq < gic_number_lines() || gic_is_valid_espi(irq);
 }
 
 static inline bool gic_is_spi(unsigned int irq)
@@ -325,6 +340,10 @@ struct gic_info {
     enum gic_version hw_version;
     /* Number of GIC lines supported */
     unsigned int nr_lines;
+#ifdef CONFIG_GICV3_ESPI
+    /* Number of GIC eSPI supported */
+    unsigned int nr_espi;
+#endif
     /* Number of LR registers */
     uint8_t nr_lrs;
     /* Maintenance irq number */
diff --git a/xen/arch/arm/include/asm/gic_v3_defs.h b/xen/arch/arm/include/asm/gic_v3_defs.h
index 2af093e774..3370b4cd52 100644
--- a/xen/arch/arm/include/asm/gic_v3_defs.h
+++ b/xen/arch/arm/include/asm/gic_v3_defs.h
@@ -37,6 +37,44 @@
 #define GICD_IROUTER1019             (0x7FD8)
 #define GICD_PIDR2                   (0xFFE8)
 
+/* Additional registers for GICv3.1 */
+#define GICD_IGROUPRnE               (0x1000)
+#define GICD_IGROUPRnEN              (0x107C)
+#define GICD_ISENABLERnE             (0x1200)
+#define GICD_ISENABLERnEN            (0x127C)
+#define GICD_ICENABLERnE             (0x1400)
+#define GICD_ICENABLERnEN            (0x147C)
+#define GICD_ISPENDRnE               (0x1600)
+#define GICD_ISPENDRnEN              (0x167C)
+#define GICD_ICPENDRnE               (0x1800)
+#define GICD_ICPENDRnEN              (0x187C)
+#define GICD_ISACTIVERnE             (0x1A00)
+#define GICD_ISACTIVERnEN            (0x1A7C)
+#define GICD_ICACTIVERnE             (0x1C00)
+#define GICD_ICACTIVERnEN            (0x1C7C)
+#define GICD_IPRIORITYRnE            (0x2000)
+#define GICD_IPRIORITYRnEN           (0x23FC)
+#define GICD_ICFGRnE                 (0x3000)
+#define GICD_ICFGRnEN                (0x30FC)
+#define GICD_IGRPMODRnE              (0x3400)
+#define GICD_IGRPMODRnEN             (0x347C)
+#define GICD_NSACRnE                 (0x3600)
+#define GICD_NSACRnEN                (0x36FC)
+#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)
+#define GICD_TYPER_ESPI              (1U << GICD_TYPER_ESPI_SHIFT)
+#define GICD_TYPER_ESPI_RANGE(espi_range) ((((espi_range) & \
+        GICD_TYPER_ESPI_RANGE_MASK) + 1) * 32)
+#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)
 #define GIC_PIDR2_ARCH_GICv3        (0x30)
-- 
2.34.1
Re: [PATCH v7 05/12] xen/arm: gicv3: implement handling of GICv3.1 eSPI
Posted by Julien Grall 1 week, 1 day ago
Hi Leonid,

On 04/09/2025 21:01, Leonid Komarianskyi wrote:
> +#ifdef CONFIG_GICV3_ESPI
> +unsigned int gic_number_espis(void)
> +{
> +    return gic_hw_ops->info->nr_espi;
> +}
> +
> +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));
> +    gicv3_info.nr_espi = espi_nr;
> +    /* The GIC HW doesn't support eSPI, so we can leave from here */
> +    if ( gicv3_info.nr_espi == 0 )
> +        return;
> +
> +    printk("GICv3: %d eSPI lines\n", gicv3_info.nr_espi);

Style: nr_espi is unsigned. So it should be %u. Can be fixed on commit 
if there is nothing else major to change.

> +
> +    /* The configuration for eSPIs is similar to that for regular SPIs */
> +    for ( i = 0; i < espi_nr; i += 16 )
> +        writel_relaxed(0, GICD + GICD_ICFGRnE + (i / 16) * 4);
> +
> +    for ( i = 0; i < espi_nr; i += 4 )
> +        writel_relaxed(GIC_PRI_IRQ_ALL,
> +                       GICD + GICD_IPRIORITYRnE + (i / 4) * 4);
> +
> +    for ( i = 0; i < espi_nr; i += 32 )
> +    {
> +        writel_relaxed(GENMASK(31, 0), GICD + GICD_ICENABLERnE + (i / 32) * 4);

Sorry I only spotted now.

The goal of gicv3_dist_espi_common_init() is to make sure the GIC is in 
a sane state for Xen (the previous OS or firmware may have left some 
state). Now the firmware may decide to use eSPI but not Xen (e.g. 
because CONFIG_ESPI=n).

This would means we may have eSPI interrupts that may be enabled and 
pending. So as soon as we re-enable the GIC we may receive interrupts we 
can't handle. So I think we may need to initialize the eSPI part of the 
distributor unconditionally. What do you think?


This could be handled as a follow-up but within the timeframe of Xen 
4.21. So for this patch:

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall
Re: [PATCH v7 05/12] xen/arm: gicv3: implement handling of GICv3.1 eSPI
Posted by Leonid Komarianskyi 1 week, 1 day ago
Hi Julien,

Thank you for your comments and AB.

On 05.09.25 10:22, Julien Grall wrote:
> Hi Leonid,
> 
> On 04/09/2025 21:01, Leonid Komarianskyi wrote:
>> +#ifdef CONFIG_GICV3_ESPI
>> +unsigned int gic_number_espis(void)
>> +{
>> +    return gic_hw_ops->info->nr_espi;
>> +}
>> +
>> +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));
>> +    gicv3_info.nr_espi = espi_nr;
>> +    /* The GIC HW doesn't support eSPI, so we can leave from here */
>> +    if ( gicv3_info.nr_espi == 0 )
>> +        return;
>> +
>> +    printk("GICv3: %d eSPI lines\n", gicv3_info.nr_espi);
> 
> Style: nr_espi is unsigned. So it should be %u. Can be fixed on commit 
> if there is nothing else major to change.
> 
>> +
>> +    /* The configuration for eSPIs is similar to that for regular 
>> SPIs */
>> +    for ( i = 0; i < espi_nr; i += 16 )
>> +        writel_relaxed(0, GICD + GICD_ICFGRnE + (i / 16) * 4);
>> +
>> +    for ( i = 0; i < espi_nr; i += 4 )
>> +        writel_relaxed(GIC_PRI_IRQ_ALL,
>> +                       GICD + GICD_IPRIORITYRnE + (i / 4) * 4);
>> +
>> +    for ( i = 0; i < espi_nr; i += 32 )
>> +    {
>> +        writel_relaxed(GENMASK(31, 0), GICD + GICD_ICENABLERnE + (i / 
>> 32) * 4);
> 
> Sorry I only spotted now.
> 
> The goal of gicv3_dist_espi_common_init() is to make sure the GIC is in 
> a sane state for Xen (the previous OS or firmware may have left some 
> state). Now the firmware may decide to use eSPI but not Xen (e.g. 
> because CONFIG_ESPI=n).
> 
> This would means we may have eSPI interrupts that may be enabled and 
> pending. So as soon as we re-enable the GIC we may receive interrupts we 
> can't handle. So I think we may need to initialize the eSPI part of the 
> distributor unconditionally. What do you think?
> 
> 
> This could be handled as a follow-up but within the timeframe of Xen 
> 4.21. So for this patch:
> 
> Acked-by: Julien Grall <jgrall@amazon.com>
> 
> Cheers,
> 

Yes, that's a really good point about firmware initialization - I did 
not think about that. In that case, we just need to move the nr_espi 
field out of the ifdef, remove the stubs for 
gicv3_dist_espi_common_init() and gicv3_dist_espi_init_aff(), and remove 
the ifdef for these functions. The verifications at the beginning of 
gicv3_dist_espi_common_init() should work correctly, simply returning 0 
on platforms without eSPI.

I will prepare a follow-up patch for this.

Best regards,
Leonid
Re: [PATCH v7 05/12] xen/arm: gicv3: implement handling of GICv3.1 eSPI
Posted by Julien Grall 1 week, 1 day ago
Hi Leonid,

On 05/09/2025 11:27, Leonid Komarianskyi wrote:
> On 05.09.25 10:22, Julien Grall wrote:
>> Hi Leonid,
>>
>> On 04/09/2025 21:01, Leonid Komarianskyi wrote:
>>> +#ifdef CONFIG_GICV3_ESPI
>>> +unsigned int gic_number_espis(void)
>>> +{
>>> +    return gic_hw_ops->info->nr_espi;
>>> +}
>>> +
>>> +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));
>>> +    gicv3_info.nr_espi = espi_nr;
>>> +    /* The GIC HW doesn't support eSPI, so we can leave from here */
>>> +    if ( gicv3_info.nr_espi == 0 )
>>> +        return;
>>> +
>>> +    printk("GICv3: %d eSPI lines\n", gicv3_info.nr_espi);
>>
>> Style: nr_espi is unsigned. So it should be %u. Can be fixed on commit
>> if there is nothing else major to change.
>>
>>> +
>>> +    /* The configuration for eSPIs is similar to that for regular
>>> SPIs */
>>> +    for ( i = 0; i < espi_nr; i += 16 )
>>> +        writel_relaxed(0, GICD + GICD_ICFGRnE + (i / 16) * 4);
>>> +
>>> +    for ( i = 0; i < espi_nr; i += 4 )
>>> +        writel_relaxed(GIC_PRI_IRQ_ALL,
>>> +                       GICD + GICD_IPRIORITYRnE + (i / 4) * 4);
>>> +
>>> +    for ( i = 0; i < espi_nr; i += 32 )
>>> +    {
>>> +        writel_relaxed(GENMASK(31, 0), GICD + GICD_ICENABLERnE + (i /
>>> 32) * 4);
>>
>> Sorry I only spotted now.
>>
>> The goal of gicv3_dist_espi_common_init() is to make sure the GIC is in
>> a sane state for Xen (the previous OS or firmware may have left some
>> state). Now the firmware may decide to use eSPI but not Xen (e.g.
>> because CONFIG_ESPI=n).
>>
>> This would means we may have eSPI interrupts that may be enabled and
>> pending. So as soon as we re-enable the GIC we may receive interrupts we
>> can't handle. So I think we may need to initialize the eSPI part of the
>> distributor unconditionally. What do you think?
>>
>>
>> This could be handled as a follow-up but within the timeframe of Xen
>> 4.21. So for this patch:
>>
>> Acked-by: Julien Grall <jgrall@amazon.com>
>>
>> Cheers,
>>
> 
> Yes, that's a really good point about firmware initialization - I did
> not think about that. In that case, we just need to move the nr_espi
> field out of the ifdef, remove the stubs for
> gicv3_dist_espi_common_init() and gicv3_dist_espi_init_aff(), and remove
> the ifdef for these functions. The verifications at the beginning of
> gicv3_dist_espi_common_init() should work correctly, simply returning 0
> on platforms without eSPI.

I don't think we need all of these. We just need to ensure the 
interrupts are disabled and deactivated. So no need to reset the 
affinity or even moving the field moving the field out of the ifdef.

Cheers,

-- 
Julien Grall