Hi Marc, Thanks for your comments and apologies for my delayed response as I was exploring ideas that you have shared.
On Sat, 25 Apr 2020 at 20:02, Marc Zyngier <m...@kernel.org> wrote: > > On 2020-04-25 11:29, Marc Zyngier wrote: > > On Fri, 24 Apr 2020 16:39:12 +0530 > > Sumit Garg <sumit.g...@linaro.org> wrote: > > > > Hi Sumit, > > > >> With pseudo NMIs enabled, interrupt controller can be configured to > >> deliver SGI as a pseudo NMI. So add corresponding handling for SGIs. > >> > >> Signed-off-by: Sumit Garg <sumit.g...@linaro.org> > >> --- > >> drivers/irqchip/irq-gic-v3.c | 22 +++++++++++++++++----- > >> 1 file changed, 17 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/irqchip/irq-gic-v3.c > >> b/drivers/irqchip/irq-gic-v3.c > >> index d7006ef..be361bf 100644 > >> --- a/drivers/irqchip/irq-gic-v3.c > >> +++ b/drivers/irqchip/irq-gic-v3.c > >> @@ -609,17 +609,29 @@ static inline void gic_handle_nmi(u32 irqnr, > >> struct pt_regs *regs) > >> if (irqs_enabled) > >> nmi_enter(); > >> > >> - if (static_branch_likely(&supports_deactivate_key)) > >> - gic_write_eoir(irqnr); > >> /* > >> * Leave the PSR.I bit set to prevent other NMIs to be > >> * received while handling this one. > >> * PSR.I will be restored when we ERET to the > >> * interrupted context. > >> */ > >> - err = handle_domain_nmi(gic_data.domain, irqnr, regs); > >> - if (err) > >> - gic_deactivate_unhandled(irqnr); > >> + if (likely(irqnr > 15)) { > >> + if (static_branch_likely(&supports_deactivate_key)) > >> + gic_write_eoir(irqnr); > >> + > >> + err = handle_domain_nmi(gic_data.domain, irqnr, regs); > >> + if (err) > >> + gic_deactivate_unhandled(irqnr); > >> + } else { > >> + gic_write_eoir(irqnr); > >> + if (static_branch_likely(&supports_deactivate_key)) > >> + gic_write_dir(irqnr); > >> +#ifdef CONFIG_SMP > >> + handle_IPI(irqnr, regs); > >> +#else > >> + WARN_ONCE(true, "Unexpected SGI received!\n"); > >> +#endif > >> + } > >> > >> if (irqs_enabled) > >> nmi_exit(); > > > > If there is one thing I would like to avoid, it is to add more ugly > > hacks to the way we handle SGIs. There is very little reason why SGIs > > should be handled differently from all other interrupts. They have the > > same properties, and it is only because of the 32bit legacy that we > > deal > > with them in such a cumbersome way. Nothing that we cannot fix though. > > > > What I would really like to see is first a conversion of the SGIs to > > normal, full fat interrupts. These interrupts can then be configured as > > NMI using the normal API. > > > > I think Julien had something along these lines (or was that limited to > > the PMU?). Otherwise, I'll happily help you with that. > > OK, to give you an idea of what I am after, here's a small series[1] > that > can be used as a base (it has been booted exactly *once* on a model, and > is thus absolutely perfect ;-). Thanks for this series. I have re-based my patch-set on top of this series [1] and just dropped this patch #2. It works fine for me. > > There is still a bit of work to be able to actually request a SGI (they > are hard-wired as chained interrupts so far, as this otherwise changes > the output of /proc/interrupts, among other things), but you will > hopefully see what I'm aiming for. I was exploring this idea: "request a SGI". I guess here you meant to request a new SGI as a normal NMI/IRQ via common APIs such as request_percpu_nmi() or request_percpu_irq() rather than statically adding a new IPI as per this patch [2], correct? If yes, then I have following follow up queries: 1. Do you envision any drivers to use SGIs in a similar manner as they use SPIs or PPIs? 2. How do you envision allocation of SGIs as currently they are hardcoded in an arch specific file (like arch/arm64/kernel/smp.c +794)? 3. AFAIK, the major difference among SGIs and SPIs or PPIs is the trigger method where SGIs are software triggered and SPIs or PPIs are hardware triggered. And I couldn't find a generalized method across architectures to invoke SGIs. So how do you envision drivers to invoke SGIs in an architecture agnostic manner? [1] https://git.linaro.org/people/sumit.garg/linux.git/?h=kgdb-nmi [2] https://git.linaro.org/people/sumit.garg/linux.git/commit/?h=kgdb-nmi&id=fc89e5f395f89966110554a15ab0fa0f0d471132 -Sumit > > Thanks, > > M. > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/gic-sgi > -- > Jazz is not dead. It just smells funny...