1. Patchew
  2. Xen
  3. View series

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

Mykola Kvach posted 1 patch 5 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/f7475c0083bf4995f2ec4afa3aaf44b9676fd1ab.1756867760.git.mykola._5Fkvach@epam.com
xen/arch/arm/irq.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
[PATCH] xen/arm: irq: Use appropriate priority for SGIs in setup_irq()
Posted by Mykola Kvach 5 months, 1 week ago 
From: Mykola Kvach <mykola_kvach@epam.com>

Use GIC_PRI_IPI priority for SGI interrupts instead of the generic
GIC_PRI_IRQ priority in setup_irq().

This change ensures that SGIs get the correct priority level when
being set up for Xen's use, maintaining proper interrupt precedence
in the system.

The priority assignment now follows ARM GIC best practices:
- SGIs (0-15): GIC_PRI_IPI (higher priority)
- PPIs/SPIs (16+): GIC_PRI_IRQ (standard priority)

Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
 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)
     /* First time the IRQ is setup */
     if ( disabled )
     {
-        gic_route_irq_to_xen(desc, GIC_PRI_IRQ);
+        unsigned int prio = GIC_PRI_IRQ;
+
+        /* Use appropriate priority based on interrupt type */
+        if (desc->irq < NR_GIC_SGI)
+            prio = GIC_PRI_IPI;
+
+        gic_route_irq_to_xen(desc, prio);
         /* It's fine to use smp_processor_id() because:
          * For SGI and PPI: irq_desc is banked
          * For SPI: we don't care for now which CPU will receive the
-- 
2.43.0
Re: [PATCH] xen/arm: irq: Use appropriate priority for SGIs in setup_irq()
Posted by Julien Grall 4 months, 3 weeks ago 
Hi Mykola,

On 03/09/2025 03:55, Mykola Kvach wrote:
> From: Mykola Kvach <mykola_kvach@epam.com>
> 
> Use GIC_PRI_IPI priority for SGI interrupts instead of the generic
> GIC_PRI_IRQ priority in setup_irq().
> 
> This change ensures that SGIs get the correct priority level when
> being set up for Xen's use, maintaining proper interrupt precedence
> in the system.
> 
> The priority assignment now follows ARM GIC best practices:
> - SGIs (0-15): GIC_PRI_IPI (higher priority)
> - PPIs/SPIs (16+): GIC_PRI_IRQ (standard priority)

Please provide a reference to the spec. But I don't follow why we should 
follow exactly what the spec suggest. This is up to us to decide what we 
want. Otherwise what's the point of having more than two priorities?

> 
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
>   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?

>       /* First time the IRQ is setup */
>       if ( disabled )
>       {
> -        gic_route_irq_to_xen(desc, GIC_PRI_IRQ);
> +        unsigned int prio = GIC_PRI_IRQ;
> +
> +        /* Use appropriate priority based on interrupt type */
> +        if (desc->irq < NR_GIC_SGI)
> +            prio = GIC_PRI_IPI;

I am a bit split with this change. I feel static SGI (e.g. EVENT_CHECK, 
CALL_FUNCTION) should have higher priority to the dynamic SGIs because 
they are critical for Xen.

Before making my mind, I would like to understand a bit more the use case.

Cheers,

-- 
Julien Grall
Re: [PATCH] xen/arm: irq: Use appropriate priority for SGIs in setup_irq()
Posted by Mykola Kvach 4 months, 3 weeks ago 
Hi Julien,

Thank you for your review and the helpful comments.
I appreciate your time and feedback.

On Sat, Sep 13, 2025 at 1:01 AM Julien Grall <julien@xen.org> wrote:
>
> Hi Mykola,
>
> On 03/09/2025 03:55, Mykola Kvach wrote:
> > From: Mykola Kvach <mykola_kvach@epam.com>
> >
> > Use GIC_PRI_IPI priority for SGI interrupts instead of the generic
> > GIC_PRI_IRQ priority in setup_irq().
> >
> > This change ensures that SGIs get the correct priority level when
> > being set up for Xen's use, maintaining proper interrupt precedence
> > in the system.
> >
> > The priority assignment now follows ARM GIC best practices:
> > - SGIs (0-15): GIC_PRI_IPI (higher priority)
> > - PPIs/SPIs (16+): GIC_PRI_IRQ (standard priority)
>
> Please provide a reference to the spec. But I don't follow why we should
> follow exactly what the spec suggest. This is up to us to decide what we
> want. Otherwise what's the point of having more than two priorities?

To clarify, the GIC specification does not require SGIs to have higher
priority than PPIs or SPIs. My reference to “best practices” is based on
how Xen typically configures SGIs with higher priority during initialization.
This is not a strict requirement, but it helps maintain interrupt precedence
in a way that aligns with established implementations.

If needed, PPIs or SPIs could be assigned higher priority depending on
system requirements.

>
> >
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > ---
> >   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.

>
> >       /* First time the IRQ is setup */
> >       if ( disabled )
> >       {
> > -        gic_route_irq_to_xen(desc, GIC_PRI_IRQ);
> > +        unsigned int prio = GIC_PRI_IRQ;
> > +
> > +        /* Use appropriate priority based on interrupt type */
> > +        if (desc->irq < NR_GIC_SGI)
> > +            prio = GIC_PRI_IPI;
>
> I am a bit split with this change. I feel static SGI (e.g. EVENT_CHECK,
> CALL_FUNCTION) should have higher priority to the dynamic SGIs because
> they are critical for Xen.

That’s a good point. My intention was to follow the general approach of
assigning higher priority to SGIs, as done during GIC initialization.

>
> Before making my mind, I would like to understand a bit more the use case.
>
> Cheers,
>
> --
> Julien Grall
>

Best regards,
Mykola
Re: [PATCH] xen/arm: irq: Use appropriate priority for SGIs in setup_irq()
Posted by Julien Grall 2 months ago 
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@xen.org> wrote:
>>> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
>>> ---
>>>    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?

Cheers,

-- 
Julien Grall
Re: [PATCH] xen/arm: irq: Use appropriate priority for SGIs in setup_irq()
Posted by Bertrand Marquis 2 months ago 
Hi Julien/Mykola,

> On 2 Dec 2025, at 19:26, Julien Grall <julien@xen.org> 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@xen.org> wrote:
>>>> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
>>>> ---
>>>>   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.

Cheers
Bertrand
Re: [PATCH] xen/arm: irq: Use appropriate priority for SGIs in setup_irq()
Posted by Mykola Kvach 3 weeks, 3 days ago 
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@arm.com> wrote:
>
> Hi Julien/Mykola,
>
> > On 2 Dec 2025, at 19:26, Julien Grall <julien@xen.org> 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@xen.org> wrote:
> >>>> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> >>>> ---
> >>>>   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?

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


Best regards,
Mykola

>
> Cheers
> Bertrand
>
Re: [PATCH] xen/arm: irq: Use appropriate priority for SGIs in setup_irq()
Posted by Bertrand Marquis 3 weeks, 2 days ago 
Hi Mykola,

> On 14 Jan 2026, at 07:00, Mykola Kvach <xakep.amatop@gmail.com> 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@arm.com> wrote:
>> 
>> Hi Julien/Mykola,
>> 
>>> On 2 Dec 2025, at 19:26, Julien Grall <julien@xen.org> 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@xen.org> wrote:
>>>>>> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
>>>>>> ---
>>>>>>  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
Re: [PATCH] xen/arm: irq: Use appropriate priority for SGIs in setup_irq()
Posted by Mykola Kvach 2 months, 2 weeks ago 
Hi Julien,

Gentle ping on this patch.

About two months ago I replied to your questions on the priority choice
and on the dynamic SGI use case (ffa_notif). Could you let me know
whether my explanation makes sense, and if you would like me to rework
the patch?

If you think the current approach is not acceptable, I am also happy to
drop or redesign it based on your guidance.

It would also be helpful to get feedback from other Arm maintainers.

Thanks,
Mykola

On Tue, Sep 16, 2025 at 1:19 PM Mykola Kvach <xakep.amatop@gmail.com> wrote:
>
> Hi Julien,
>
> Thank you for your review and the helpful comments.
> I appreciate your time and feedback.
>
> On Sat, Sep 13, 2025 at 1:01 AM Julien Grall <julien@xen.org> wrote:
> >
> > Hi Mykola,
> >
> > On 03/09/2025 03:55, Mykola Kvach wrote:
> > > From: Mykola Kvach <mykola_kvach@epam.com>
> > >
> > > Use GIC_PRI_IPI priority for SGI interrupts instead of the generic
> > > GIC_PRI_IRQ priority in setup_irq().
> > >
> > > This change ensures that SGIs get the correct priority level when
> > > being set up for Xen's use, maintaining proper interrupt precedence
> > > in the system.
> > >
> > > The priority assignment now follows ARM GIC best practices:
> > > - SGIs (0-15): GIC_PRI_IPI (higher priority)
> > > - PPIs/SPIs (16+): GIC_PRI_IRQ (standard priority)
> >
> > Please provide a reference to the spec. But I don't follow why we should
> > follow exactly what the spec suggest. This is up to us to decide what we
> > want. Otherwise what's the point of having more than two priorities?
>
> To clarify, the GIC specification does not require SGIs to have higher
> priority than PPIs or SPIs. My reference to “best practices” is based on
> how Xen typically configures SGIs with higher priority during initialization.
> This is not a strict requirement, but it helps maintain interrupt precedence
> in a way that aligns with established implementations.
>
> If needed, PPIs or SPIs could be assigned higher priority depending on
> system requirements.
>
> >
> > >
> > > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > > ---
> > >   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.
>
> >
> > >       /* First time the IRQ is setup */
> > >       if ( disabled )
> > >       {
> > > -        gic_route_irq_to_xen(desc, GIC_PRI_IRQ);
> > > +        unsigned int prio = GIC_PRI_IRQ;
> > > +
> > > +        /* Use appropriate priority based on interrupt type */
> > > +        if (desc->irq < NR_GIC_SGI)
> > > +            prio = GIC_PRI_IPI;
> >
> > I am a bit split with this change. I feel static SGI (e.g. EVENT_CHECK,
> > CALL_FUNCTION) should have higher priority to the dynamic SGIs because
> > they are critical for Xen.
>
> That’s a good point. My intention was to follow the general approach of
> assigning higher priority to SGIs, as done during GIC initialization.
>
> >
> > Before making my mind, I would like to understand a bit more the use case.
> >
> > Cheers,
> >
> > --
> > Julien Grall
> >
>
> Best regards,
> Mykola
Re: [PATCH] xen/arm: irq: Use appropriate priority for SGIs in setup_irq()
Posted by Oleksandr Tyshchenko 5 months ago 

On 03.09.25 05:55, Mykola Kvach wrote:

Hello Mykola

> From: Mykola Kvach <mykola_kvach@epam.com>
> 
> Use GIC_PRI_IPI priority for SGI interrupts instead of the generic
> GIC_PRI_IRQ priority in setup_irq().
> 
> This change ensures that SGIs get the correct priority level when
> being set up for Xen's use, maintaining proper interrupt precedence
> in the system.
> 
> The priority assignment now follows ARM GIC best practices:
> - SGIs (0-15): GIC_PRI_IPI (higher priority)
> - PPIs/SPIs (16+): GIC_PRI_IRQ (standard priority)
> 
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>

Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>


> ---
>   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)
>       /* First time the IRQ is setup */
>       if ( disabled )
>       {
> -        gic_route_irq_to_xen(desc, GIC_PRI_IRQ);
> +        unsigned int prio = GIC_PRI_IRQ;
> +
> +        /* Use appropriate priority based on interrupt type */
> +        if (desc->irq < NR_GIC_SGI)
> +            prio = GIC_PRI_IPI;
> +
> +        gic_route_irq_to_xen(desc, prio);
>           /* It's fine to use smp_processor_id() because:
>            * For SGI and PPI: irq_desc is banked
>            * For SPI: we don't care for now which CPU will receive the

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.