From 336f560a8917fd60bcdb71b97263257611411453 Mon Sep 17 00:00:00 2001 From: Juergen Gross Date: Fri, 25 Nov 2022 07:32:47 +0100 Subject: x86/xen: don't let xen_pv_play_dead() return A function called via the paravirt play_dead() hook should not return to the caller. xen_pv_play_dead() has a problem in this regard, as it currently will return in case an offlined cpu is brought to life again. This can be changed only by doing basically a longjmp() to cpu_bringup_and_idle(), as the hypercall for bringing down the cpu will just return when the cpu is coming up again. Just re-initializing the cpu isn't possible, as the Xen hypervisor will deny that operation. So introduce xen_cpu_bringup_again() resetting the stack and calling cpu_bringup_and_idle(), which can be called after HYPERVISOR_vcpu_op() in xen_pv_play_dead(). Signed-off-by: Juergen Gross Reviewed-by: Boris Ostrovsky Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20221125063248.30256-2-jgross@suse.com Signed-off-by: Juergen Gross --- arch/x86/xen/smp.h | 2 ++ arch/x86/xen/smp_pv.c | 13 ++----------- arch/x86/xen/xen-head.S | 7 +++++++ 3 files changed, 11 insertions(+), 11 deletions(-) (limited to 'arch') diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h index bd02f9d50107..6e90a312067b 100644 --- a/arch/x86/xen/smp.h +++ b/arch/x86/xen/smp.h @@ -21,6 +21,8 @@ void xen_smp_send_reschedule(int cpu); void xen_smp_send_call_function_ipi(const struct cpumask *mask); void xen_smp_send_call_function_single_ipi(int cpu); +void xen_cpu_bringup_again(unsigned long stack); + struct xen_common_irq { int irq; char *name; diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c index 6175f2c5c822..a97b0505a483 100644 --- a/arch/x86/xen/smp_pv.c +++ b/arch/x86/xen/smp_pv.c @@ -385,17 +385,8 @@ static void xen_pv_play_dead(void) /* used only with HOTPLUG_CPU */ { play_dead_common(); HYPERVISOR_vcpu_op(VCPUOP_down, xen_vcpu_nr(smp_processor_id()), NULL); - cpu_bringup(); - /* - * commit 4b0c0f294 (tick: Cleanup NOHZ per cpu data on cpu down) - * clears certain data that the cpu_idle loop (which called us - * and that we return from) expects. The only way to get that - * data back is to call: - */ - tick_nohz_idle_enter(); - tick_nohz_idle_stop_tick_protected(); - - cpuhp_online_idle(CPUHP_AP_ONLINE_IDLE); + xen_cpu_bringup_again((unsigned long)task_pt_regs(current)); + BUG(); } #else /* !CONFIG_HOTPLUG_CPU */ diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S index ffaa62167f6e..e36ea4268bd2 100644 --- a/arch/x86/xen/xen-head.S +++ b/arch/x86/xen/xen-head.S @@ -76,6 +76,13 @@ SYM_CODE_START(asm_cpu_bringup_and_idle) call cpu_bringup_and_idle SYM_CODE_END(asm_cpu_bringup_and_idle) + +SYM_CODE_START(xen_cpu_bringup_again) + UNWIND_HINT_FUNC + mov %rdi, %rsp + UNWIND_HINT_REGS + call cpu_bringup_and_idle +SYM_CODE_END(xen_cpu_bringup_again) .popsection #endif #endif -- cgit v1.2.3 From f697cb00afa90b68748f246540270b5865c801ba Mon Sep 17 00:00:00 2001 From: Juergen Gross Date: Fri, 25 Nov 2022 07:32:48 +0100 Subject: x86/xen: mark xen_pv_play_dead() as __noreturn Mark xen_pv_play_dead() and related to that xen_cpu_bringup_again() as "__noreturn". Signed-off-by: Juergen Gross Reviewed-by: Boris Ostrovsky Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20221125063248.30256-3-jgross@suse.com Signed-off-by: Juergen Gross --- arch/x86/xen/smp.h | 2 +- arch/x86/xen/smp_pv.c | 4 ++-- tools/objtool/check.c | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) (limited to 'arch') diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h index 6e90a312067b..22fb982ff971 100644 --- a/arch/x86/xen/smp.h +++ b/arch/x86/xen/smp.h @@ -21,7 +21,7 @@ void xen_smp_send_reschedule(int cpu); void xen_smp_send_call_function_ipi(const struct cpumask *mask); void xen_smp_send_call_function_single_ipi(int cpu); -void xen_cpu_bringup_again(unsigned long stack); +void __noreturn xen_cpu_bringup_again(unsigned long stack); struct xen_common_irq { int irq; diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c index a97b0505a483..a9cf8c8fa074 100644 --- a/arch/x86/xen/smp_pv.c +++ b/arch/x86/xen/smp_pv.c @@ -381,7 +381,7 @@ static void xen_pv_cpu_die(unsigned int cpu) } } -static void xen_pv_play_dead(void) /* used only with HOTPLUG_CPU */ +static void __noreturn xen_pv_play_dead(void) /* used only with HOTPLUG_CPU */ { play_dead_common(); HYPERVISOR_vcpu_op(VCPUOP_down, xen_vcpu_nr(smp_processor_id()), NULL); @@ -400,7 +400,7 @@ static void xen_pv_cpu_die(unsigned int cpu) BUG(); } -static void xen_pv_play_dead(void) +static void __noreturn xen_pv_play_dead(void) { BUG(); } diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 4b7c8b33069e..68e87b45caef 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -186,6 +186,7 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func, "snp_abort", "stop_this_cpu", "usercopy_abort", + "xen_cpu_bringup_again", "xen_start_kernel", }; -- cgit v1.2.3 From caea091e48ed9d3951506507abf26e9918d08e35 Mon Sep 17 00:00:00 2001 From: Krister Johansen Date: Fri, 16 Dec 2022 08:21:18 -0800 Subject: x86/xen/time: prefer tsc as clocksource when it is invariant Kvm elects to use tsc instead of kvm-clock when it can detect that the TSC is invariant. (As of commit 7539b174aef4 ("x86: kvmguest: use TSC clocksource if invariant TSC is exposed")). Notable cloud vendors[1] and performance engineers[2] recommend that Xen users preferentially select tsc over xen-clocksource due the performance penalty incurred by the latter. These articles are persuasive and tailored to specific use cases. In order to understand the tradeoffs around this choice more fully, this author had to reference the documented[3] complexities around the Xen configuration, as well as the kernel's clocksource selection algorithm. Many users may not attempt this to correctly configure the right clock source in their guest. The approach taken in the kvm-clock module spares users this confusion, where possible. Both the Intel SDM[4] and the Xen tsc documentation explain that marking a tsc as invariant means that it should be considered stable by the OS and is elibile to be used as a wall clock source. In order to obtain better out-of-the-box performance, and reduce the need for user tuning, follow kvm's approach and decrease the xen clock rating so that tsc is preferable, if it is invariant, stable, and the tsc will never be emulated. [1] https://aws.amazon.com/premiumsupport/knowledge-center/manage-ec2-linux-clock-source/ [2] https://www.brendangregg.com/blog/2021-09-26/the-speed-of-time.html [3] https://xenbits.xen.org/docs/unstable/man/xen-tscmode.7.html [4] Intel 64 and IA-32 Architectures Sofware Developer's Manual Volume 3b: System Programming Guide, Part 2, Section 17.17.1, Invariant TSC Signed-off-by: Krister Johansen Code-reviewed-by: David Reaver Reviewed-by: Juergen Gross Link: https://lore.kernel.org/r/20221216162118.GB2633@templeofstupid.com Signed-off-by: Juergen Gross --- arch/x86/xen/time.c | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) (limited to 'arch') diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c index 9ef0a5cca96e..95140609c8a8 100644 --- a/arch/x86/xen/time.c +++ b/arch/x86/xen/time.c @@ -474,15 +474,51 @@ static void xen_setup_vsyscall_time_info(void) xen_clocksource.vdso_clock_mode = VDSO_CLOCKMODE_PVCLOCK; } +/* + * Check if it is possible to safely use the tsc as a clocksource. This is + * only true if the hypervisor notifies the guest that its tsc is invariant, + * the tsc is stable, and the tsc instruction will never be emulated. + */ +static int __init xen_tsc_safe_clocksource(void) +{ + u32 eax, ebx, ecx, edx; + + if (!(boot_cpu_has(X86_FEATURE_CONSTANT_TSC))) + return 0; + + if (!(boot_cpu_has(X86_FEATURE_NONSTOP_TSC))) + return 0; + + if (check_tsc_unstable()) + return 0; + + /* Leaf 4, sub-leaf 0 (0x40000x03) */ + cpuid_count(xen_cpuid_base() + 3, 0, &eax, &ebx, &ecx, &edx); + + /* tsc_mode = no_emulate (2) */ + if (ebx != 2) + return 0; + + return 1; +} + static void __init xen_time_init(void) { struct pvclock_vcpu_time_info *pvti; int cpu = smp_processor_id(); struct timespec64 tp; - /* As Dom0 is never moved, no penalty on using TSC there */ + /* + * As Dom0 is never moved, no penalty on using TSC there. + * + * If it is possible for the guest to determine that the tsc is a safe + * clocksource, then set xen_clocksource rating below that of the tsc + * so that the system prefers tsc instead. + */ if (xen_initial_domain()) xen_clocksource.rating = 275; + else if (xen_tsc_safe_clocksource()) + xen_clocksource.rating = 299; clocksource_register_hz(&xen_clocksource, NSEC_PER_SEC); -- cgit v1.2.3 From 20e7da1bbba8474839157fb28fb538d2c058128d Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Wed, 15 Feb 2023 12:27:52 +0100 Subject: x86/Xen: drop leftover VM-assist uses Both the 4Gb-segments and the PAE-extended-CR3 one are applicable to 32-bit guests only. The PAE-extended-CR3 use, furthermore, was redundant with the PAE_MODE ELF note anyway. Signed-off-by: Jan Beulich Reviewed-by: Juergen Gross Link: https://lore.kernel.org/r/215515af-cfb9-3237-03ba-3312e3fa0d34@suse.com Signed-off-by: Juergen Gross --- arch/x86/xen/setup.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'arch') diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c index 8db26f10fb1d..c2be3efb2ba0 100644 --- a/arch/x86/xen/setup.c +++ b/arch/x86/xen/setup.c @@ -934,12 +934,8 @@ void xen_enable_syscall(void) static void __init xen_pvmmu_arch_setup(void) { - HYPERVISOR_vm_assist(VMASST_CMD_enable, VMASST_TYPE_4gb_segments); HYPERVISOR_vm_assist(VMASST_CMD_enable, VMASST_TYPE_writable_pagetables); - HYPERVISOR_vm_assist(VMASST_CMD_enable, - VMASST_TYPE_pae_extended_cr3); - if (register_callback(CALLBACKTYPE_event, xen_asm_exc_xen_hypervisor_callback) || register_callback(CALLBACKTYPE_failsafe, xen_failsafe_callback)) -- cgit v1.2.3