Anirudh Rayabharam <anir...@anirudhrb.com> writes:
> From: Anirudh Rayabharam (Microsoft) <anir...@anirudhrb.com>
>
> commit 9636be85cc5b ("x86/hyperv: Fix hyperv_pcpu_input_arg handling when
> CPUs go online/offline") introduces a new cpuhp state for hyperv
> initialization.
>
> cpuhp_setup_state() returns the state number if state is
> CPUHP_AP_ONLINE_DYN or CPUHP_BP_PREPARE_DYN and 0 for all other states.
> For the hyperv case, since a new cpuhp state was introduced it would
> return 0. However, in hv_machine_shutdown(), the cpuhp_remove_state() call
> is conditioned upon "hyperv_init_cpuhp > 0". This will never be true and
> so hv_cpu_die() won't be called on all CPUs. This means the VP assist page
> won't be reset. When the kexec kernel tries to setup the VP assist page
> again, the hypervisor corrupts the memory region of the old VP assist page
> causing a panic in case the kexec kernel is using that memory elsewhere.
> This was originally fixed in commit dfe94d4086e4 ("x86/hyperv: Fix kexec
> panic/hang issues").
>
> Get rid of hyperv_init_cpuhp entirely since we are no longer using a
> dynamic cpuhp state and use CPUHP_AP_HYPERV_ONLINE directly with
> cpuhp_remove_state().
>
> Cc: sta...@vger.kernel.org
> Fixes: 9636be85cc5b ("x86/hyperv: Fix hyperv_pcpu_input_arg handling when
> CPUs go online/offline")
> Signed-off-by: Anirudh Rayabharam (Microsoft) <anir...@anirudhrb.com>
> ---
>
> v1->v2:
> - Remove hyperv_init_cpuhp entirely and use CPUHP_AP_HYPERV_ONLINE directly
> with cpuhp_remove_state().
Reviewed-by: Vitaly Kuznetsov <vkuzn...@redhat.com>
Thanks!
>
> v1:
> https://lore.kernel.org/linux-hyperv/87wmk2xt5i....@redhat.com/T/#m54b8ae17e98d65e77a09002e478669d15d9830d0
>
> ---
> arch/x86/hyperv/hv_init.c | 5 +----
> arch/x86/include/asm/mshyperv.h | 1 -
> arch/x86/kernel/cpu/mshyperv.c | 4 ++--
> 3 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 17a71e92a343..95eada2994e1 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -35,7 +35,6 @@
> #include <clocksource/hyperv_timer.h>
> #include <linux/highmem.h>
>
> -int hyperv_init_cpuhp;
> u64 hv_current_partition_id = ~0ull;
> EXPORT_SYMBOL_GPL(hv_current_partition_id);
>
> @@ -607,8 +606,6 @@ void __init hyperv_init(void)
>
> register_syscore_ops(&hv_syscore_ops);
>
> - hyperv_init_cpuhp = cpuhp;
> -
> if (cpuid_ebx(HYPERV_CPUID_FEATURES) & HV_ACCESS_PARTITION_ID)
> hv_get_partition_id();
>
> @@ -637,7 +634,7 @@ void __init hyperv_init(void)
> clean_guest_os_id:
> wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
> hv_ivm_msr_write(HV_X64_MSR_GUEST_OS_ID, 0);
> - cpuhp_remove_state(cpuhp);
> + cpuhp_remove_state(CPUHP_AP_HYPERV_ONLINE);
> free_ghcb_page:
> free_percpu(hv_ghcb_pg);
> free_vp_assist_page:
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 390c4d13956d..5f0bc6a6d025 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -40,7 +40,6 @@ static inline unsigned char hv_get_nmi_reason(void)
> }
>
> #if IS_ENABLED(CONFIG_HYPERV)
> -extern int hyperv_init_cpuhp;
> extern bool hyperv_paravisor_present;
>
> extern void *hv_hypercall_pg;
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index e0fd57a8ba84..e98db51f25ba 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -199,8 +199,8 @@ static void hv_machine_shutdown(void)
> * Call hv_cpu_die() on all the CPUs, otherwise later the hypervisor
> * corrupts the old VP Assist Pages and can crash the kexec kernel.
> */
> - if (kexec_in_progress && hyperv_init_cpuhp > 0)
> - cpuhp_remove_state(hyperv_init_cpuhp);
> + if (kexec_in_progress)
> + cpuhp_remove_state(CPUHP_AP_HYPERV_ONLINE);
>
> /* The function calls stop_other_cpus(). */
> native_machine_shutdown();
--
Vitaly
