| Message ID | 20241108122909.763663-1-patryk.wlazlyn@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | SRF: Fix offline CPU preventing pc6 entry | expand |
On 11/8/24 04:29, Patryk Wlazlyn wrote:
> Applied suggestions from Dave and Rafael.
The basic approach here is looking pretty sound, so thanks for that
Patryk. I mostly have mechanical nits left.
On Fri, Nov 08, 2024 at 01:29:06PM +0100, Patryk Wlazlyn wrote: > Removing the existing "kexec hack" by bringing all offlined CPUs back > online before proceeding with the kexec would make it even simpler, but > I am not sure we can do that. It looks kind of obvious to me, but for > some reason the hack exist. There's a comment there that explains why this is done. If you don't understand this, then please don't touch this code.
> There's a comment there that explains why this is done. If you don't > understand this, then please don't touch this code. /* * Kexec is about to happen. Don't go back into mwait() as * the kexec kernel might overwrite text and data including * page tables and stack. So mwait() would resume when the * monitor cache line is written to and then the CPU goes * south due to overwritten text, page tables and stack. * * Note: This does _NOT_ protect against a stray MCE, NMI, * SMI. They will resume execution at the instruction * following the HLT instruction and run into the problem * which this is trying to prevent. */ If you are referring to this comment above, I do understand the need to enter hlt loop before the kexec happens. I thought that I could bring all of the offlined CPUs back online, effectively getting them out of the mwait loop.
On Tue, Nov 12 2024 at 16:43, Patryk Wlazlyn wrote: >> There's a comment there that explains why this is done. If you don't >> understand this, then please don't touch this code. > > /* > * Kexec is about to happen. Don't go back into mwait() as > * the kexec kernel might overwrite text and data including > * page tables and stack. So mwait() would resume when the > * monitor cache line is written to and then the CPU goes > * south due to overwritten text, page tables and stack. > * > * Note: This does _NOT_ protect against a stray MCE, NMI, > * SMI. They will resume execution at the instruction > * following the HLT instruction and run into the problem > * which this is trying to prevent. > */ > > If you are referring to this comment above, I do understand the need to > enter hlt loop before the kexec happens. I thought that I could bring > all of the offlined CPUs back online, effectively getting them out of > the mwait loop. That's not really working: 1) Regular kexec offlines them again. 2) Kexec in panic can't do any of that. Thanks tglx
Applied suggestions from Dave and Rafael. Because we now call a smp function, we have to have ifdefs for CONFIG_SMP or rely on mwait_play_dead_with_hint to return immediatelly with an error on non-smp builds. I decided to do the later, but maybe we should return -ENODEV (or other error constant) instead of 1. I am open for suggestions. Removing the existing "kexec hack" by bringing all offlined CPUs back online before proceeding with the kexec would make it even simpler, but I am not sure we can do that. It looks kind of obvious to me, but for some reason the hack exist. Changes since v2: The whole approach changed, but there main things are below. * Split mwait_play_dead (old code) into two parts: 1. Computing mwait hint based on cpuid leaf 0x5 2. Entering while(1) mwait loop with "kexec hack" handling * Prefer cpuidle_play_dead over mwait_play_dead by reordering calls in native_play_dead. * Add implementation for enter_dead() handler in intel_idle that calls executes old mwait_play_dead code, but with the mwait hint from the cpuidle state table. Patryk Wlazlyn (3): x86/smp: Allow calling mwait_play_dead with arbitrary hint x86/smp native_play_dead: Prefer cpuidle_play_dead() over mwait_play_dead() intel_idle: Provide enter_dead() handler for SRF arch/x86/include/asm/smp.h | 6 ++++++ arch/x86/kernel/smpboot.c | 29 ++++++++++++++++++++--------- drivers/idle/intel_idle.c | 16 ++++++++++++++++ 3 files changed, 42 insertions(+), 9 deletions(-)