| Message ID | 20230824083012.v11.3.I7209db47ef8ec151d3de61f59005bbc59fe8f113@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Add IPI for backtraces / kgdb; try to use NMI for some IPIs | expand |
Quoting Douglas Anderson (2023-08-24 08:30:29) > From: Mark Rutland <mark.rutland@arm.com> > > To enable NMI backtrace and KGDB's NMI cpu roundup, we need to free up > at least one dedicated IPI. > > On arm64 the IPI_WAKEUP IPI is only used for the ACPI parking protocol, > which itself is only used on some very early ARMv8 systems which > couldn't implement PSCI. > > Remove the IPI_WAKEUP IPI, and rely on the IPI_RESCHEDULE IPI to wake > CPUs from the parked state. This will cause a tiny amonut of redundant > work to check the thread flags, but this is miniscule in relation to the > cost of taking and handling the IPI in the first place. We can safely > handle redundant IPI_RESCHEDULE IPIs, so there should be no functional > impact as a result of this change. > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Sumit Garg <sumit.garg@linaro.org> > Cc: Will Deacon <will@kernel.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
On Thu, 24 Aug 2023 at 21:03, Douglas Anderson <dianders@chromium.org> wrote: > > From: Mark Rutland <mark.rutland@arm.com> > > To enable NMI backtrace and KGDB's NMI cpu roundup, we need to free up > at least one dedicated IPI. > > On arm64 the IPI_WAKEUP IPI is only used for the ACPI parking protocol, > which itself is only used on some very early ARMv8 systems which > couldn't implement PSCI. > > Remove the IPI_WAKEUP IPI, and rely on the IPI_RESCHEDULE IPI to wake > CPUs from the parked state. This will cause a tiny amonut of redundant > work to check the thread flags, but this is miniscule in relation to the > cost of taking and handling the IPI in the first place. We can safely > handle redundant IPI_RESCHEDULE IPIs, so there should be no functional > impact as a result of this change. > > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Sumit Garg <sumit.garg@linaro.org> > Cc: Will Deacon <will@kernel.org> Reviewed-by: Sumit Garg <sumit.garg@linaro.org> -Sumit > --- > I have no idea how to test this. I just took Mark's patch and jammed > it into my series. Logicially the patch seems reasonable to me. > > Changes in v11: > - arch_send_wakeup_ipi() now takes an unsigned int. > > Changes in v10: > - ("arm64: smp: Remove dedicated wakeup IPI") new for v10. > > arch/arm64/include/asm/smp.h | 4 ++-- > arch/arm64/kernel/acpi_parking_protocol.c | 2 +- > arch/arm64/kernel/smp.c | 28 +++++++++-------------- > 3 files changed, 14 insertions(+), 20 deletions(-) > > diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h > index 9b31e6d0da17..efb13112b408 100644 > --- a/arch/arm64/include/asm/smp.h > +++ b/arch/arm64/include/asm/smp.h > @@ -89,9 +89,9 @@ extern void arch_send_call_function_single_ipi(int cpu); > extern void arch_send_call_function_ipi_mask(const struct cpumask *mask); > > #ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL > -extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask); > +extern void arch_send_wakeup_ipi(unsigned int cpu); > #else > -static inline void arch_send_wakeup_ipi_mask(const struct cpumask *mask) > +static inline void arch_send_wakeup_ipi(unsigned int cpu) > { > BUILD_BUG(); > } > diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c > index b1990e38aed0..e1be29e608b7 100644 > --- a/arch/arm64/kernel/acpi_parking_protocol.c > +++ b/arch/arm64/kernel/acpi_parking_protocol.c > @@ -103,7 +103,7 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu) > &mailbox->entry_point); > writel_relaxed(cpu_entry->gic_cpu_id, &mailbox->cpu_id); > > - arch_send_wakeup_ipi_mask(cpumask_of(cpu)); > + arch_send_wakeup_ipi(cpu); > > return 0; > } > diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c > index 960b98b43506..a5848f1ef817 100644 > --- a/arch/arm64/kernel/smp.c > +++ b/arch/arm64/kernel/smp.c > @@ -72,7 +72,6 @@ enum ipi_msg_type { > IPI_CPU_CRASH_STOP, > IPI_TIMER, > IPI_IRQ_WORK, > - IPI_WAKEUP, > NR_IPI > }; > > @@ -764,7 +763,6 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = { > [IPI_CPU_CRASH_STOP] = "CPU stop (for crash dump) interrupts", > [IPI_TIMER] = "Timer broadcast interrupts", > [IPI_IRQ_WORK] = "IRQ work interrupts", > - [IPI_WAKEUP] = "CPU wake-up interrupts", > }; > > static void smp_cross_call(const struct cpumask *target, unsigned int ipinr); > @@ -797,13 +795,6 @@ void arch_send_call_function_single_ipi(int cpu) > smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC); > } > > -#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL > -void arch_send_wakeup_ipi_mask(const struct cpumask *mask) > -{ > - smp_cross_call(mask, IPI_WAKEUP); > -} > -#endif > - > #ifdef CONFIG_IRQ_WORK > void arch_irq_work_raise(void) > { > @@ -897,14 +888,6 @@ static void do_handle_IPI(int ipinr) > break; > #endif > > -#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL > - case IPI_WAKEUP: > - WARN_ONCE(!acpi_parking_protocol_valid(cpu), > - "CPU%u: Wake-up IPI outside the ACPI parking protocol\n", > - cpu); > - break; > -#endif > - > default: > pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr); > break; > @@ -979,6 +962,17 @@ void arch_smp_send_reschedule(int cpu) > smp_cross_call(cpumask_of(cpu), IPI_RESCHEDULE); > } > > +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL > +void arch_send_wakeup_ipi(unsigned int cpu) > +{ > + /* > + * We use a scheduler IPI to wake the CPU as this avoids the need for a > + * dedicated IPI and we can safely handle spurious scheduler IPIs. > + */ > + arch_smp_send_reschedule(cpu); > +} > +#endif > + > #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST > void tick_broadcast(const struct cpumask *mask) > { > -- > 2.42.0.rc1.204.g551eb34607-goog >
diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h index 9b31e6d0da17..efb13112b408 100644 --- a/arch/arm64/include/asm/smp.h +++ b/arch/arm64/include/asm/smp.h @@ -89,9 +89,9 @@ extern void arch_send_call_function_single_ipi(int cpu); extern void arch_send_call_function_ipi_mask(const struct cpumask *mask); #ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL -extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask); +extern void arch_send_wakeup_ipi(unsigned int cpu); #else -static inline void arch_send_wakeup_ipi_mask(const struct cpumask *mask) +static inline void arch_send_wakeup_ipi(unsigned int cpu) { BUILD_BUG(); } diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c index b1990e38aed0..e1be29e608b7 100644 --- a/arch/arm64/kernel/acpi_parking_protocol.c +++ b/arch/arm64/kernel/acpi_parking_protocol.c @@ -103,7 +103,7 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu) &mailbox->entry_point); writel_relaxed(cpu_entry->gic_cpu_id, &mailbox->cpu_id); - arch_send_wakeup_ipi_mask(cpumask_of(cpu)); + arch_send_wakeup_ipi(cpu); return 0; } diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c index 960b98b43506..a5848f1ef817 100644 --- a/arch/arm64/kernel/smp.c +++ b/arch/arm64/kernel/smp.c @@ -72,7 +72,6 @@ enum ipi_msg_type { IPI_CPU_CRASH_STOP, IPI_TIMER, IPI_IRQ_WORK, - IPI_WAKEUP, NR_IPI }; @@ -764,7 +763,6 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = { [IPI_CPU_CRASH_STOP] = "CPU stop (for crash dump) interrupts", [IPI_TIMER] = "Timer broadcast interrupts", [IPI_IRQ_WORK] = "IRQ work interrupts", - [IPI_WAKEUP] = "CPU wake-up interrupts", }; static void smp_cross_call(const struct cpumask *target, unsigned int ipinr); @@ -797,13 +795,6 @@ void arch_send_call_function_single_ipi(int cpu) smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC); } -#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL -void arch_send_wakeup_ipi_mask(const struct cpumask *mask) -{ - smp_cross_call(mask, IPI_WAKEUP); -} -#endif - #ifdef CONFIG_IRQ_WORK void arch_irq_work_raise(void) { @@ -897,14 +888,6 @@ static void do_handle_IPI(int ipinr) break; #endif -#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL - case IPI_WAKEUP: - WARN_ONCE(!acpi_parking_protocol_valid(cpu), - "CPU%u: Wake-up IPI outside the ACPI parking protocol\n", - cpu); - break; -#endif - default: pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr); break; @@ -979,6 +962,17 @@ void arch_smp_send_reschedule(int cpu) smp_cross_call(cpumask_of(cpu), IPI_RESCHEDULE); } +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL +void arch_send_wakeup_ipi(unsigned int cpu) +{ + /* + * We use a scheduler IPI to wake the CPU as this avoids the need for a + * dedicated IPI and we can safely handle spurious scheduler IPIs. + */ + arch_smp_send_reschedule(cpu); +} +#endif + #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST void tick_broadcast(const struct cpumask *mask) {