Re: [PATCH] xen/arm: irq: Use appropriate priority for SGIs in setup_irq()

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Mykola,

> On 14 Jan 2026, at 07:00, Mykola Kvach <xakep.amatop@xxxxxxxxx> wrote:
> 
> Hi Bertrand, Julien,
> 
> First of all, please accept my apologies for the delayed response.
> 
> On Wed, Dec 3, 2025 at 10:10 AM Bertrand Marquis
> <Bertrand.Marquis@xxxxxxx> wrote:
>> 
>> Hi Julien/Mykola,
>> 
>>> On 2 Dec 2025, at 19:26, Julien Grall <julien@xxxxxxx> wrote:
>>> 
>>> Hi,
>>> 
>>> Sorry for the late answer.
>>> 
>>> On 16/09/2025 11:19, Mykola Kvach wrote:
>>>> On Sat, Sep 13, 2025 at 1:01 AM Julien Grall <julien@xxxxxxx> wrote:
>>>>>> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
>>>>>> ---
>>>>>>  xen/arch/arm/irq.c | 8 +++++++-
>>>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>> 
>>>>>> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
>>>>>> index 02ca82c089..17c7ac92b5 100644
>>>>>> --- a/xen/arch/arm/irq.c
>>>>>> +++ b/xen/arch/arm/irq.c
>>>>>> @@ -397,7 +397,13 @@ int setup_irq(unsigned int irq, unsigned int 
>>>>>> irqflags, struct irqaction *new)
>>>>> AFAIK, we are not using setup_irq() to handle SGIs because they are all
>>>>> static and always enabled. Are you planning to handle dynamic SGIs? If
>>>>> yes, then can you provide more details?As far as I know, there can be at 
>>>>> least one “dynamic” SGI in Xen.
>>>> As far as I know, there is at least one “dynamic” SGI in Xen. For
>>>> example, see ffa_notif.c in the functions ffa_notif_init_interrupt
>>>> and ffa_notif_init, which handle initialization of such SGIs.
>>> 
>>> Bertrand can you comment on this? In particular, do we want the FFA SGIs to 
>>> have the priority of the internal ones?
>> 
>> The following is only an advice, definitely not a requirement. I would
>> be ok to ack the current way to do things as right now FF-A is unsupported 
>> and
>> is the only case of usage of dynamic SGI.
>> I would though require to have a log message to warn the user that SGI xx
>> has the same priority as xen internal interrupts during request_irq.
>> 
>> Here is what I think:
>> 
>> FFA SGIs can only be generated by the secure world and in practice they will
>> be generated mostly when coming coming back from the secure world (either
>> after a preemption or on a return to an smc call) but one could also be
>> generated from the secure world from another core, preempting whatever runs
>> (but same would occur when an interrupt is directly handled in the secure 
>> world).
>> 
>> Linux kernel implementation is not lowering the FF-A SGI interrupt as far as 
>> I know.
>> 
>> In my view having the FFA SGI having the same priority as ffa internal SGI 
>> would mean
>> we have some trust that the secure world will not overload us.
>> 
>> But in reality it would make sense to have a priority ordering like:
>> - Xen internal SGIs
>> - FF-A SGI (or any other dynamic SGI)
>> - any other kind of interrupt
>> 
>> So that Xen internal SGIs have the highest priority, but having other SGIs 
>> still having
>> a better priority than other interrupts.
>> 
>> In any case, whatever we do, we should keep it possible to have one specific 
>> dynamic
>> SGI at the maximum level or even at an higher level (ie lower down xen 
>> internal SGIs)
>> for specific use cases (handling hardware errors comes to mind) but this is 
>> ok to make
>> this possible only by changing xen code or when creating such a specific 
>> driver.
> 
> Thank you for the detailed feedback regarding the priority handling for
> dynamic SGIs. Based on Bertrand's suggestions, I would like to propose
> a more structured approach to interrupt priorities.
> 
> To avoid having dynamic SGIs share the exact same priority as Xen's
> internal IPIs, while still ensuring they can preempt normal interrupts,
> I propose the following hierarchy:
> 
> #define GIC_PRI_LOWEST 0xf0U
> #define GIC_PRI_IRQ 0xb0U
> #define GIC_PRI_DYNAMIC_SGI 0xa0U
> #define GIC_PRI_IPI 0x90U /* IPIs must preempt normal interrupts */
> #define GIC_PRI_HIGHEST 0x80U /* Higher priorities belong to Secure-World */
> 
> 
> Key changes:
> 1. Shift GIC_PRI_IRQ to 0xb0U: This moves standard interrupts one level
> down.
> 2. Introduce GIC_PRI_DYNAMIC_SGI at 0xa0U: This creates a dedicated
> priority level for dynamic SGIs (like FF-A).
> 
> This structure ensures that:
> - Internal Xen IPIs (0x90) remain the highest priority for the
> hypervisor.
> - Dynamic SGIs (0xa0) can preempt normal interrupts but cannot
> interfere with internal Xen signaling.
> - We stay within the safe range for Xen (starting from 0x80).
> 
> Does this approach look acceptable to you? In particular, do you see any
> concerns with shifting the default GIC_PRI_IRQ from 0xa0 to 0xb0 on ARM?

This sounds reasonable but would definitely need some extensive testing to 
validate
that we do not break existing systems based on the current scheme.

Maybe we should have command line parameters so that a system somehow dependent
on the old priority scheme could have a way to configure the priorities without 
modifying
xen code (gic_pri_irq=, gic_pri_sgi=, gic_pri_ipi= with values 0 to 8 to make 
things easier),
this would give a way to be backward compatible easily.

Regarding FF-A interrup priorities, I am not completely sure if that should or 
not have higher
priority than normal IRQs but that i could make configurable through a command 
line parameter.

> 
> If this looks good to you, I will send a v2 with these changes.

Works for me

Cheers
Bertrand