On Tue, Oct 12, 2021 at 05:50:05PM +0200, Vitaly Kuznetsov wrote:
> The following issue is observed with CONFIG_DEBUG_PREEMPT when KVM loads:
>
> KVM: vmx: using Hyper-V Enlightened VMCS
> BUG: using smp_processor_id() in preemptible [00000000] code: systemd-udevd/488
> caller is set_hv_tscchange_cb+0x16/0x80
> CPU: 1 PID: 488 Comm: systemd-udevd Not tainted 5.15.0-rc5+ #396
> Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v4.0 12/17/2019
> Call Trace:
> dump_stack_lvl+0x6a/0x9a
> check_preemption_disabled+0xde/0xe0
> ? kvm_gen_update_masterclock+0xd0/0xd0 [kvm]
> set_hv_tscchange_cb+0x16/0x80
> kvm_arch_init+0x23f/0x290 [kvm]
> kvm_init+0x30/0x310 [kvm]
> vmx_init+0xaf/0x134 [kvm_intel]
> ...
>
> set_hv_tscchange_cb() can get preempted in between acquiring
> smp_processor_id() and writing to HV_X64_MSR_REENLIGHTENMENT_CONTROL. This
> is not an issue by itself: HV_X64_MSR_REENLIGHTENMENT_CONTROL is a
> partition-wide MSR and it doesn't matter which particular CPU will be
> used to receive reenlightenment notifications. The only real problem can
> (in theory) be observed if the CPU whose id was acquired with
> smp_processor_id() goes offline before we manage to write to the MSR,
> the logic in hv_cpu_die() won't be able to reassign it correctly.
>
> Reported-by: Michael Kelley <mikelley@xxxxxxxxxxxxx>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
Applied to hyperv-next.
> ---
> arch/x86/hyperv/hv_init.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 708a2712a516..179fc173104d 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -139,7 +139,6 @@ void set_hv_tscchange_cb(void (*cb)(void))
> struct hv_reenlightenment_control re_ctrl = {
> .vector = HYPERV_REENLIGHTENMENT_VECTOR,
> .enabled = 1,
> - .target_vp = hv_vp_index[smp_processor_id()]
> };
> struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1};
>
> @@ -153,8 +152,12 @@ void set_hv_tscchange_cb(void (*cb)(void))
> /* Make sure callback is registered before we write to MSRs */
> wmb();
>
> + re_ctrl.target_vp = hv_vp_index[get_cpu()];
> +
> wrmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *((u64 *)&re_ctrl));
> wrmsrl(HV_X64_MSR_TSC_EMULATION_CONTROL, *((u64 *)&emu_ctrl));
> +
> + put_cpu();
> }
> EXPORT_SYMBOL_GPL(set_hv_tscchange_cb);
>
> --
> 2.31.1
>
