Backport: add IPI for backtraces/kgdb and try to use NMI for some IPIs - #147
Open
changruinj wants to merge 11 commits into
Open
changruinj wants to merge 11 commits into
changruinj wants to merge 11 commits into
Conversation
commit a02026b upstream. As of commit 6abbd69 ("irqchip/gic, gic-v3: Make SGIs use handle_percpu_devid_irq()") SGIs are treated the same as PPIs/EPPIs and use handle_percpu_devid_irq() by default. Unfortunately, handle_percpu_devid_irq() isn't NMI safe, and so to run in an NMI context those should use handle_percpu_devid_fasteoi_nmi(). In order to accomplish this, we just have to make room for SGIs in the array of refcounts that keeps track of which interrupts are set as NMI. We also rename the array and create a new indexing scheme that accounts for SGIs. Also, enable NMI support prior to gic_smp_init() as allocation of SGIs as IRQs/NMIs happen as part of this routine. Co-developed-by: Sumit Garg <sumit.garg@linaro.org> Signed-off-by: Sumit Garg <sumit.garg@linaro.org> Acked-by: Mark Rutland <mark.rutland@arm.com> Tested-by: Chen-Yu Tsai <wenst@chromium.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> Acked-by: Marc Zyngier <maz@kernel.org> Link: https://lore.kernel.org/r/20230906090246.v13.1.I1223c11c88937bd0cbd9b086d4ef216985797302@changeid Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Rui Chang <rui.chang@arm.com>
commit d0c14a7 upstream. As per the (somewhat recent) comment before the definition of `__cpuidle`, the tag is like `noinstr` but also marks a function so it can be identified by cpu_in_idle(). Let's add these markings to arm64 cpuidle functions With this change we get useful backtraces like: NMI backtrace for cpu N skipped: idling at cpu_do_idle+0x94/0x98 instead of useless backtraces when dumping all processors using nmi_cpu_backtrace(). NOTE: this patch won't make cpu_in_idle() work perfectly for arm64, but it doesn't hurt and does catch some cases. Specifically an example that wasn't caught in my testing looked like this: gic_cpu_sys_reg_init+0x1f8/0x314 gic_cpu_pm_notifier+0x40/0x78 raw_notifier_call_chain+0x5c/0x134 cpu_pm_notify+0x38/0x64 cpu_pm_exit+0x20/0x2c psci_enter_idle_state+0x48/0x70 cpuidle_enter_state+0xb8/0x260 cpuidle_enter+0x44/0x5c do_idle+0x188/0x30c Acked-by: Mark Rutland <mark.rutland@arm.com> Reviewed-by: Stephen Boyd <swboyd@chromium.org> Acked-by: Sumit Garg <sumit.garg@linaro.org> Tested-by: Chen-Yu Tsai <wenst@chromium.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> Link: https://lore.kernel.org/r/20230906090246.v13.2.I4baba13e220bdd24d11400c67f137c35f07f82c7@changeid Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Rui Chang <rui.chang@arm.com>
commit 2b2d0a7 upstream. 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> Reviewed-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Sumit Garg <sumit.garg@linaro.org> Tested-by: Chen-Yu Tsai <wenst@chromium.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> Cc: Marc Zyngier <maz@kernel.org> Cc: Will Deacon <will@kernel.org> Link: https://lore.kernel.org/r/20230906090246.v13.3.I7209db47ef8ec151d3de61f59005bbc59fe8f113@changeid Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Rui Chang <rui.chang@arm.com>
commit 331a1b3 upstream. Enable arch_trigger_cpumask_backtrace() support on arm64. This enables things much like they are enabled on arm32 (including some of the funky logic around NR_IPI, nr_ipi, and MAX_IPI) but with the difference that, unlike arm32, we'll try to enable the backtrace to use pseudo-NMI. NOTE: this patch is a squash of the little bit of code adding the ability to mark an IPI to try to use pseudo-NMI plus the little bit of code to hook things up for kgdb. This approach was decided upon in the discussion of v9 [1]. This patch depends on commit 8d539b8 ("nmi_backtrace: allow excluding an arbitrary CPU") since that commit changed the prototype of arch_trigger_cpumask_backtrace(), which this patch implements. [1] https://lore.kernel.org/r/ZORY51mF4alI41G1@FVFF77S0Q05N Co-developed-by: Sumit Garg <sumit.garg@linaro.org> Signed-off-by: Sumit Garg <sumit.garg@linaro.org> Co-developed-by: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Mark Rutland <mark.rutland@arm.com> Reviewed-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Misono Tomohiro <misono.tomohiro@fujitsu.com> Tested-by: Chen-Yu Tsai <wenst@chromium.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> Link: https://lore.kernel.org/r/20230906090246.v13.4.Ie6c132b96ebbbcddbf6954b9469ed40a6960343c@changeid Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Rui Chang <rui.chang@arm.com>
commit d740251 upstream. There's no reason why IPI_CPU_STOP and IPI_CPU_CRASH_STOP can't be handled as NMI. They are very simple and everything in them is NMI-safe. Mark them as things to use NMI for if NMI is available. Suggested-by: Mark Rutland <mark.rutland@arm.com> Reviewed-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Misono Tomohiro <misono.tomohiro@fujitsu.com> Reviewed-by: Sumit Garg <sumit.garg@linaro.org> Acked-by: Mark Rutland <mark.rutland@arm.com> Tested-by: Mark Rutland <mark.rutland@arm.com> Tested-by: Chen-Yu Tsai <wenst@chromium.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> Link: https://lore.kernel.org/r/20230906090246.v13.5.Ifadbfd45b22c52edcb499034dd4783d096343260@changeid Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Rui Chang <rui.chang@arm.com>
commit 2f5cd0c upstream. Up until now we've been using the generic (weak) implementation for kgdb_roundup_cpus() when using kgdb on arm64. Let's move to a custom one. The advantage here is that, when pseudo-NMI is enabled on a device, we'll be able to round up CPUs using pseudo-NMI. This allows us to debug CPUs that are stuck with interrupts disabled. If pseudo-NMIs are not enabled then we'll fallback to just using an IPI, which is still slightly better than the generic implementation since it avoids the potential situation described in the generic kgdb_call_nmi_hook(). Co-developed-by: Sumit Garg <sumit.garg@linaro.org> Signed-off-by: Sumit Garg <sumit.garg@linaro.org> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> Reviewed-by: Stephen Boyd <swboyd@chromium.org> Acked-by: Mark Rutland <mark.rutland@arm.com> Tested-by: Chen-Yu Tsai <wenst@chromium.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> Link: https://lore.kernel.org/r/20230906090246.v13.6.I2ef26d1b3bfbed2d10a281942b0da7d9854de05e@changeid Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Rui Chang <rui.chang@arm.com>
commit 62817d5 upstream. Mark the three IPI-related globals in smp.c as "__ro_after_init" since they are only ever set in set_smp_ipi_range(), which is marked "__init". This is a better and more secure marking than the old "__read_mostly". Suggested-by: Stephen Boyd <swboyd@chromium.org> Acked-by: Mark Rutland <mark.rutland@arm.com> Tested-by: Chen-Yu Tsai <wenst@chromium.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Stephen Boyd <swboyd@chromium.org> Link: https://lore.kernel.org/r/20230906090246.v13.7.I625d393afd71e1766ef73d3bfaac0b347a4afd19@changeid Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Rui Chang <rui.chang@arm.com>
commit ef31b8c upstream. In commit 2b2d0a7 ("arm64: smp: Remove dedicated wakeup IPI") we started using a scheduler IPI to avoid a dedicated reschedule. When we did this, we used arch_smp_send_reschedule() directly rather than calling smp_send_reschedule(). The only difference is that calling arch_smp_send_reschedule() directly avoids tracing. Presumably we _don't_ want to avoid tracing here, so switch to smp_send_reschedule(). Fixes: 2b2d0a7 ("arm64: smp: Remove dedicated wakeup IPI") Signed-off-by: Douglas Anderson <dianders@chromium.org> Acked-by: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Rui Chang <rui.chang@arm.com>
commit a07a594 upstream. Some MediaTek devices have broken firmware which corrupts some GICR registers behind the back of the OS, and pseudo-NMIs cannot be used on these devices. For more details see commit: 44bd78d ("irqchip/gic-v3: Disable pseudo NMIs on Mediatek devices w/ firmware issues") We did not take this problem into account in commit: 331a1b3 ("arm64: smp: Add arch support for backtrace using pseudo-NMI") Since that commit arm64's SMP code will try to setup some IPIs as pseudo-NMIs, even on systems with broken FW. The GICv3 code will (rightly) reject attempts to request interrupts as pseudo-NMIs, resulting in boot-time failures. Avoid the problem by taking the broken FW into account when deciding to request IPIs as pseudo-NMIs. The GICv3 driver maintains a static_key named "supports_pseudo_nmis" which is false on systems with broken FW, and we can consult this within ipi_should_be_nmi(). Fixes: 331a1b3 ("arm64: smp: Add arch support for backtrace using pseudo-NMI") Reported-by: Chen-Yu Tsai <wenst@chromium.org> Closes: https://issuetracker.google.com/issues/197061987#comment68 Signed-off-by: Mark Rutland <mark.rutland@arm.com> Reviewed-by: Douglas Anderson <dianders@chromium.org> Tested-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Marc Zyngier <maz@kernel.org> Tested-by: Chen-Yu Tsai <wenst@chromium.org> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Rui Chang <rui.chang@arm.com>
commit 916b93f upstream. commit 83cfac9 ("genirq: Allow interrupts to be excluded from /proc/interrupts") is to avoid IPIs appear twice in /proc/interrupts. But the commit 331a1b3 ("arm64: smp: Add arch support for backtrace using pseudo-NMI") and commit 2f5cd0c("arm64: kgdb: Implement kgdb_roundup_cpus() to enable pseudo-NMI roundup") set CPU_BACKTRACE and KGDB_ROUNDUP IPIs "IRQ_HIDDEN" flag but not show them in arch_show_interrupts(), which cause the interrupt kstat_irqs accounting is missing in display. Before this patch, CPU_BACKTRACE and KGDB_ROUNDUP IPIs are missing: / # cat /proc/interrupts CPU0 CPU1 CPU2 CPU3 11: 466 600 309 332 GICv3 27 Level arch_timer 13: 24 0 0 0 GICv3 33 Level uart-pl011 15: 64 0 0 0 GICv3 78 Edge virtio0 16: 0 0 0 0 GICv3 79 Edge virtio1 17: 0 0 0 0 GICv3 34 Level rtc-pl031 18: 3 3 3 3 GICv3 23 Level arm-pmu 19: 0 0 0 0 9030000.pl061 3 Edge GPIO Key Poweroff IPI0: 7 14 9 26 Rescheduling interrupts IPI1: 354 93 233 255 Function call interrupts IPI2: 0 0 0 0 CPU stop interrupts IPI3: 0 0 0 0 CPU stop (for crash dump) interrupts IPI4: 0 0 0 0 Timer broadcast interrupts IPI5: 1 0 0 0 IRQ work interrupts Err: 0 After this pacth, CPU_BACKTRACE and KGDB_ROUNDUP IPIs are displayed: / # cat /proc/interrupts CPU0 CPU1 CPU2 CPU3 11: 393 281 532 449 GICv3 27 Level arch_timer 13: 15 0 0 0 GICv3 33 Level uart-pl011 15: 64 0 0 0 GICv3 78 Edge virtio0 16: 0 0 0 0 GICv3 79 Edge virtio1 17: 0 0 0 0 GICv3 34 Level rtc-pl031 18: 2 2 2 2 GICv3 23 Level arm-pmu 19: 0 0 0 0 9030000.pl061 3 Edge GPIO Key Poweroff IPI0: 11 19 4 23 Rescheduling interrupts IPI1: 279 347 222 72 Function call interrupts IPI2: 0 0 0 0 CPU stop interrupts IPI3: 0 0 0 0 CPU stop (for crash dump) interrupts IPI4: 0 0 0 0 Timer broadcast interrupts IPI5: 1 0 0 1 IRQ work interrupts IPI6: 0 0 0 0 CPU backtrace interrupts IPI7: 0 0 0 0 KGDB roundup interrupts Err: 0 Fixes: 331a1b3 ("arm64: smp: Add arch support for backtrace using pseudo-NMI") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> Suggested-by: Doug Anderson <dianders@chromium.org> Acked-by: Will Deacon <will@kernel.org> Reviewed-by: Douglas Anderson <dianders@chromium.org> Link: https://lore.kernel.org/r/20240620063600.573559-1-ruanjinjie@huawei.com Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Rui Chang <rui.chang@arm.com>
…NMI first commit fdfa588 upstream. When testing hard lockup handling on my sc7180-trogdor-lazor device with pseudo-NMI enabled, with serial console enabled and with kgdb disabled, I found that the stack crawls printed to the serial console ended up as a jumbled mess. After rebooting, the pstore-based console looked fine though. Also, enabling kgdb to trap the panic made the console look fine and avoided the mess. After a bit of tracking down, I came to the conclusion that this was what was happening: 1. The panic path was stopping all other CPUs with panic_other_cpus_shutdown(). 2. At least one of those other CPUs was in the middle of printing to the serial console and holding the console port's lock, which is grabbed with "irqsave". ...but since we were stopping with an NMI we didn't care about the "irqsave" and interrupted anyway. 3. Since we stopped the CPU while it was holding the lock it would never release it. 4. All future calls to output to the console would end up failing to get the lock in qcom_geni_serial_console_write(). This isn't _totally_ unexpected at panic time but it's a code path that's not well tested, hard to get right, and apparently doesn't work terribly well on the Qualcomm geni serial driver. The Qualcomm geni serial driver was fixed to be a bit better in commit 9e957a1 ("serial: qcom-geni: Don't cancel/abort if we can't get the port lock") but it's nice not to get into this situation in the first place. Taking a page from what x86 appears to do in native_stop_other_cpus(), do this: 1. First, try to stop other CPUs with a normal IPI and wait a second. This gives them a chance to leave critical sections. 2. If CPUs fail to stop then retry with an NMI, but give a much lower timeout since there's no good reason for a CPU not to react quickly to a NMI. This works well and avoids the corrupted console and (presumably) could help avoid other similar issues. In order to do this, we need to do a little re-organization of our IPIs since we don't have any more free IDs. Do what was suggested in previous conversations and combine "stop" and "crash stop". That frees up an IPI so now we can have a "stop" and "stop NMI". In order to do this we also need a slight change in the way we keep track of which CPUs still need to be stopped. We need to know specifically which CPUs haven't stopped yet when we fall back to NMI but in the "crash stop" case the "cpu_online_mask" isn't updated as CPUs go down. This is why that code path had an atomic of the number of CPUs left. Solve this by also updating the "cpu_online_mask" for crash stops. All of the above lets us combine the logic for "stop" and "crash stop" code, which appeared to have a bunch of arbitrary implementation differences. Aside from the above change where we try a normal IPI and then an NMI, the combined function has a few subtle differences: * In the normal smp_send_stop(), if we fail to stop one or more CPUs then we won't include the current CPU (the one running smp_send_stop()) in the error message. * In crash_smp_send_stop(), if we fail to stop some CPUs we'll print the CPUs that we failed to stop instead of printing all _but_ the current running CPU. * In crash_smp_send_stop(), we will now only print "SMP: stopping secondary CPUs" if (system_state <= SYSTEM_RUNNING). Fixes: d740251 ("arm64: smp: IPI_CPU_STOP and IPI_CPU_CRASH_STOP should try for NMI") Signed-off-by: Douglas Anderson <dianders@chromium.org> Link: https://lore.kernel.org/r/20240821145353.v3.1.Id4817adef610302554b8aa42b090d57270dc119c@changeid Signed-off-by: Will Deacon <will@kernel.org> Signed-off-by: Rui Chang <rui.chang@arm.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport this patch serial: https://lore.kernel.org/linux-arm-kernel/20230906160505.2431857-1-dianders@chromium.org/
The commits in the list:
The subsequent fixes:
ByteDance static check passed:
Kselftest passed
Run arm64 and cpu-hotplug test. There is no dedicated test for the path serials.
Unit tests done