Re: [RFC Patch v1 2/4] irqchip/gic-v3: Add support to handle SGI as pseudo NMI

[Sumit Garg]

On Fri, 1 May 2020 at 18:33, Sumit Garg <sumit.garg@xxxxxxxxxx> wrote:
>
> Hi Marc,
>
> On Thu, 30 Apr 2020 at 17:43, Sumit Garg <sumit.garg@xxxxxxxxxx> wrote:
> >
> > On Thu, 30 Apr 2020 at 14:43, Marc Zyngier <maz@xxxxxxxxxx> wrote:
> > >
> > > On Thu, 30 Apr 2020 12:50:28 +0530
> > > Sumit Garg <sumit.garg@xxxxxxxxxx> wrote:
> > >
> > > > Hi Marc,
> > > >
> > > > On Wed, 29 Apr 2020 at 13:53, Marc Zyngier <maz@xxxxxxxxxx> wrote:
> > >
> > > [...]
> > >
> > > > > What I would like is for the arch code to request these interrupts as
> > > > > normal interrupts, for which there is one problem to solve: how do you
> > > > > find out about the Linux IRQ number for a given IPI. Or rather, how
> > > > > do you get rid of the requirement to have IPI numbers at all and just
> > > > > say "give me a per-cpu interrupt that I can use as an IPI, and by the
> > > > > way here's the handler you can call".
> > > >
> > > > I think what you are looking for here is something that could be
> > > > sufficed via enabling "CONFIG_GENERIC_IRQ_IPI" framework for arm64/arm
> > > > architectures. It's currently used for mips architecture. Looking at
> > > > its implementation, I think it should be possible to hook up SGIs
> > > > under new IPI irq_domain for GICv2/v3.
> > > >
> > > > So with this framework we should be able to dynamically allocate IPIs
> > > > and use common APIs such as request_irq()/request_nmi() to tell IPI
> > > > specific handlers.
> > > >
> > > > If this approach looks sane to you then I can start working on a PoC
> > > > implementation for arm64.
> > >
> > > I can't say I'm keen. This IPI framework doesn't really work for the
> > > GIC:
> > >
> > > - it requires a separate irqdomain to be able to guarantee that you
> > >   allocate the hwirq in the SGI range. What is the point?
> > > - it creates yet another level of indirection on IPI injection
> > >
> > > This framework was created to deal with two cases:
> > > - systems that can't represent their IPI with a single hwirq spanning
> > >   all the CPUs
> > > - "accelerator cores" that don't run Linux
> > >
> > > The GIC architecture avoids the first point, and I don't even want to
> > > think of the second one.
> > >
> > > Also, it doesn't solve the initial problem, which is to bootstrap the
> > > whole thing. The IPI framework relies on an irqdomain to be created the
> > > first place. This would mean teaching the arch code about the
> > > intricacies of irqdomains, FW nodes and other terrible things. All
> > > things which are better hidden in the GIC drivers (not to mention the
> > > other horror stories that are the RPi-2/3 irqchip and the Huawei GIC
> > > knock-off).
> > >
> > > What I have in mind is to replace the set_smp_cross_call() with
> > > something that passes the required set of information (interrupt range,
> > > at the very least). The only thing that I plan to reuse from the IPI
> > > framework is the chip->ipi_send_mask() callback.
> > >
> >
> > Fair enough, I will just pass the allocated interrupt range base
> > instead of set_smp_cross_call() and use __ipi_send_mask() to invoke a
> > particular IPI.
>
> Thinking more about this, there seems to be multiple irqchip drivers
> registering softirq API via set_smp_cross_call(). So we need to
> introduce a new API instead of replacing set_smp_cross_call() under
> "CONFIG_GENERIC_IRQ_IPI" macro until all drivers switch to IPIs as
> full interrupts.
>
> BTW, could we take up this generalization as follow-up work as it
> seems to be independent of current IPI NMI work?
>
> >
> > And to request an arch specific IPI as NMI, will use
> > arch_get_ipinr_nmi() and in turn use request_percpu_nmi() to turn that
> > particular IPI as NMI.
>
> I have updated the second patch [1] in my tree to incorporate these
> changes. The updated commit log is as follows:
>
> commit 25c96663126264ec758c49a4a01a9c285f4ccc61
> Author: Sumit Garg <sumit.garg@xxxxxxxxxx>
> Date:   Wed Apr 22 16:29:59 2020 +0530
>
>     irqchip/gic-v3: Setup arch specific IPI as pseudo NMI
>
>     Add support to mark arch specific IPI as pseudo NMI. Currently its used
>     to allow arm64 specific IPI_CALL_NMI_FUNC to be marked as pseudo NMI.
>
>     Brief description of changes:
>     - Update NMI setup/teardown routines for SGIs.
>     - Enable NMI support prior to gic_smp_init().
>     - Setup custom flow handler for SGI setup as NMI.
>     - Request, prepare and enable arch specific IPI as per CPU NMI using
>       common APIs.
>
>     Signed-off-by: Sumit Garg <sumit.garg@xxxxxxxxxx>
>
> Please have a look and let me know if this is something you were looking for.
>

In case there are no major objections to this approach, I will post
complete v2 patch-set (alongwith Marc's patches) for detailed review.

-Sumit

> [1] https://git.linaro.org/people/sumit.garg/linux.git/commit/?h=kgdb-nmi&id=25c96663126264ec758c49a4a01a9c285f4ccc61
>
> -Sumit
>
> > > Thanks,
> > >
> > >         M.
> > > --
> > > Jazz is not dead. It just smells funny...