linux-kernel - Re: [PATCH v12 06/11] x86/smpboot: Remove initial

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <64B60F62-3760-43A0-A0FB-C349DA70C013@infradead.org>
Date:   Tue, 28 Feb 2023 20:43:41 +0000
From:   David Woodhouse <dwmw2@...radead.org>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Usama Arif <usama.arif@...edance.com>, kim.phillips@....com,
        brgerst@...il.com
CC:     piotrgorski@...hyos.org, oleksandr@...alenko.name,
        arjan@...ux.intel.com, mingo@...hat.com, bp@...en8.de,
        dave.hansen@...ux.intel.com, hpa@...or.com, x86@...nel.org,
        pbonzini@...hat.com, paulmck@...nel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        rcu@...r.kernel.org, mimoja@...oja.de, hewenliang4@...wei.com,
        thomas.lendacky@....com, seanjc@...gle.com, pmenzel@...gen.mpg.de,
        fam.zheng@...edance.com, punit.agrawal@...edance.com,
        simon.evans@...edance.com, liangma@...ngbit.com
Subject: Re: [PATCH v12 06/11] x86/smpboot: Remove initial_stack on 64-bit



On 28 February 2023 20:17:19 GMT, Thomas Gleixner <tglx@...utronix.de> wrote:
>On Tue, Feb 28 2023 at 17:09, David Woodhouse wrote:
>> On Tue, 2023-02-28 at 17:13 +0100, Thomas Gleixner wrote:
>>> As this patch is now part of the parallel boot series and actually
>>> introduces smpboot_control, the above is neither accurate nor useful.
>>
>> Better commit message, add a comment where we abuse current->thread.sp
>> in the sleep path. Didn't remove the {} which would be added back in
>> the very next patch. Pushed to my tree for Usama's next round.
>
>Ok.
>
>> However, we start by introducing one more: smpboot_control. For now this
>
>s/we// :)

Yeah, actually spotted that one as I hit send and it's different in the git tree already.


>> merely holds the CPU# of the CPU which is coming up. That CPU can then
>> find its own per-cpu data, and everything else it needs can be found from
>> there, allowing the other global variables to be removed.
>>
>> First to be removed is initial_stack. Each CPU can load %rsp from its
>> current_task->thread.sp instead. That is already set up with the correct
>> idle thread for APs. Set up the .sp field in INIT_THREAD on x86 so that
>> the BSP also finds a suitable stack pointer in the static per-cpu data
>> when coming up on first boot.
>>
>> On resume from S3, the CPU needs a temporary stack because its idle task
>> is already active. Instead of setting initial_stack, the sleep code can
>> simply set its own current->thread.sp to point to the temporary stack.
>> The true stack pointer will get restored with the rest of the CPU
>> context in do_suspend_lowlevel().
>
>Thanks for writing this up!
>
>> +	/*
>> +	 * As each CPU starts up, it will find its own stack pointer
>> +	 * from its current_task->thread.sp. Typically that will be
>> +	 * the idle thread for a newly-started AP, or even the boot
>> +	 * CPU which will find it set to &init_task in the static
>> +	 * per-cpu data.
>> +	 *
>> +	 * Make the resuming CPU use the temporary stack at startup
>> +	 * by setting current->thread.sp to point to that. The true
>> +	 * %rsp will be restored with the rest of the CPU context,
>> +	 * by do_suspend_lowlevel().
>
>Right, but what restores current->thread.sp? thread.sp is used by
>unwinders...

Unwinding a thread that is actually *on* the CPU? By the time it's taken off, won't ->thread.sp have been written out again? I figured it was just a dead variable while the actual %rsp was in use?