[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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





On 27.08.25 21:24, Leonid Komarianskyi wrote:

Hello Leonid


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@xxxxxxxx>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>

---
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

only NITs below

Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>


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

Changes for V4:

Changes in V4:
- added offsets for GICD_IGRPMODRnE and GICD_NSACRnE that are required
   for vGIC emulation
- added newline after ifdef and before gic_is_valid_line
- added reviewed-by from Volodymyr Babchuk
- added a log banner with eSPI information, similar to the one for
   regular SPI
---
  xen/arch/arm/gic-v3.c                  | 82 ++++++++++++++++++++++++++
  xen/arch/arm/include/asm/gic.h         | 22 +++++++
  xen/arch/arm/include/asm/gic_v3_defs.h | 38 ++++++++++++
  3 files changed, 142 insertions(+)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index a959fefebe..b939a1f490 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, u32 offset)
          default:
              break;
          }
+#ifdef CONFIG_GICV3_ESPI
+    case ESPI_BASE_INTID ... ESPI_MAX_INTID:
+    {
+        u32 irq_index = ESPI_INTID2IDX(irqd->irq);

NIT: I have heard that no uN for new code, but uintN_t (sorry for didn't spot this before), so I would use uint32_t


+
+        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,54 @@ 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);
+
+    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);

NIT: From what I see, the eSPIs are configured exactly as regular SPIs in gicv3_dist_init() (i.e. the eSPIs are level-triggered, disabled and deactivated, belong to the same group, etc). In gicv3_dist_init() we have comments clarying the actions, but here we do not. I would at least write a sentence in patch description/in-code comment saying that eSPIs configuration is the same as for regular SPIs.


+}
+
+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 +778,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 +793,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)


[snip]




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.