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

[Leonid Komarianskyi <Leonid_Komarianskyi@xxxxxxxx>]

Hi Julien,

Thank you for your comment.

On 05.09.25 10:10, Julien Grall wrote:
> Hi Leonid,
> 
> On 04/09/2025 21:01, Leonid Komarianskyi wrote:
>> diff --git a/xen/arch/arm/include/asm/irq.h b/xen/arch/arm/include/ 
>> asm/irq.h
>> index 5bc6475eb4..2ff2d07d6d 100644
>> --- a/xen/arch/arm/include/asm/irq.h
>> +++ b/xen/arch/arm/include/asm/irq.h
>> @@ -32,6 +32,10 @@ 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
>> +
>>   /* LPIs are always numbered starting at 8192, so 0 is a good invalid 
>> case. */
>>   #define INVALID_LPI     0
>> @@ -39,7 +43,12 @@ struct arch_irq_desc {
>>   #define INVALID_IRQ     1023
>>   extern const unsigned int nr_irqs;
>> +#ifdef CONFIG_GICV3_ESPI
>> +/* This will cover the eSPI range, to allow asignmant of eSPIs to 
>> domains. */
> 
> Typo: s/asignmant/assignment/
> 
> [...]
> 
>> Unless INTIDs from the eSPI
>> + * range are mistakenly defined in Xen DTS when the appropriate 
>> config is
>> + * disabled, this function will not be reached because is_espi will 
>> return
>> + * false for non-eSPI INTIDs.
> 
> I am still confused with this paragraph. How is this function can be 
> reached if it is compiled out? Surely, if the DT is misconfigured, we 
> should get an error when trying to route the interrupt. No? If so, can 
> you point me to that code?
> 
> Cheers,
> 

Oh, sorry, the second part of the comment is redundant with the current 
implementation. It was correct when the function had an implementation 
and returned NULL. The correct comment is:

Defined as a prototype as it should not be called if 
CONFIG_GICV3_ESPI=n. Without CONFIG_GICV3_ESPI, the additional 1024 IRQ 
descriptors will not be defined, and thus, they cannot be used.

Should I prepare V8 with the comment fix, or can this be corrected on 
commit?

Best regards,
Leonid