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

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Julien Grall <julien@xxxxxxx> writes:

> On 21/08/2025 17:59, Volodymyr Babchuk wrote:
>> Julien Grall <julien@xxxxxxx> writes:
>> 
>>> 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.
>> Probably, we can expect more in the future...
>
> Well yes. But I was under the impression this the preferred
> approach. See the discussion about disabling 32-bit support on 64-bit:
>
> 20250723075835.3993182-1-grygorii_strashko@xxxxxxxx

Ah yes, safety certification. Welp, can't argue with that.

>
>>  Anyways, after reviewing
>> all patches in the series, I can see that code will be littered with
>> #ifdef CONFIG_GICV3_ESPI, which, probably, not a good thing.
>
> The solution is to provide wrappers to reduce the number of #ifdef. I
> haven't checked all the places.

Yes, I was also thinking about that, but I got an impression, that in
many cases it will be hard to provide such wrappers. Anyways, something
is needs to be done here.

[...]


>
>> As a bonus point, we can't leave this pointer
>> present even if CONFIG_GICV3_ESPI=n, which will simplify some code in
>> latter patches.
>
> Did you intend to say "We can leave" rather than "we can't leave"?

Correct

-- 
WBR, Volodymyr