2021-01-16 22:35:19

by Dexuan Cui

[permalink] [raw]
Subject: [PATCH] x86/hyperv: Initialize clockevents after LAPIC is initialized

With commit 4df4cb9e99f8, the Hyper-V direct-mode STIMER is actually
initialized before LAPIC is initialized: see

apic_intr_mode_init()

x86_platform.apic_post_init()
hyperv_init()
hv_stimer_alloc()

apic_bsp_setup()
setup_local_APIC()

setup_local_APIC() temporarily disables LAPIC, initializes it and
re-eanble it. The direct-mode STIMER depends on LAPIC, and when it's
registered, it can be programmed immediately and the timer can fire
very soon:

hv_stimer_init
clockevents_config_and_register
clockevents_register_device
tick_check_new_device
tick_setup_device
tick_setup_periodic(), tick_setup_oneshot()
clockevents_program_event

When the timer fires in the hypervisor, if the LAPIC is in the
disabled state, new versions of Hyper-V ignore the event and don't inject
the timer interrupt into the VM, and hence the VM hangs when it boots.

Note: when the VM starts/reboots, the LAPIC is pre-enabled by the
firmware, so the window of LAPIC being temporarily disabled is pretty
small, and the issue can only happen once out of 100~200 reboots for
a 40-vCPU VM on one dev host, and on another host the issue doesn't
reproduce after 2000 reboots.

The issue is more noticeable for kdump/kexec, because the LAPIC is
disabled by the first kernel, and stays disabled until the kdump/kexec
kernel enables it. This is especially an issue to a Generation-2 VM
(for which Hyper-V doesn't emulate the PIT timer) when CONFIG_HZ=1000
(rather than CONFIG_HZ=250) is used.

Fix the issue by moving hv_stimer_alloc() to a later place where the
LAPIC timer is initialized.

Fixes: 4df4cb9e99f8 ("x86/hyperv: Initialize clockevents earlier in CPU onlining")
Signed-off-by: Dexuan Cui <[email protected]>
---
arch/x86/hyperv/hv_init.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 4638a52d8eae..6375967a8244 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -315,6 +315,25 @@ static struct syscore_ops hv_syscore_ops = {
.resume = hv_resume,
};

+static void (* __initdata old_setup_percpu_clockev)(void);
+
+static void __init hv_stimer_setup_percpu_clockev(void)
+{
+ /*
+ * Ignore any errors in setting up stimer clockevents
+ * as we can run with the LAPIC timer as a fallback.
+ */
+ (void)hv_stimer_alloc();
+
+ /*
+ * Still register the LAPIC timer, because the direct-mode STIMER is
+ * not supported by old versions of Hyper-V. This also allows users
+ * to switch to LAPIC timer via /sys, if they want to.
+ */
+ if (old_setup_percpu_clockev)
+ old_setup_percpu_clockev();
+}
+
/*
* This function is to be invoked early in the boot sequence after the
* hypervisor has been detected.
@@ -393,10 +412,14 @@ void __init hyperv_init(void)
wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);

/*
- * Ignore any errors in setting up stimer clockevents
- * as we can run with the LAPIC timer as a fallback.
+ * hyperv_init() is called before LAPIC is initialized: see
+ * apic_intr_mode_init() -> x86_platform.apic_post_init() and
+ * apic_bsp_setup() -> setup_local_APIC(). The direct-mode STIMER
+ * depends on LAPIC, so hv_stimer_alloc() should be called from
+ * x86_init.timers.setup_percpu_clockev.
*/
- (void)hv_stimer_alloc();
+ old_setup_percpu_clockev = x86_init.timers.setup_percpu_clockev;
+ x86_init.timers.setup_percpu_clockev = hv_stimer_setup_percpu_clockev;

hv_apic_init();

--
2.19.1


2021-01-16 22:51:48

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH] x86/hyperv: Initialize clockevents after LAPIC is initialized

From: Dexuan Cui <[email protected]> Sent: Saturday, January 16, 2021 2:32 PM
>
> With commit 4df4cb9e99f8, the Hyper-V direct-mode STIMER is actually
> initialized before LAPIC is initialized: see
>
> apic_intr_mode_init()
>
> x86_platform.apic_post_init()
> hyperv_init()
> hv_stimer_alloc()
>
> apic_bsp_setup()
> setup_local_APIC()
>
> setup_local_APIC() temporarily disables LAPIC, initializes it and
> re-eanble it. The direct-mode STIMER depends on LAPIC, and when it's
> registered, it can be programmed immediately and the timer can fire
> very soon:
>
> hv_stimer_init
> clockevents_config_and_register
> clockevents_register_device
> tick_check_new_device
> tick_setup_device
> tick_setup_periodic(), tick_setup_oneshot()
> clockevents_program_event
>
> When the timer fires in the hypervisor, if the LAPIC is in the
> disabled state, new versions of Hyper-V ignore the event and don't inject
> the timer interrupt into the VM, and hence the VM hangs when it boots.
>
> Note: when the VM starts/reboots, the LAPIC is pre-enabled by the
> firmware, so the window of LAPIC being temporarily disabled is pretty
> small, and the issue can only happen once out of 100~200 reboots for
> a 40-vCPU VM on one dev host, and on another host the issue doesn't
> reproduce after 2000 reboots.
>
> The issue is more noticeable for kdump/kexec, because the LAPIC is
> disabled by the first kernel, and stays disabled until the kdump/kexec
> kernel enables it. This is especially an issue to a Generation-2 VM
> (for which Hyper-V doesn't emulate the PIT timer) when CONFIG_HZ=1000
> (rather than CONFIG_HZ=250) is used.
>
> Fix the issue by moving hv_stimer_alloc() to a later place where the
> LAPIC timer is initialized.
>
> Fixes: 4df4cb9e99f8 ("x86/hyperv: Initialize clockevents earlier in CPU onlining")
> Signed-off-by: Dexuan Cui <[email protected]>
> ---
> arch/x86/hyperv/hv_init.c | 29 ++++++++++++++++++++++++++---
> 1 file changed, 26 insertions(+), 3 deletions(-)
>

Reviewed-by: Michael Kelley <[email protected]>

2021-01-17 15:27:11

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH] x86/hyperv: Initialize clockevents after LAPIC is initialized

On Sat, Jan 16, 2021 at 10:48:04PM +0000, Michael Kelley wrote:
> From: Dexuan Cui <[email protected]> Sent: Saturday, January 16, 2021 2:32 PM
> >
> > Fixes: 4df4cb9e99f8 ("x86/hyperv: Initialize clockevents earlier in CPU onlining")
> > Signed-off-by: Dexuan Cui <[email protected]>
> > ---
> > arch/x86/hyperv/hv_init.c | 29 ++++++++++++++++++++++++++---
> > 1 file changed, 26 insertions(+), 3 deletions(-)
> >
>
> Reviewed-by: Michael Kelley <[email protected]>

Applied to hyperv-fixes. Thanks.

>