Hi Julien,
On 29.07.25 16:59, Julien Grall wrote:
> Hi,
>
> On 24/07/2025 15:57, Leonid Komarianskyi wrote:
>> The do_IRQ() function is the main handler for processing IRQs.
>> Currently, due to restrictive checks, it does not process interrupt
>> numbers greater than 1024. This patch updates the condition to allow
>> the handling of interrupts from the eSPI range.
>>
>> Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@epam.com>
>> ---
>> xen/arch/arm/gic.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
>> index d5f2addf9f..b4a185fcc5 100644
>> --- a/xen/arch/arm/gic.c
>> +++ b/xen/arch/arm/gic.c
>> @@ -342,7 +342,7 @@ void gic_interrupt(struct cpu_user_regs *regs, int
>> is_fiq)
>> /* Reading IRQ will ACK it */
>> irq = gic_hw_ops->read_irq();
>> - if ( likely(irq >= GIC_SGI_STATIC_MAX && irq < 1020) )
>> + if ( likely(irq >= GIC_SGI_STATIC_MAX && irq < 1020) ||
>> is_espi(irq) )
>
> Looking at the series, we seem to have a common pattern which is "check
> if an SPI or an eSPI". AFAIU, pretty much everywhere we use an SPI, we
> want to be able to use an eSPI.
>
> So rather than open-coding everywhere, can we create a new helper to
> check whether we have an (e)SPI? This would make easier to read the code.
>
Oh, thank you for your comment - it helped me identify mistakes in the
previous patches :)
Basically, in some parts of the code like this, we only need to verify
whether the INTID is valid. However, in other cases (e.g., vGIC-specific
code), we also need to check whether the domain can utilize the IRQ or
if the HW GIC actually supports that INTID. For instance, the HW might
support only 512 eSPIs, and simply using the is_espi check would pass
incorrectly because it only verifies if the INTID falls in the eSPI
range but doesn't check if the hardware supports that INTID or if the
domain can use that IRQ.
I will revise the code based on these findings, introduce helper
functions, and address the potential issues in v2.
Cheers,
Leonid