* [PATCH v3 1/4] arm64: smp: Introduce a new IPI as IPI_CALL_NMI_FUNC
2020-09-03 12:05 [PATCH v3 0/4] arm64: Introduce new IPI as IPI_CALL_NMI_FUNC Sumit Garg
@ 2020-09-03 12:05 ` Sumit Garg
2020-09-03 16:36 ` Marc Zyngier
2020-09-03 12:05 ` [PATCH v3 2/4] irqchip/gic-v3: Enable support for SGIs to act as NMIs Sumit Garg
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Sumit Garg @ 2020-09-03 12:05 UTC (permalink / raw)
To: maz, catalin.marinas, will
Cc: linux-arm-kernel, tglx, jason, julien.thierry.kdev, dianders,
daniel.thompson, jason.wessel, kgdb-bugreport, linux-kernel,
Sumit Garg
Introduce a new inter processor interrupt as IPI_CALL_NMI_FUNC that
can be invoked to run special handlers in NMI context. One such handler
example is kgdb_nmicallback() which is invoked in order to round up CPUs
to enter kgdb context.
As currently pseudo NMIs are supported on specific arm64 platforms which
incorporates GICv3 or later version of interrupt controller. In case a
particular platform doesn't support pseudo NMIs, IPI_CALL_NMI_FUNC will
act as a normal IPI which can still be used to invoke special handlers.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
arch/arm64/include/asm/smp.h | 1 +
arch/arm64/kernel/smp.c | 11 +++++++++++
2 files changed, 12 insertions(+)
diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
index 2e7f529..e85f5d5 100644
--- a/arch/arm64/include/asm/smp.h
+++ b/arch/arm64/include/asm/smp.h
@@ -89,6 +89,7 @@ extern void secondary_entry(void);
extern void arch_send_call_function_single_ipi(int cpu);
extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
+extern void arch_send_call_nmi_func_ipi_mask(const struct cpumask *mask);
#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index b6bde26..1b4c07c 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -74,6 +74,7 @@ enum ipi_msg_type {
IPI_TIMER,
IPI_IRQ_WORK,
IPI_WAKEUP,
+ IPI_CALL_NMI_FUNC,
NR_IPI
};
@@ -793,6 +794,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
S(IPI_TIMER, "Timer broadcast interrupts"),
S(IPI_IRQ_WORK, "IRQ work interrupts"),
S(IPI_WAKEUP, "CPU wake-up interrupts"),
+ S(IPI_CALL_NMI_FUNC, "NMI function call interrupts"),
};
static void smp_cross_call(const struct cpumask *target, unsigned int ipinr);
@@ -840,6 +842,11 @@ void arch_irq_work_raise(void)
}
#endif
+void arch_send_call_nmi_func_ipi_mask(const struct cpumask *mask)
+{
+ smp_cross_call(mask, IPI_CALL_NMI_FUNC);
+}
+
static void local_cpu_stop(void)
{
set_cpu_online(smp_processor_id(), false);
@@ -932,6 +939,10 @@ static void do_handle_IPI(int ipinr)
break;
#endif
+ case IPI_CALL_NMI_FUNC:
+ /* nop, IPI handlers for special features can be added here. */
+ break;
+
default:
pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
break;
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/4] arm64: smp: Introduce a new IPI as IPI_CALL_NMI_FUNC
2020-09-03 12:05 ` [PATCH v3 1/4] arm64: smp: Introduce a " Sumit Garg
@ 2020-09-03 16:36 ` Marc Zyngier
2020-09-04 5:30 ` Sumit Garg
0 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2020-09-03 16:36 UTC (permalink / raw)
To: Sumit Garg
Cc: catalin.marinas, will, linux-arm-kernel, tglx, jason,
julien.thierry.kdev, dianders, daniel.thompson, jason.wessel,
kgdb-bugreport, linux-kernel
On 2020-09-03 13:05, Sumit Garg wrote:
> Introduce a new inter processor interrupt as IPI_CALL_NMI_FUNC that
> can be invoked to run special handlers in NMI context. One such handler
> example is kgdb_nmicallback() which is invoked in order to round up
> CPUs
> to enter kgdb context.
>
> As currently pseudo NMIs are supported on specific arm64 platforms
> which
> incorporates GICv3 or later version of interrupt controller. In case a
> particular platform doesn't support pseudo NMIs, IPI_CALL_NMI_FUNC will
> act as a normal IPI which can still be used to invoke special handlers.
>
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
> arch/arm64/include/asm/smp.h | 1 +
> arch/arm64/kernel/smp.c | 11 +++++++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/arch/arm64/include/asm/smp.h
> b/arch/arm64/include/asm/smp.h
> index 2e7f529..e85f5d5 100644
> --- a/arch/arm64/include/asm/smp.h
> +++ b/arch/arm64/include/asm/smp.h
> @@ -89,6 +89,7 @@ extern void secondary_entry(void);
>
> extern void arch_send_call_function_single_ipi(int cpu);
> extern void arch_send_call_function_ipi_mask(const struct cpumask
> *mask);
> +extern void arch_send_call_nmi_func_ipi_mask(const struct cpumask
> *mask);
>
> #ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index b6bde26..1b4c07c 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -74,6 +74,7 @@ enum ipi_msg_type {
> IPI_TIMER,
> IPI_IRQ_WORK,
> IPI_WAKEUP,
> + IPI_CALL_NMI_FUNC,
> NR_IPI
> };
>
> @@ -793,6 +794,7 @@ static const char *ipi_types[NR_IPI]
> __tracepoint_string = {
> S(IPI_TIMER, "Timer broadcast interrupts"),
> S(IPI_IRQ_WORK, "IRQ work interrupts"),
> S(IPI_WAKEUP, "CPU wake-up interrupts"),
> + S(IPI_CALL_NMI_FUNC, "NMI function call interrupts"),
> };
>
> static void smp_cross_call(const struct cpumask *target, unsigned int
> ipinr);
> @@ -840,6 +842,11 @@ void arch_irq_work_raise(void)
> }
> #endif
>
> +void arch_send_call_nmi_func_ipi_mask(const struct cpumask *mask)
> +{
> + smp_cross_call(mask, IPI_CALL_NMI_FUNC);
> +}
> +
> static void local_cpu_stop(void)
> {
> set_cpu_online(smp_processor_id(), false);
> @@ -932,6 +939,10 @@ static void do_handle_IPI(int ipinr)
> break;
> #endif
>
> + case IPI_CALL_NMI_FUNC:
> + /* nop, IPI handlers for special features can be added here. */
> + break;
> +
> default:
> pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
> break;
I'm really not keen on adding more IPIs to the SMP code. One of the
main reasons for using these SGIs as normal IRQs was to make them
"requestable" from non-arch code as if they were standard percpu
interrupts.
What prevents you from moving that all the way to the kgdb code?
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/4] arm64: smp: Introduce a new IPI as IPI_CALL_NMI_FUNC
2020-09-03 16:36 ` Marc Zyngier
@ 2020-09-04 5:30 ` Sumit Garg
2020-09-04 8:00 ` Marc Zyngier
0 siblings, 1 reply; 9+ messages in thread
From: Sumit Garg @ 2020-09-04 5:30 UTC (permalink / raw)
To: Marc Zyngier
Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, Thomas Gleixner,
Jason Cooper, julien.thierry.kdev, Douglas Anderson,
Daniel Thompson, Jason Wessel, kgdb-bugreport,
Linux Kernel Mailing List
On Thu, 3 Sep 2020 at 22:06, Marc Zyngier <maz@kernel.org> wrote:
>
> On 2020-09-03 13:05, Sumit Garg wrote:
> > Introduce a new inter processor interrupt as IPI_CALL_NMI_FUNC that
> > can be invoked to run special handlers in NMI context. One such handler
> > example is kgdb_nmicallback() which is invoked in order to round up
> > CPUs
> > to enter kgdb context.
> >
> > As currently pseudo NMIs are supported on specific arm64 platforms
> > which
> > incorporates GICv3 or later version of interrupt controller. In case a
> > particular platform doesn't support pseudo NMIs, IPI_CALL_NMI_FUNC will
> > act as a normal IPI which can still be used to invoke special handlers.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> > arch/arm64/include/asm/smp.h | 1 +
> > arch/arm64/kernel/smp.c | 11 +++++++++++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/smp.h
> > b/arch/arm64/include/asm/smp.h
> > index 2e7f529..e85f5d5 100644
> > --- a/arch/arm64/include/asm/smp.h
> > +++ b/arch/arm64/include/asm/smp.h
> > @@ -89,6 +89,7 @@ extern void secondary_entry(void);
> >
> > extern void arch_send_call_function_single_ipi(int cpu);
> > extern void arch_send_call_function_ipi_mask(const struct cpumask
> > *mask);
> > +extern void arch_send_call_nmi_func_ipi_mask(const struct cpumask
> > *mask);
> >
> > #ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> > extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
> > diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> > index b6bde26..1b4c07c 100644
> > --- a/arch/arm64/kernel/smp.c
> > +++ b/arch/arm64/kernel/smp.c
> > @@ -74,6 +74,7 @@ enum ipi_msg_type {
> > IPI_TIMER,
> > IPI_IRQ_WORK,
> > IPI_WAKEUP,
> > + IPI_CALL_NMI_FUNC,
> > NR_IPI
> > };
> >
> > @@ -793,6 +794,7 @@ static const char *ipi_types[NR_IPI]
> > __tracepoint_string = {
> > S(IPI_TIMER, "Timer broadcast interrupts"),
> > S(IPI_IRQ_WORK, "IRQ work interrupts"),
> > S(IPI_WAKEUP, "CPU wake-up interrupts"),
> > + S(IPI_CALL_NMI_FUNC, "NMI function call interrupts"),
> > };
> >
> > static void smp_cross_call(const struct cpumask *target, unsigned int
> > ipinr);
> > @@ -840,6 +842,11 @@ void arch_irq_work_raise(void)
> > }
> > #endif
> >
> > +void arch_send_call_nmi_func_ipi_mask(const struct cpumask *mask)
> > +{
> > + smp_cross_call(mask, IPI_CALL_NMI_FUNC);
> > +}
> > +
> > static void local_cpu_stop(void)
> > {
> > set_cpu_online(smp_processor_id(), false);
> > @@ -932,6 +939,10 @@ static void do_handle_IPI(int ipinr)
> > break;
> > #endif
> >
> > + case IPI_CALL_NMI_FUNC:
> > + /* nop, IPI handlers for special features can be added here. */
> > + break;
> > +
> > default:
> > pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
> > break;
>
> I'm really not keen on adding more IPIs to the SMP code. One of the
> main reasons for using these SGIs as normal IRQs was to make them
> "requestable" from non-arch code as if they were standard percpu
> interrupts.
>
> What prevents you from moving that all the way to the kgdb code?
>
Since we only have one SGI left (SGI7) which this patch reserves for
NMI purposes, I am not sure what value add do you see to make it
requestable from non-arch code.
Also, allocating SGI7 entirely to kgdb would not allow us to leverage
it for NMI backtrace support via magic sysrq. But with current
implementation we should be able to achieve that.
Moreover, irq ids for normal interrupts are assigned via DT/ACPI, how
do you envision this for SGIs?
-Sumit
> M.
> --
> Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/4] arm64: smp: Introduce a new IPI as IPI_CALL_NMI_FUNC
2020-09-04 5:30 ` Sumit Garg
@ 2020-09-04 8:00 ` Marc Zyngier
2020-09-04 11:32 ` Sumit Garg
0 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2020-09-04 8:00 UTC (permalink / raw)
To: Sumit Garg
Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, Thomas Gleixner,
Jason Cooper, julien.thierry.kdev, Douglas Anderson,
Daniel Thompson, Jason Wessel, kgdb-bugreport,
Linux Kernel Mailing List
On 2020-09-04 06:30, Sumit Garg wrote:
> On Thu, 3 Sep 2020 at 22:06, Marc Zyngier <maz@kernel.org> wrote:
>>
>> On 2020-09-03 13:05, Sumit Garg wrote:
>> > Introduce a new inter processor interrupt as IPI_CALL_NMI_FUNC that
>> > can be invoked to run special handlers in NMI context. One such handler
>> > example is kgdb_nmicallback() which is invoked in order to round up
>> > CPUs
>> > to enter kgdb context.
>> >
>> > As currently pseudo NMIs are supported on specific arm64 platforms
>> > which
>> > incorporates GICv3 or later version of interrupt controller. In case a
>> > particular platform doesn't support pseudo NMIs, IPI_CALL_NMI_FUNC will
>> > act as a normal IPI which can still be used to invoke special handlers.
>> >
>> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
>> > ---
>> > arch/arm64/include/asm/smp.h | 1 +
>> > arch/arm64/kernel/smp.c | 11 +++++++++++
>> > 2 files changed, 12 insertions(+)
>> >
>> > diff --git a/arch/arm64/include/asm/smp.h
>> > b/arch/arm64/include/asm/smp.h
>> > index 2e7f529..e85f5d5 100644
>> > --- a/arch/arm64/include/asm/smp.h
>> > +++ b/arch/arm64/include/asm/smp.h
>> > @@ -89,6 +89,7 @@ extern void secondary_entry(void);
>> >
>> > extern void arch_send_call_function_single_ipi(int cpu);
>> > extern void arch_send_call_function_ipi_mask(const struct cpumask
>> > *mask);
>> > +extern void arch_send_call_nmi_func_ipi_mask(const struct cpumask
>> > *mask);
>> >
>> > #ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
>> > extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
>> > diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
>> > index b6bde26..1b4c07c 100644
>> > --- a/arch/arm64/kernel/smp.c
>> > +++ b/arch/arm64/kernel/smp.c
>> > @@ -74,6 +74,7 @@ enum ipi_msg_type {
>> > IPI_TIMER,
>> > IPI_IRQ_WORK,
>> > IPI_WAKEUP,
>> > + IPI_CALL_NMI_FUNC,
>> > NR_IPI
>> > };
>> >
>> > @@ -793,6 +794,7 @@ static const char *ipi_types[NR_IPI]
>> > __tracepoint_string = {
>> > S(IPI_TIMER, "Timer broadcast interrupts"),
>> > S(IPI_IRQ_WORK, "IRQ work interrupts"),
>> > S(IPI_WAKEUP, "CPU wake-up interrupts"),
>> > + S(IPI_CALL_NMI_FUNC, "NMI function call interrupts"),
>> > };
>> >
>> > static void smp_cross_call(const struct cpumask *target, unsigned int
>> > ipinr);
>> > @@ -840,6 +842,11 @@ void arch_irq_work_raise(void)
>> > }
>> > #endif
>> >
>> > +void arch_send_call_nmi_func_ipi_mask(const struct cpumask *mask)
>> > +{
>> > + smp_cross_call(mask, IPI_CALL_NMI_FUNC);
>> > +}
>> > +
>> > static void local_cpu_stop(void)
>> > {
>> > set_cpu_online(smp_processor_id(), false);
>> > @@ -932,6 +939,10 @@ static void do_handle_IPI(int ipinr)
>> > break;
>> > #endif
>> >
>> > + case IPI_CALL_NMI_FUNC:
>> > + /* nop, IPI handlers for special features can be added here. */
>> > + break;
>> > +
>> > default:
>> > pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
>> > break;
>>
>> I'm really not keen on adding more IPIs to the SMP code. One of the
>> main reasons for using these SGIs as normal IRQs was to make them
>> "requestable" from non-arch code as if they were standard percpu
>> interrupts.
>>
>> What prevents you from moving that all the way to the kgdb code?
>>
>
> Since we only have one SGI left (SGI7) which this patch reserves for
> NMI purposes, I am not sure what value add do you see to make it
> requestable from non-arch code.
We have one *guaranteed* SGI left. Potentially another 8 which we
could dynamically discover (reading the priority registers should
be enough to weed out the secure SGIs). And I'd like to keep that
last interrupt "just in case" we'd need it to workaround something.
> Also, allocating SGI7 entirely to kgdb would not allow us to leverage
> it for NMI backtrace support via magic sysrq. But with current
> implementation we should be able to achieve that.
I'd argue that there is no need for this interrupt to be a "well known
number". Maybe putting this code in the kgdb code is one step too far,
but keeping out of the arm64 SMP code is IMO the right thing to do.
And if NMI backtracing is a thing, then we should probably implement
that first.
> Moreover, irq ids for normal interrupts are assigned via DT/ACPI, how
> do you envision this for SGIs?
Nothing could be further from the truth. How do you think MSIs work?
We'd just bolt an allocator for non-arch IPIs.
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/4] arm64: smp: Introduce a new IPI as IPI_CALL_NMI_FUNC
2020-09-04 8:00 ` Marc Zyngier
@ 2020-09-04 11:32 ` Sumit Garg
0 siblings, 0 replies; 9+ messages in thread
From: Sumit Garg @ 2020-09-04 11:32 UTC (permalink / raw)
To: Marc Zyngier
Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, Thomas Gleixner,
Jason Cooper, julien.thierry.kdev, Douglas Anderson,
Daniel Thompson, Jason Wessel, kgdb-bugreport,
Linux Kernel Mailing List
On Fri, 4 Sep 2020 at 13:30, Marc Zyngier <maz@kernel.org> wrote:
>
> On 2020-09-04 06:30, Sumit Garg wrote:
> > On Thu, 3 Sep 2020 at 22:06, Marc Zyngier <maz@kernel.org> wrote:
> >>
> >> On 2020-09-03 13:05, Sumit Garg wrote:
> >> > Introduce a new inter processor interrupt as IPI_CALL_NMI_FUNC that
> >> > can be invoked to run special handlers in NMI context. One such handler
> >> > example is kgdb_nmicallback() which is invoked in order to round up
> >> > CPUs
> >> > to enter kgdb context.
> >> >
> >> > As currently pseudo NMIs are supported on specific arm64 platforms
> >> > which
> >> > incorporates GICv3 or later version of interrupt controller. In case a
> >> > particular platform doesn't support pseudo NMIs, IPI_CALL_NMI_FUNC will
> >> > act as a normal IPI which can still be used to invoke special handlers.
> >> >
> >> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> >> > ---
> >> > arch/arm64/include/asm/smp.h | 1 +
> >> > arch/arm64/kernel/smp.c | 11 +++++++++++
> >> > 2 files changed, 12 insertions(+)
> >> >
> >> > diff --git a/arch/arm64/include/asm/smp.h
> >> > b/arch/arm64/include/asm/smp.h
> >> > index 2e7f529..e85f5d5 100644
> >> > --- a/arch/arm64/include/asm/smp.h
> >> > +++ b/arch/arm64/include/asm/smp.h
> >> > @@ -89,6 +89,7 @@ extern void secondary_entry(void);
> >> >
> >> > extern void arch_send_call_function_single_ipi(int cpu);
> >> > extern void arch_send_call_function_ipi_mask(const struct cpumask
> >> > *mask);
> >> > +extern void arch_send_call_nmi_func_ipi_mask(const struct cpumask
> >> > *mask);
> >> >
> >> > #ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> >> > extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
> >> > diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> >> > index b6bde26..1b4c07c 100644
> >> > --- a/arch/arm64/kernel/smp.c
> >> > +++ b/arch/arm64/kernel/smp.c
> >> > @@ -74,6 +74,7 @@ enum ipi_msg_type {
> >> > IPI_TIMER,
> >> > IPI_IRQ_WORK,
> >> > IPI_WAKEUP,
> >> > + IPI_CALL_NMI_FUNC,
> >> > NR_IPI
> >> > };
> >> >
> >> > @@ -793,6 +794,7 @@ static const char *ipi_types[NR_IPI]
> >> > __tracepoint_string = {
> >> > S(IPI_TIMER, "Timer broadcast interrupts"),
> >> > S(IPI_IRQ_WORK, "IRQ work interrupts"),
> >> > S(IPI_WAKEUP, "CPU wake-up interrupts"),
> >> > + S(IPI_CALL_NMI_FUNC, "NMI function call interrupts"),
> >> > };
> >> >
> >> > static void smp_cross_call(const struct cpumask *target, unsigned int
> >> > ipinr);
> >> > @@ -840,6 +842,11 @@ void arch_irq_work_raise(void)
> >> > }
> >> > #endif
> >> >
> >> > +void arch_send_call_nmi_func_ipi_mask(const struct cpumask *mask)
> >> > +{
> >> > + smp_cross_call(mask, IPI_CALL_NMI_FUNC);
> >> > +}
> >> > +
> >> > static void local_cpu_stop(void)
> >> > {
> >> > set_cpu_online(smp_processor_id(), false);
> >> > @@ -932,6 +939,10 @@ static void do_handle_IPI(int ipinr)
> >> > break;
> >> > #endif
> >> >
> >> > + case IPI_CALL_NMI_FUNC:
> >> > + /* nop, IPI handlers for special features can be added here. */
> >> > + break;
> >> > +
> >> > default:
> >> > pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
> >> > break;
> >>
> >> I'm really not keen on adding more IPIs to the SMP code. One of the
> >> main reasons for using these SGIs as normal IRQs was to make them
> >> "requestable" from non-arch code as if they were standard percpu
> >> interrupts.
> >>
> >> What prevents you from moving that all the way to the kgdb code?
> >>
> >
> > Since we only have one SGI left (SGI7) which this patch reserves for
> > NMI purposes, I am not sure what value add do you see to make it
> > requestable from non-arch code.
>
> We have one *guaranteed* SGI left. Potentially another 8 which we
> could dynamically discover (reading the priority registers should
> be enough to weed out the secure SGIs). And I'd like to keep that
> last interrupt "just in case" we'd need it to workaround something.
>
> > Also, allocating SGI7 entirely to kgdb would not allow us to leverage
> > it for NMI backtrace support via magic sysrq. But with current
> > implementation we should be able to achieve that.
>
> I'd argue that there is no need for this interrupt to be a "well known
> number". Maybe putting this code in the kgdb code is one step too far,
> but keeping out of the arm64 SMP code is IMO the right thing to do.
IIUC, you would prefer to only allocate SGI7 (only one left) if there
is a corresponding user. And I agree that kgdb isn't commonly active
for most users. But I think magic sysrq is enabled for most users and
supporting NMI backtrace would be quite useful while debugging systems
in the field as well.
So if you like, I can create a separate file like
"arch/arm64/kernel/ipi_nmi.c" for this implementation.
> And if NMI backtracing is a thing, then we should probably implement
> that first.
Have a look at IPI_CPU_BACKTRACE implementation for arm [1]. Similar
implementation would be required for arm64 but now with IPI turned as
a pseudo NMI. So I will try to add corresponding support.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/kernel/smp.c#n72
>
> > Moreover, irq ids for normal interrupts are assigned via DT/ACPI, how
> > do you envision this for SGIs?
>
> Nothing could be further from the truth. How do you think MSIs work?
> We'd just bolt an allocator for non-arch IPIs.
>
Okay.
-Sumit
> M.
> --
> Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/4] irqchip/gic-v3: Enable support for SGIs to act as NMIs
2020-09-03 12:05 [PATCH v3 0/4] arm64: Introduce new IPI as IPI_CALL_NMI_FUNC Sumit Garg
2020-09-03 12:05 ` [PATCH v3 1/4] arm64: smp: Introduce a " Sumit Garg
@ 2020-09-03 12:05 ` Sumit Garg
2020-09-03 12:05 ` [PATCH v3 3/4] arm64: smp: Setup IPI_CALL_NMI_FUNC as a pseudo NMI Sumit Garg
2020-09-03 12:05 ` [PATCH v3 4/4] arm64: kgdb: Round up cpus using IPI_CALL_NMI_FUNC Sumit Garg
3 siblings, 0 replies; 9+ messages in thread
From: Sumit Garg @ 2020-09-03 12:05 UTC (permalink / raw)
To: maz, catalin.marinas, will
Cc: linux-arm-kernel, tglx, jason, julien.thierry.kdev, dianders,
daniel.thompson, jason.wessel, kgdb-bugreport, linux-kernel,
Sumit Garg
Add support to handle SGIs as regular NMIs. As SGIs or IPIs defaults to a
special flow handler: handle_percpu_devid_fasteoi_ipi(), so skip NMI
handler update in case of SGIs.
Also, enable NMI support prior to gic_smp_init() as allocation of SGIs
as IRQs/NMIs happen as part of this routine.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
drivers/irqchip/irq-gic-v3.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 7170645..dfd8e03 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -476,6 +476,11 @@ static int gic_irq_nmi_setup(struct irq_data *d)
if (WARN_ON(gic_irq(d) >= 8192))
return -EINVAL;
+ if (get_intid_range(d) == SGI_RANGE) {
+ gic_irq_set_prio(d, GICD_INT_NMI_PRI);
+ return 0;
+ }
+
/* desc lock should already be held */
if (gic_irq_in_rdist(d)) {
u32 idx = gic_get_ppi_index(d);
@@ -513,6 +518,11 @@ static void gic_irq_nmi_teardown(struct irq_data *d)
if (WARN_ON(gic_irq(d) >= 8192))
return;
+ if (get_intid_range(d) == SGI_RANGE) {
+ gic_irq_set_prio(d, GICD_INT_DEF_PRI);
+ return;
+ }
+
/* desc lock should already be held */
if (gic_irq_in_rdist(d)) {
u32 idx = gic_get_ppi_index(d);
@@ -1666,6 +1676,7 @@ static int __init gic_init_bases(void __iomem *dist_base,
gic_dist_init();
gic_cpu_init();
+ gic_enable_nmi_support();
gic_smp_init();
gic_cpu_pm_init();
@@ -1677,8 +1688,6 @@ static int __init gic_init_bases(void __iomem *dist_base,
gicv2m_init(handle, gic_data.domain);
}
- gic_enable_nmi_support();
-
return 0;
out_free:
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 3/4] arm64: smp: Setup IPI_CALL_NMI_FUNC as a pseudo NMI
2020-09-03 12:05 [PATCH v3 0/4] arm64: Introduce new IPI as IPI_CALL_NMI_FUNC Sumit Garg
2020-09-03 12:05 ` [PATCH v3 1/4] arm64: smp: Introduce a " Sumit Garg
2020-09-03 12:05 ` [PATCH v3 2/4] irqchip/gic-v3: Enable support for SGIs to act as NMIs Sumit Garg
@ 2020-09-03 12:05 ` Sumit Garg
2020-09-03 12:05 ` [PATCH v3 4/4] arm64: kgdb: Round up cpus using IPI_CALL_NMI_FUNC Sumit Garg
3 siblings, 0 replies; 9+ messages in thread
From: Sumit Garg @ 2020-09-03 12:05 UTC (permalink / raw)
To: maz, catalin.marinas, will
Cc: linux-arm-kernel, tglx, jason, julien.thierry.kdev, dianders,
daniel.thompson, jason.wessel, kgdb-bugreport, linux-kernel,
Sumit Garg
Setup IPI_CALL_NMI_FUNC as a pseudo NMI using generic interrupt framework
APIs. In case a plarform doesn't provide support for pseudo NMIs, switch
back to IPI_CALL_NMI_FUNC being a normal interrupt.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
arch/arm64/kernel/smp.c | 38 ++++++++++++++++++++++++++++----------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 1b4c07c..572f8f5 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -81,6 +81,7 @@ enum ipi_msg_type {
static int ipi_irq_base __read_mostly;
static int nr_ipi __read_mostly = NR_IPI;
static struct irq_desc *ipi_desc[NR_IPI] __read_mostly;
+static int ipi_nmi = -1;
static void ipi_setup(int cpu);
static void ipi_teardown(int cpu);
@@ -971,8 +972,14 @@ static void ipi_setup(int cpu)
if (WARN_ON_ONCE(!ipi_irq_base))
return;
- for (i = 0; i < nr_ipi; i++)
- enable_percpu_irq(ipi_irq_base + i, 0);
+ for (i = 0; i < nr_ipi; i++) {
+ if (ipi_nmi == ipi_irq_base + i) {
+ if (!prepare_percpu_nmi(ipi_nmi))
+ enable_percpu_nmi(ipi_nmi, 0);
+ } else {
+ enable_percpu_irq(ipi_irq_base + i, 0);
+ }
+ }
}
static void ipi_teardown(int cpu)
@@ -982,23 +989,34 @@ static void ipi_teardown(int cpu)
if (WARN_ON_ONCE(!ipi_irq_base))
return;
- for (i = 0; i < nr_ipi; i++)
- disable_percpu_irq(ipi_irq_base + i);
+ for (i = 0; i < nr_ipi; i++) {
+ if (ipi_nmi == ipi_irq_base + i) {
+ disable_percpu_nmi(ipi_nmi);
+ teardown_percpu_nmi(ipi_nmi);
+ } else {
+ disable_percpu_irq(ipi_irq_base + i);
+ }
+ }
}
void __init set_smp_ipi_range(int ipi_base, int n)
{
- int i;
+ int i, err;
WARN_ON(n < NR_IPI);
nr_ipi = min(n, NR_IPI);
- for (i = 0; i < nr_ipi; i++) {
- int err;
+ err = request_percpu_nmi(ipi_base + IPI_CALL_NMI_FUNC,
+ ipi_handler, "IPI", &cpu_number);
+ if (!err)
+ ipi_nmi = ipi_base + IPI_CALL_NMI_FUNC;
- err = request_percpu_irq(ipi_base + i, ipi_handler,
- "IPI", &cpu_number);
- WARN_ON(err);
+ for (i = 0; i < nr_ipi; i++) {
+ if (ipi_base + i != ipi_nmi) {
+ err = request_percpu_irq(ipi_base + i, ipi_handler,
+ "IPI", &cpu_number);
+ WARN_ON(err);
+ }
ipi_desc[i] = irq_to_desc(ipi_base + i);
irq_set_status_flags(ipi_base + i, IRQ_HIDDEN);
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 4/4] arm64: kgdb: Round up cpus using IPI_CALL_NMI_FUNC
2020-09-03 12:05 [PATCH v3 0/4] arm64: Introduce new IPI as IPI_CALL_NMI_FUNC Sumit Garg
` (2 preceding siblings ...)
2020-09-03 12:05 ` [PATCH v3 3/4] arm64: smp: Setup IPI_CALL_NMI_FUNC as a pseudo NMI Sumit Garg
@ 2020-09-03 12:05 ` Sumit Garg
3 siblings, 0 replies; 9+ messages in thread
From: Sumit Garg @ 2020-09-03 12:05 UTC (permalink / raw)
To: maz, catalin.marinas, will
Cc: linux-arm-kernel, tglx, jason, julien.thierry.kdev, dianders,
daniel.thompson, jason.wessel, kgdb-bugreport, linux-kernel,
Sumit Garg
arm64 platforms with GICv3 or later supports pseudo NMIs which can be
leveraged to round up CPUs which are stuck in hard lockup state with
interrupts disabled that wouldn't be possible with a normal IPI.
So instead switch to round up CPUs using IPI_CALL_NMI_FUNC. And in
case a particular arm64 platform doesn't supports pseudo NMIs,
IPI_CALL_NMI_FUNC will act as a normal IPI which maintains existing
kgdb functionality.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
arch/arm64/include/asm/kgdb.h | 8 ++++++++
arch/arm64/kernel/kgdb.c | 21 +++++++++++++++++++++
arch/arm64/kernel/smp.c | 3 ++-
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/kgdb.h b/arch/arm64/include/asm/kgdb.h
index 21fc85e..6f3d3af 100644
--- a/arch/arm64/include/asm/kgdb.h
+++ b/arch/arm64/include/asm/kgdb.h
@@ -24,6 +24,14 @@ static inline void arch_kgdb_breakpoint(void)
extern void kgdb_handle_bus_error(void);
extern int kgdb_fault_expected;
+#ifdef CONFIG_KGDB
+extern void ipi_kgdb_nmicallback(int cpu, void *regs);
+#else
+static inline void ipi_kgdb_nmicallback(int cpu, void *regs)
+{
+}
+#endif
+
#endif /* !__ASSEMBLY__ */
/*
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index 1a157ca3..3a8ed97 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -14,6 +14,7 @@
#include <linux/kgdb.h>
#include <linux/kprobes.h>
#include <linux/sched/task_stack.h>
+#include <linux/smp.h>
#include <asm/debug-monitors.h>
#include <asm/insn.h>
@@ -353,3 +354,23 @@ int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
return aarch64_insn_write((void *)bpt->bpt_addr,
*(u32 *)bpt->saved_instr);
}
+
+void ipi_kgdb_nmicallback(int cpu, void *regs)
+{
+ if (atomic_read(&kgdb_active) != -1)
+ kgdb_nmicallback(cpu, regs);
+}
+
+#ifdef CONFIG_SMP
+void kgdb_roundup_cpus(void)
+{
+ struct cpumask mask;
+
+ cpumask_copy(&mask, cpu_online_mask);
+ cpumask_clear_cpu(raw_smp_processor_id(), &mask);
+ if (cpumask_empty(&mask))
+ return;
+
+ arch_send_call_nmi_func_ipi_mask(&mask);
+}
+#endif
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 572f8f5..b4760d3 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -32,6 +32,7 @@
#include <linux/irq_work.h>
#include <linux/kernel_stat.h>
#include <linux/kexec.h>
+#include <linux/kgdb.h>
#include <linux/kvm_host.h>
#include <asm/alternative.h>
@@ -941,7 +942,7 @@ static void do_handle_IPI(int ipinr)
#endif
case IPI_CALL_NMI_FUNC:
- /* nop, IPI handlers for special features can be added here. */
+ ipi_kgdb_nmicallback(cpu, get_irq_regs());
break;
default:
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread