On Mon, 2023-12-25 at 11:05 +0300, Kirill A. Shutemov wrote:
> If the helper is defined, it is called instead of halt() to stop the CPU
> at the end of stop_this_cpu() and on crash CPU shutdown.
>
> ACPI MADT will use it to hand over the CPU to BIOS in order to be able
> to wake it up again after kexec.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> ---
>
> v5.1:
> - Fix build for !SMP;
>
> ---
> arch/x86/include/asm/smp.h | 1 +
> arch/x86/kernel/process.c | 7 +++++++
> arch/x86/kernel/reboot.c | 12 ++++++++----
> 3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> index 4fab2ed454f3..390d53fd34f9 100644
> --- a/arch/x86/include/asm/smp.h
> +++ b/arch/x86/include/asm/smp.h
> @@ -38,6 +38,7 @@ struct smp_ops {
> int (*cpu_disable)(void);
> void (*cpu_die)(unsigned int cpu);
> void (*play_dead)(void);
> + void (*stop_this_cpu)(void);
>
> void (*send_call_func_ipi)(const struct cpumask *mask);
> void (*send_call_func_single_ipi)(int cpu);
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index b6f4e8399fca..ea4c812c7bf3 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -835,6 +835,13 @@ void __noreturn stop_this_cpu(void *dummy)
> */
> cpumask_clear_cpu(cpu, &cpus_stop_mask);
>
> +#ifdef CONFIG_SMP
> + if (smp_ops.stop_this_cpu) {
> + smp_ops.stop_this_cpu();
> + unreachable();
> + }
> +#endif
If I read correctly this will result in stop_this_cpu() having different
behaviour for SMP and !SMP build for TDX guest. For example, AFAICT
machine_halt() also calls stop_this_cpu() on local cpu after it stops other
cpus. So for the local cpu, in SMP build it will calls into BIOS's reset vector
but in !SMP it will call native_halt().
> +
> for (;;) {
> /*
> * Use native_halt() so that memory contents don't change
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index 16dde83df49a..738b3e810196 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -881,10 +881,14 @@ static int crash_nmi_callback(unsigned int val, struct pt_regs *regs)
> cpu_emergency_disable_virtualization();
>
> atomic_dec(&waiting_for_crash_ipi);
> - /* Assume hlt works */
> - halt();
> - for (;;)
> - cpu_relax();
> +
> + if (smp_ops.stop_this_cpu) {
> + smp_ops.stop_this_cpu();
Could you explain why unreachable() is called in stop_this_cpu() but not here?
> + } else {
> + halt();
> + for (;;)
> + cpu_relax();
> + }
Similar to stop_this_cpu(), if you also call unreachable() here, then I think
you can remove the 'else' here but directly calls halt() + cpu_relax() loop.
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
