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
