Re: [PATCH v2 04/10] xen/arm/irq: add handling for IRQs in the eSPI range

[Julien Grall <julien@xxxxxxx>]

Hi,

On 21/08/2025 16:59, Volodymyr Babchuk wrote:
> Leonid Komarianskyi <Leonid_Komarianskyi@xxxxxxxx> 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.
> I don't think that it is a good idea to hide this feature under Kconfig
> option, as this will increase number of different build variants.
> I believe that runtime check for GICD_TYPER.ESPI should be 
sufficient,> but maintainers can correct me there.
I haven't seen many board with ESPI available. So I think it would be 
better if this is under a Kconfig because not everyone may want to have 
the code.
[...]

> >   struct irq_desc;
> >   struct irqaction;
> > @@ -55,6 +71,15 @@ 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
> > +    return false;
> > +#endif
> > +}
> > +
> >   #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 50e57aaea7..9bc72fbbc9 100644
> > --- a/xen/arch/arm/irq.c
> > +++ b/xen/arch/arm/irq.c
> > @@ -19,7 +19,11 @@
> >   #include <asm/gic.h>
> >   #include <asm/vgic.h>
> >   
> > +#ifdef CONFIG_GICV3_ESPI
> > +const unsigned int nr_irqs = ESPI_MAX_INTID + 1;
> > +#else
> >   const unsigned int nr_irqs = NR_IRQS;
> > +#endif
> >   
> >   static unsigned int local_irqs_type[NR_LOCAL_IRQS];
> >   static DEFINE_SPINLOCK(local_irqs_type_lock);
> > @@ -46,6 +50,9 @@ void irq_end_none(struct irq_desc *irq)
> >   }
> >   
> >   static irq_desc_t irq_desc[NR_IRQS - NR_LOCAL_IRQS];
> > +#ifdef CONFIG_GICV3_ESPI
> > +static irq_desc_t espi_desc[NR_IRQS];
By how much will this increase the Xen binary?

> This is really confusing. Should it be something like espi_desc[NR_ESPI_IRQS]?
+1.

Cheers,

--
Julien Grall