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

[Julien Grall <julien@xxxxxxx>]

Hi,

On 02/09/2025 09:56, Leonid Komarianskyi wrote:
> > > diff --git a/xen/arch/arm/include/asm/irq.h b/xen/arch/arm/include/
> > > asm/irq.h
> > > index 5bc6475eb4..4443799648 100644
> > > --- a/xen/arch/arm/include/asm/irq.h
> > > +++ b/xen/arch/arm/include/asm/irq.h
> > > @@ -32,6 +32,13 @@ 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
> > > +
> > > +#define ESPI_INTID2IDX(intid) ((intid) - ESPI_BASE_INTID)
> > > +#define ESPI_IDX2INTID(idx)   ((idx) + ESPI_BASE_INTID)
> > NIT: I would consider adding sanity check (i.e. ASSERT()) to confirm
> > that both ``intid`` and ``idx`` are within the bounds.
> Okay, I will add sanity check with ASSERTs in V6 (similar to
> GNTPIN_incr2oflow_mask):
>
> #define ESPI_INTID2IDX(intid)                           \
>       ({                                                  \
>           ASSERT(((intid) >= ESPI_BASE_INTID) &&          \
>                  ((intid) <= ESPI_MAX_INTID));            \
>           ((intid) - ESPI_BASE_INTID);                    \
>       })
If you are using a macro, then you will need to stash "intid" in a local 
variable. Otherwise, it would be evaluated multiple time.
The alternative is to use a static inline helper which is usually preferred.

Cheers,

--
Julien Grall