From:   Thomas Gleixner <tglx@linutronix.de>
To:     David Woodhouse <dwmw2@infradead.org>,
        Usama Arif <usama.arif@bytedance.com>, kim.phillips@amd.com,
        brgerst@gmail.com
Cc:     piotrgorski@cachyos.org, oleksandr@natalenko.name,
        arjan@linux.intel.com, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, hpa@zytor.com, x86@kernel.org,
        pbonzini@redhat.com, paulmck@kernel.org,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        rcu@vger.kernel.org, mimoja@mimoja.de, hewenliang4@huawei.com,
        thomas.lendacky@amd.com, seanjc@google.com, pmenzel@molgen.mpg.de,
        fam.zheng@bytedance.com, punit.agrawal@bytedance.com,
        simon.evans@bytedance.com, liangma@liangbit.com
Subject: Re: [PATCH v12 06/11] x86/smpboot: Remove initial_stack on 64-bit
In-Reply-To: <c6863590f5fbf139f6aec50d0f3bc8e8b00cfcaf.camel@infradead.org>
References: <20230226110802.103134-1-usama.arif@bytedance.com>
 <20230226110802.103134-7-usama.arif@bytedance.com> <87k001n4xo.ffs@tglx>
 <c6863590f5fbf139f6aec50d0f3bc8e8b00cfcaf.camel@infradead.org>
Date:   Tue, 28 Feb 2023 21:17:19 +0100
Message-ID: <87edq9mto0.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

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// :)

> 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...

Thanks,

        tglx