| Message ID | 20240409153630.2026584-5-jens.wiklander@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | FF-A notifications | expand |
Hi Jens, On 09/04/2024 17:36, Jens Wiklander wrote: > > > Updates so request_irq() can be used with a dynamically assigned SGI irq > as input. At this point it would be handy to mention the use case for which you need to add this support > > gic_route_irq_to_xen() don't gic_set_irq_type() for SGIs since they have > their type assigned earlier during boot Could you elaborate more on that? Do you mean that SGIs are always edge triggered and there's no need for setting the type? > > gic_interrupt() is updated to route the dynamically assigned SGIs to > do_IRQ() instead of do_sgi(). The latter still handles the statically > assigned SGI handlers like for instance GIC_SGI_CALL_FUNCTION. > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> Other than that, it LGTM: Acked-by: Michal Orzel <michal.orzel@amd.com> but I would like other maintainers (especially Julien) to cross check it. ~Michal
Hi Michal, On Wed, Apr 10, 2024 at 3:24 PM Michal Orzel <michal.orzel@amd.com> wrote: > > Hi Jens, > > On 09/04/2024 17:36, Jens Wiklander wrote: > > > > > > Updates so request_irq() can be used with a dynamically assigned SGI irq > > as input. > At this point it would be handy to mention the use case for which you need to add this support OK, I'll add something like: This prepares for a later patch where an FF-A schedule receiver interrupt handler is installed for an SGI generated by the secure world. > > > > > gic_route_irq_to_xen() don't gic_set_irq_type() for SGIs since they have > > their type assigned earlier during boot > Could you elaborate more on that? Do you mean that SGIs are always edge triggered and there's no need > for setting the type? Yes, see https://developer.arm.com/documentation/ihi0069/h 4.4 Software Generated Interrupts SGIs are typically used for inter-processor communication, and are generated by a write to an SGI register in the GIC. SGIs can be either Group 0 or Group 1 interrupts, and they can support only edge-triggered behavior. How about: SGI should only be configured as edge triggered. Thanks, Jens > > > > > gic_interrupt() is updated to route the dynamically assigned SGIs to > > do_IRQ() instead of do_sgi(). The latter still handles the statically > > assigned SGI handlers like for instance GIC_SGI_CALL_FUNCTION. > > > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> > Other than that, it LGTM: > Acked-by: Michal Orzel <michal.orzel@amd.com> > > but I would like other maintainers (especially Julien) to cross check it. > > ~Michal
Hi Jens, On 11/04/2024 08:12, Jens Wiklander wrote: > > > Hi Michal, > > On Wed, Apr 10, 2024 at 3:24 PM Michal Orzel <michal.orzel@amd.com> wrote: >> >> Hi Jens, >> >> On 09/04/2024 17:36, Jens Wiklander wrote: >>> >>> >>> Updates so request_irq() can be used with a dynamically assigned SGI irq >>> as input. >> At this point it would be handy to mention the use case for which you need to add this support > > OK, I'll add something like: > This prepares for a later patch where an FF-A schedule receiver > interrupt handler is installed for an SGI generated by the secure > world. ok > >> >>> >>> gic_route_irq_to_xen() don't gic_set_irq_type() for SGIs since they have >>> their type assigned earlier during boot >> Could you elaborate more on that? Do you mean that SGIs are always edge triggered and there's no need >> for setting the type? > > Yes, see https://developer.arm.com/documentation/ihi0069/h > 4.4 Software Generated Interrupts > SGIs are typically used for inter-processor communication, and are > generated by a write to an SGI register in the > GIC. SGIs can be either Group 0 or Group 1 interrupts, and they can > support only edge-triggered behavior. Exactly. But you wrote "have their type assigned earlier during boot" which is not true. There is no write to ICFGR0 in Xen codebase. They are implicitly edge triggered. So I would write: "... for SGIs since they are always edge triggered" ~Michal
Hi Michal, On Thu, Apr 11, 2024 at 8:44 AM Michal Orzel <michal.orzel@amd.com> wrote: > > Hi Jens, > > On 11/04/2024 08:12, Jens Wiklander wrote: > > > > > > Hi Michal, > > > > On Wed, Apr 10, 2024 at 3:24 PM Michal Orzel <michal.orzel@amd.com> wrote: > >> > >> Hi Jens, > >> > >> On 09/04/2024 17:36, Jens Wiklander wrote: > >>> > >>> > >>> Updates so request_irq() can be used with a dynamically assigned SGI irq > >>> as input. > >> At this point it would be handy to mention the use case for which you need to add this support > > > > OK, I'll add something like: > > This prepares for a later patch where an FF-A schedule receiver > > interrupt handler is installed for an SGI generated by the secure > > world. > ok > > > > >> > >>> > >>> gic_route_irq_to_xen() don't gic_set_irq_type() for SGIs since they have > >>> their type assigned earlier during boot > >> Could you elaborate more on that? Do you mean that SGIs are always edge triggered and there's no need > >> for setting the type? > > > > Yes, see https://developer.arm.com/documentation/ihi0069/h > > 4.4 Software Generated Interrupts > > SGIs are typically used for inter-processor communication, and are > > generated by a write to an SGI register in the > > GIC. SGIs can be either Group 0 or Group 1 interrupts, and they can > > support only edge-triggered behavior. > Exactly. But you wrote "have their type assigned earlier during boot" which is not true. > There is no write to ICFGR0 in Xen codebase. They are implicitly edge triggered. > So I would write: > "... for SGIs since they are always edge triggered" I'll use that instead. Thanks, Jens > > ~Michal
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 44c40e86defe..e9aeb7138455 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -117,7 +117,8 @@ void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority) desc->handler = gic_hw_ops->gic_host_irq_type; - gic_set_irq_type(desc, desc->arch.type); + if ( desc->irq >= NR_GIC_SGI) + gic_set_irq_type(desc, desc->arch.type); gic_set_irq_priority(desc, priority); } @@ -375,7 +376,7 @@ void gic_interrupt(struct cpu_user_regs *regs, int is_fiq) /* Reading IRQ will ACK it */ irq = gic_hw_ops->read_irq(); - if ( likely(irq >= 16 && irq < 1020) ) + if ( likely(irq >= GIC_SGI_MAX && irq < 1020) ) { isb(); do_IRQ(regs, irq, is_fiq); diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c index bcce80a4d624..fdb214560978 100644 --- a/xen/arch/arm/irq.c +++ b/xen/arch/arm/irq.c @@ -224,9 +224,12 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq) perfc_incr(irqs); - ASSERT(irq >= 16); /* SGIs do not come down this path */ + /* Statically assigned SGIs do not come down this path */ + ASSERT(irq >= GIC_SGI_MAX); - if ( irq < 32 ) + if ( irq < NR_GIC_SGI ) + perfc_incr(ipis); + else if ( irq < NR_GIC_LOCAL_IRQS ) perfc_incr(ppis); else perfc_incr(spis);
Updates so request_irq() can be used with a dynamically assigned SGI irq as input. gic_route_irq_to_xen() don't gic_set_irq_type() for SGIs since they have their type assigned earlier during boot gic_interrupt() is updated to route the dynamically assigned SGIs to do_IRQ() instead of do_sgi(). The latter still handles the statically assigned SGI handlers like for instance GIC_SGI_CALL_FUNCTION. Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> --- xen/arch/arm/gic.c | 5 +++-- xen/arch/arm/irq.c | 7 +++++-- 2 files changed, 8 insertions(+), 4 deletions(-)