* [PATCH 0/2] x86/hyperv: Bug fix and what I hope is an enhancement @ 2021-10-28 22:21 Sean Christopherson 2021-10-28 22:21 ` [PATCH 1/2] x86/hyperv: Fix NULL deref in set_hv_tscchange_cb() if Hyper-V setup fails Sean Christopherson 2021-10-28 22:21 ` [PATCH 2/2] x86/hyperv: Move required MSRs check to initial platform probing Sean Christopherson 0 siblings, 2 replies; 10+ messages in thread From: Sean Christopherson @ 2021-10-28 22:21 UTC (permalink / raw) To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu, Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86 Cc: H. Peter Anvin, linux-hyperv, linux-kernel, Vitaly Kuznetsov, Sean Christopherson Patch 01 is a fix for a NULL pointer deref that I ran into with a bad VMM configuration. Patch 02 effectively makes the required MSRs mandatory for recognizing Hyper-V at all. I'm not confident this is truly desirable, e.g. there might be some features that are still kinda sorta usable, but on the other hand there's a large pile of features that end up being a waste of cycles to worm their way back to the native ops. QEMU 5.1 (and other versions) makes it all too easy to advertise Hyper-V and a slew of features without advertising the Hyper-V HYPERCALL MSR, e.g. forcing QEMU features +hv-ipi,+hv-tlbflush,+hv-vpindex,+hv-reenlightenment advertises a bunch of things, but not the HYPERCALL MSR. That results in the guest identifying Hyper-V and setting a variety of PV ops that then get ignored because hyperv_init() silently disables Hyper-V for all intents and purposes. The VMM (or its controller) is obviously off in the weeds, but ideally the guest kernel would acknowledge the bad setup in some way. Sean Christopherson (2): x86/hyperv: Fix NULL deref in set_hv_tscchange_cb() if Hyper-V setup fails x86/hyperv: Move required MSRs check to initial platform probing arch/x86/hyperv/hv_init.c | 16 +++++++--------- arch/x86/kernel/cpu/mshyperv.c | 20 +++++++++++++++----- 2 files changed, 22 insertions(+), 14 deletions(-) -- 2.33.0.1079.g6e70778dc9-goog ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] x86/hyperv: Fix NULL deref in set_hv_tscchange_cb() if Hyper-V setup fails 2021-10-28 22:21 [PATCH 0/2] x86/hyperv: Bug fix and what I hope is an enhancement Sean Christopherson @ 2021-10-28 22:21 ` Sean Christopherson 2021-10-29 9:14 ` Vitaly Kuznetsov 2021-10-28 22:21 ` [PATCH 2/2] x86/hyperv: Move required MSRs check to initial platform probing Sean Christopherson 1 sibling, 1 reply; 10+ messages in thread From: Sean Christopherson @ 2021-10-28 22:21 UTC (permalink / raw) To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu, Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86 Cc: H. Peter Anvin, linux-hyperv, linux-kernel, Vitaly Kuznetsov, Sean Christopherson Check for re-enlightenment support and for a valid hv_vp_index array prior to derefencing hv_vp_index when setting Hyper-V's TSC change callback. If Hyper-V setup failed in hyperv_init(), e.g. because of a bad VMM config that doesn't advertise the HYPERCALL MSR, the kernel will still report that it's running under Hyper-V, but will have silently disabled nearly all functionality. BUG: kernel NULL pointer dereference, address: 0000000000000010 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 0 P4D 0 Oops: 0000 [#1] SMP CPU: 4 PID: 1 Comm: swapper/0 Not tainted 5.15.0-rc2+ #75 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 RIP: 0010:set_hv_tscchange_cb+0x15/0xa0 Code: <8b> 04 82 8b 15 12 17 85 01 48 c1 e0 20 48 0d ee 00 01 00 f6 c6 08 ... Call Trace: kvm_arch_init+0x17c/0x280 kvm_init+0x31/0x330 vmx_init+0xba/0x13a do_one_initcall+0x41/0x1c0 kernel_init_freeable+0x1f2/0x23b kernel_init+0x16/0x120 ret_from_fork+0x22/0x30 Fixes: 93286261de1b ("x86/hyperv: Reenlightenment notifications support") Cc: stable@vger.kernel.org Cc: Vitaly Kuznetsov <vkuznets@redhat.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/hyperv/hv_init.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index 708a2712a516..6cc845c026d4 100644 --- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c @@ -139,7 +139,7 @@ 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()] + .target_vp = -1, }; struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1}; @@ -148,6 +148,11 @@ void set_hv_tscchange_cb(void (*cb)(void)) return; } + if (!hv_vp_index) + return; + + re_ctrl.target_vp = hv_vp_index[smp_processor_id()]; + hv_reenlightenment_cb = cb; /* Make sure callback is registered before we write to MSRs */ -- 2.33.0.1079.g6e70778dc9-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] x86/hyperv: Fix NULL deref in set_hv_tscchange_cb() if Hyper-V setup fails 2021-10-28 22:21 ` [PATCH 1/2] x86/hyperv: Fix NULL deref in set_hv_tscchange_cb() if Hyper-V setup fails Sean Christopherson @ 2021-10-29 9:14 ` Vitaly Kuznetsov 2021-11-02 12:17 ` Wei Liu 0 siblings, 1 reply; 10+ messages in thread From: Vitaly Kuznetsov @ 2021-10-29 9:14 UTC (permalink / raw) To: Sean Christopherson Cc: H. Peter Anvin, linux-hyperv, linux-kernel, Sean Christopherson, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu, Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86 Sean Christopherson <seanjc@google.com> writes: > Check for re-enlightenment support and for a valid hv_vp_index array > prior to derefencing hv_vp_index when setting Hyper-V's TSC change > callback. If Hyper-V setup failed in hyperv_init(), e.g. because of a > bad VMM config that doesn't advertise the HYPERCALL MSR, the kernel will > still report that it's running under Hyper-V, but will have silently > disabled nearly all functionality. > > BUG: kernel NULL pointer dereference, address: 0000000000000010 > #PF: supervisor read access in kernel mode > #PF: error_code(0x0000) - not-present page > PGD 0 P4D 0 > Oops: 0000 [#1] SMP > CPU: 4 PID: 1 Comm: swapper/0 Not tainted 5.15.0-rc2+ #75 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 > RIP: 0010:set_hv_tscchange_cb+0x15/0xa0 > Code: <8b> 04 82 8b 15 12 17 85 01 48 c1 e0 20 48 0d ee 00 01 00 f6 c6 08 > ... > Call Trace: > kvm_arch_init+0x17c/0x280 > kvm_init+0x31/0x330 > vmx_init+0xba/0x13a > do_one_initcall+0x41/0x1c0 > kernel_init_freeable+0x1f2/0x23b > kernel_init+0x16/0x120 > ret_from_fork+0x22/0x30 > > Fixes: 93286261de1b ("x86/hyperv: Reenlightenment notifications support") > Cc: stable@vger.kernel.org > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/hyperv/hv_init.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > index 708a2712a516..6cc845c026d4 100644 > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -139,7 +139,7 @@ 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()] > + .target_vp = -1, > }; > struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1}; > > @@ -148,6 +148,11 @@ void set_hv_tscchange_cb(void (*cb)(void)) > return; > } > > + if (!hv_vp_index) > + return; > + > + re_ctrl.target_vp = hv_vp_index[smp_processor_id()]; > + > hv_reenlightenment_cb = cb; > > /* Make sure callback is registered before we write to MSRs */ The patch looks good, however, it needs to be applied on top of the already merged: https://lore.kernel.org/linux-hyperv/20211012155005.1613352-1-vkuznets@redhat.com/ -- Vitaly ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] x86/hyperv: Fix NULL deref in set_hv_tscchange_cb() if Hyper-V setup fails 2021-10-29 9:14 ` Vitaly Kuznetsov @ 2021-11-02 12:17 ` Wei Liu 2021-11-02 14:31 ` Sean Christopherson 0 siblings, 1 reply; 10+ messages in thread From: Wei Liu @ 2021-11-02 12:17 UTC (permalink / raw) To: Vitaly Kuznetsov Cc: Sean Christopherson, H. Peter Anvin, linux-hyperv, linux-kernel, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu, Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86 On Fri, Oct 29, 2021 at 11:14:59AM +0200, Vitaly Kuznetsov wrote: > Sean Christopherson <seanjc@google.com> writes: > > > Check for re-enlightenment support and for a valid hv_vp_index array > > prior to derefencing hv_vp_index when setting Hyper-V's TSC change > > callback. If Hyper-V setup failed in hyperv_init(), e.g. because of a > > bad VMM config that doesn't advertise the HYPERCALL MSR, the kernel will > > still report that it's running under Hyper-V, but will have silently > > disabled nearly all functionality. > > > > BUG: kernel NULL pointer dereference, address: 0000000000000010 > > #PF: supervisor read access in kernel mode > > #PF: error_code(0x0000) - not-present page > > PGD 0 P4D 0 > > Oops: 0000 [#1] SMP > > CPU: 4 PID: 1 Comm: swapper/0 Not tainted 5.15.0-rc2+ #75 > > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 > > RIP: 0010:set_hv_tscchange_cb+0x15/0xa0 > > Code: <8b> 04 82 8b 15 12 17 85 01 48 c1 e0 20 48 0d ee 00 01 00 f6 c6 08 > > ... > > Call Trace: > > kvm_arch_init+0x17c/0x280 > > kvm_init+0x31/0x330 > > vmx_init+0xba/0x13a > > do_one_initcall+0x41/0x1c0 > > kernel_init_freeable+0x1f2/0x23b > > kernel_init+0x16/0x120 > > ret_from_fork+0x22/0x30 > > > > Fixes: 93286261de1b ("x86/hyperv: Reenlightenment notifications support") > > Cc: stable@vger.kernel.org > > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > --- > > arch/x86/hyperv/hv_init.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > > index 708a2712a516..6cc845c026d4 100644 > > --- a/arch/x86/hyperv/hv_init.c > > +++ b/arch/x86/hyperv/hv_init.c > > @@ -139,7 +139,7 @@ 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()] > > + .target_vp = -1, > > }; > > struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1}; > > > > @@ -148,6 +148,11 @@ void set_hv_tscchange_cb(void (*cb)(void)) > > return; > > } > > > > + if (!hv_vp_index) > > + return; > > + > > + re_ctrl.target_vp = hv_vp_index[smp_processor_id()]; > > + > > hv_reenlightenment_cb = cb; > > > > /* Make sure callback is registered before we write to MSRs */ > > The patch looks good, however, it needs to be applied on top of the > already merged: > > https://lore.kernel.org/linux-hyperv/20211012155005.1613352-1-vkuznets@redhat.com/ Sean, are you going to rebase? Wei. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] x86/hyperv: Fix NULL deref in set_hv_tscchange_cb() if Hyper-V setup fails 2021-11-02 12:17 ` Wei Liu @ 2021-11-02 14:31 ` Sean Christopherson 2021-11-04 12:10 ` Wei Liu 0 siblings, 1 reply; 10+ messages in thread From: Sean Christopherson @ 2021-11-02 14:31 UTC (permalink / raw) To: Wei Liu Cc: Vitaly Kuznetsov, H. Peter Anvin, linux-hyperv, linux-kernel, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86 On Tue, Nov 02, 2021, Wei Liu wrote: > On Fri, Oct 29, 2021 at 11:14:59AM +0200, Vitaly Kuznetsov wrote: > > Sean Christopherson <seanjc@google.com> writes: > > The patch looks good, however, it needs to be applied on top of the > > already merged: > > > > https://lore.kernel.org/linux-hyperv/20211012155005.1613352-1-vkuznets@redhat.com/ > > Sean, are you going to rebase? Yep, I'll rebase. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] x86/hyperv: Fix NULL deref in set_hv_tscchange_cb() if Hyper-V setup fails 2021-11-02 14:31 ` Sean Christopherson @ 2021-11-04 12:10 ` Wei Liu 2021-11-04 12:24 ` Wei Liu 0 siblings, 1 reply; 10+ messages in thread From: Wei Liu @ 2021-11-04 12:10 UTC (permalink / raw) To: Sean Christopherson Cc: Wei Liu, Vitaly Kuznetsov, H. Peter Anvin, linux-hyperv, linux-kernel, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86 On Tue, Nov 02, 2021 at 02:31:57PM +0000, Sean Christopherson wrote: > On Tue, Nov 02, 2021, Wei Liu wrote: > > On Fri, Oct 29, 2021 at 11:14:59AM +0200, Vitaly Kuznetsov wrote: > > > Sean Christopherson <seanjc@google.com> writes: > > > The patch looks good, however, it needs to be applied on top of the > > > already merged: > > > > > > https://lore.kernel.org/linux-hyperv/20211012155005.1613352-1-vkuznets@redhat.com/ > > > > Sean, are you going to rebase? > > Yep, I'll rebase. Thank you! Wei. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] x86/hyperv: Fix NULL deref in set_hv_tscchange_cb() if Hyper-V setup fails 2021-11-04 12:10 ` Wei Liu @ 2021-11-04 12:24 ` Wei Liu 0 siblings, 0 replies; 10+ messages in thread From: Wei Liu @ 2021-11-04 12:24 UTC (permalink / raw) To: Sean Christopherson Cc: Wei Liu, Vitaly Kuznetsov, H. Peter Anvin, linux-hyperv, linux-kernel, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86 On Thu, Nov 04, 2021 at 12:10:30PM +0000, Wei Liu wrote: > On Tue, Nov 02, 2021 at 02:31:57PM +0000, Sean Christopherson wrote: > > On Tue, Nov 02, 2021, Wei Liu wrote: > > > On Fri, Oct 29, 2021 at 11:14:59AM +0200, Vitaly Kuznetsov wrote: > > > > Sean Christopherson <seanjc@google.com> writes: > > > > The patch looks good, however, it needs to be applied on top of the > > > > already merged: > > > > > > > > https://lore.kernel.org/linux-hyperv/20211012155005.1613352-1-vkuznets@redhat.com/ > > > > > > Sean, are you going to rebase? > > > > Yep, I'll rebase. > > Thank you! One further note: Vitaly's patch just hit mainline two days ago, but v5.16-rc1 is not tagged yet. Feel free to base you new patches on hyperv-next or linux-next. I will handle the rest. Or you can wait until v5.16-rc1 is tagged and base your patches on that. Your call. Thanks, Wei. > > Wei. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] x86/hyperv: Move required MSRs check to initial platform probing 2021-10-28 22:21 [PATCH 0/2] x86/hyperv: Bug fix and what I hope is an enhancement Sean Christopherson 2021-10-28 22:21 ` [PATCH 1/2] x86/hyperv: Fix NULL deref in set_hv_tscchange_cb() if Hyper-V setup fails Sean Christopherson @ 2021-10-28 22:21 ` Sean Christopherson 2021-10-29 9:20 ` Vitaly Kuznetsov 1 sibling, 1 reply; 10+ messages in thread From: Sean Christopherson @ 2021-10-28 22:21 UTC (permalink / raw) To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu, Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86 Cc: H. Peter Anvin, linux-hyperv, linux-kernel, Vitaly Kuznetsov, Sean Christopherson Explicitly check for MSR_HYPERCALL and MSR_VP_INDEX support when probing for running as a Hyper-V guest instead of waiting until hyperv_init() to detect the bogus configuration. Add messages to give the admin a heads up that they are likely running on a broken virtual machine setup. At best, silently disabling Hyper-V is confusing and difficult to debug, e.g. the kernel _says_ it's using all these fancy Hyper-V features, but always falls back to the native versions. At worst, the half baked setup will crash/hang the kernel. Cc: Vitaly Kuznetsov <vkuznets@redhat.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/hyperv/hv_init.c | 9 +-------- arch/x86/kernel/cpu/mshyperv.c | 20 +++++++++++++++----- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index 6cc845c026d4..abfb09610e22 100644 --- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c @@ -347,20 +347,13 @@ static void __init hv_get_partition_id(void) */ void __init hyperv_init(void) { - u64 guest_id, required_msrs; + u64 guest_id; union hv_x64_msr_hypercall_contents hypercall_msr; int cpuhp; if (x86_hyper_type != X86_HYPER_MS_HYPERV) return; - /* Absolutely required MSRs */ - required_msrs = HV_MSR_HYPERCALL_AVAILABLE | - HV_MSR_VP_INDEX_AVAILABLE; - - if ((ms_hyperv.features & required_msrs) != required_msrs) - return; - if (hv_common_init()) return; diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c index e095c28d27ae..ef6316fef99f 100644 --- a/arch/x86/kernel/cpu/mshyperv.c +++ b/arch/x86/kernel/cpu/mshyperv.c @@ -163,12 +163,22 @@ static uint32_t __init ms_hyperv_platform(void) cpuid(HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS, &eax, &hyp_signature[0], &hyp_signature[1], &hyp_signature[2]); - if (eax >= HYPERV_CPUID_MIN && - eax <= HYPERV_CPUID_MAX && - !memcmp("Microsoft Hv", hyp_signature, 12)) - return HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS; + if (eax < HYPERV_CPUID_MIN || eax > HYPERV_CPUID_MAX || + memcmp("Microsoft Hv", hyp_signature, 12)) + return 0; - return 0; + /* HYPERCALL and VP_INDEX MSRs are mandatory for all features. */ + eax = cpuid_eax(HYPERV_CPUID_FEATURES); + if (!(eax & HV_MSR_HYPERCALL_AVAILABLE)) { + pr_warn("x86/hyperv: HYPERCALL MSR not available.\n"); + return 0; + } + if (!(eax & HV_MSR_VP_INDEX_AVAILABLE)) { + pr_warn("x86/hyperv: VP_INDEX MSR not available.\n"); + return 0; + } + + return HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS; } static unsigned char hv_get_nmi_reason(void) -- 2.33.0.1079.g6e70778dc9-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] x86/hyperv: Move required MSRs check to initial platform probing 2021-10-28 22:21 ` [PATCH 2/2] x86/hyperv: Move required MSRs check to initial platform probing Sean Christopherson @ 2021-10-29 9:20 ` Vitaly Kuznetsov 2021-11-04 12:13 ` Wei Liu 0 siblings, 1 reply; 10+ messages in thread From: Vitaly Kuznetsov @ 2021-10-29 9:20 UTC (permalink / raw) To: Sean Christopherson Cc: H. Peter Anvin, linux-hyperv, linux-kernel, Sean Christopherson, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu, Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86 Sean Christopherson <seanjc@google.com> writes: > Explicitly check for MSR_HYPERCALL and MSR_VP_INDEX support when probing > for running as a Hyper-V guest instead of waiting until hyperv_init() to > detect the bogus configuration. Add messages to give the admin a heads > up that they are likely running on a broken virtual machine setup. > > At best, silently disabling Hyper-V is confusing and difficult to debug, > e.g. the kernel _says_ it's using all these fancy Hyper-V features, but > always falls back to the native versions. At worst, the half baked setup > will crash/hang the kernel. > > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/hyperv/hv_init.c | 9 +-------- > arch/x86/kernel/cpu/mshyperv.c | 20 +++++++++++++++----- > 2 files changed, 16 insertions(+), 13 deletions(-) > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > index 6cc845c026d4..abfb09610e22 100644 > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -347,20 +347,13 @@ static void __init hv_get_partition_id(void) > */ > void __init hyperv_init(void) > { > - u64 guest_id, required_msrs; > + u64 guest_id; > union hv_x64_msr_hypercall_contents hypercall_msr; > int cpuhp; > > if (x86_hyper_type != X86_HYPER_MS_HYPERV) > return; > > - /* Absolutely required MSRs */ > - required_msrs = HV_MSR_HYPERCALL_AVAILABLE | > - HV_MSR_VP_INDEX_AVAILABLE; > - > - if ((ms_hyperv.features & required_msrs) != required_msrs) > - return; > - > if (hv_common_init()) > return; > > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c > index e095c28d27ae..ef6316fef99f 100644 > --- a/arch/x86/kernel/cpu/mshyperv.c > +++ b/arch/x86/kernel/cpu/mshyperv.c > @@ -163,12 +163,22 @@ static uint32_t __init ms_hyperv_platform(void) > cpuid(HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS, > &eax, &hyp_signature[0], &hyp_signature[1], &hyp_signature[2]); > > - if (eax >= HYPERV_CPUID_MIN && > - eax <= HYPERV_CPUID_MAX && > - !memcmp("Microsoft Hv", hyp_signature, 12)) > - return HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS; > + if (eax < HYPERV_CPUID_MIN || eax > HYPERV_CPUID_MAX || > + memcmp("Microsoft Hv", hyp_signature, 12)) > + return 0; > > - return 0; > + /* HYPERCALL and VP_INDEX MSRs are mandatory for all features. */ > + eax = cpuid_eax(HYPERV_CPUID_FEATURES); > + if (!(eax & HV_MSR_HYPERCALL_AVAILABLE)) { > + pr_warn("x86/hyperv: HYPERCALL MSR not available.\n"); > + return 0; > + } > + if (!(eax & HV_MSR_VP_INDEX_AVAILABLE)) { > + pr_warn("x86/hyperv: VP_INDEX MSR not available.\n"); > + return 0; > + } > + > + return HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS; > } > > static unsigned char hv_get_nmi_reason(void) In theory, we can get away without VP_INDEX MSR as e.g. PV spinlocks don't need it but it will require us to add the check to all other features which actually need it and disable them so it's probably not worth the effort (and unless we're running on KVM/QEMU which actually *can* create such configuration, it's likely impossible to meet such setup in real life). Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> -- Vitaly ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] x86/hyperv: Move required MSRs check to initial platform probing 2021-10-29 9:20 ` Vitaly Kuznetsov @ 2021-11-04 12:13 ` Wei Liu 0 siblings, 0 replies; 10+ messages in thread From: Wei Liu @ 2021-11-04 12:13 UTC (permalink / raw) To: Vitaly Kuznetsov Cc: Sean Christopherson, H. Peter Anvin, linux-hyperv, linux-kernel, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu, Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86 On Fri, Oct 29, 2021 at 11:20:49AM +0200, Vitaly Kuznetsov wrote: > Sean Christopherson <seanjc@google.com> writes: > > > Explicitly check for MSR_HYPERCALL and MSR_VP_INDEX support when probing > > for running as a Hyper-V guest instead of waiting until hyperv_init() to > > detect the bogus configuration. Add messages to give the admin a heads > > up that they are likely running on a broken virtual machine setup. > > > > At best, silently disabling Hyper-V is confusing and difficult to debug, > > e.g. the kernel _says_ it's using all these fancy Hyper-V features, but > > always falls back to the native versions. At worst, the half baked setup > > will crash/hang the kernel. > > > > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > --- > > arch/x86/hyperv/hv_init.c | 9 +-------- > > arch/x86/kernel/cpu/mshyperv.c | 20 +++++++++++++++----- > > 2 files changed, 16 insertions(+), 13 deletions(-) > > > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > > index 6cc845c026d4..abfb09610e22 100644 > > --- a/arch/x86/hyperv/hv_init.c > > +++ b/arch/x86/hyperv/hv_init.c > > @@ -347,20 +347,13 @@ static void __init hv_get_partition_id(void) > > */ > > void __init hyperv_init(void) > > { > > - u64 guest_id, required_msrs; > > + u64 guest_id; > > union hv_x64_msr_hypercall_contents hypercall_msr; > > int cpuhp; > > > > if (x86_hyper_type != X86_HYPER_MS_HYPERV) > > return; > > > > - /* Absolutely required MSRs */ > > - required_msrs = HV_MSR_HYPERCALL_AVAILABLE | > > - HV_MSR_VP_INDEX_AVAILABLE; > > - > > - if ((ms_hyperv.features & required_msrs) != required_msrs) > > - return; > > - > > if (hv_common_init()) > > return; > > > > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c > > index e095c28d27ae..ef6316fef99f 100644 > > --- a/arch/x86/kernel/cpu/mshyperv.c > > +++ b/arch/x86/kernel/cpu/mshyperv.c > > @@ -163,12 +163,22 @@ static uint32_t __init ms_hyperv_platform(void) > > cpuid(HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS, > > &eax, &hyp_signature[0], &hyp_signature[1], &hyp_signature[2]); > > > > - if (eax >= HYPERV_CPUID_MIN && > > - eax <= HYPERV_CPUID_MAX && > > - !memcmp("Microsoft Hv", hyp_signature, 12)) > > - return HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS; > > + if (eax < HYPERV_CPUID_MIN || eax > HYPERV_CPUID_MAX || > > + memcmp("Microsoft Hv", hyp_signature, 12)) > > + return 0; > > > > - return 0; > > + /* HYPERCALL and VP_INDEX MSRs are mandatory for all features. */ > > + eax = cpuid_eax(HYPERV_CPUID_FEATURES); > > + if (!(eax & HV_MSR_HYPERCALL_AVAILABLE)) { > > + pr_warn("x86/hyperv: HYPERCALL MSR not available.\n"); > > + return 0; > > + } > > + if (!(eax & HV_MSR_VP_INDEX_AVAILABLE)) { > > + pr_warn("x86/hyperv: VP_INDEX MSR not available.\n"); > > + return 0; > > + } > > + > > + return HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS; > > } > > > > static unsigned char hv_get_nmi_reason(void) > > In theory, we can get away without VP_INDEX MSR as e.g. PV spinlocks > don't need it but it will require us to add the check to all other > features which actually need it and disable them so it's probably not > worth the effort (and unless we're running on KVM/QEMU which actually > *can* create such configuration, it's likely impossible to meet such > setup in real life). > Indeed. There is no need to make things more complicated than necessary. > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> Thanks for reviewing. Wei. > > -- > Vitaly > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-11-04 12:24 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-28 22:21 [PATCH 0/2] x86/hyperv: Bug fix and what I hope is an enhancement Sean Christopherson 2021-10-28 22:21 ` [PATCH 1/2] x86/hyperv: Fix NULL deref in set_hv_tscchange_cb() if Hyper-V setup fails Sean Christopherson 2021-10-29 9:14 ` Vitaly Kuznetsov 2021-11-02 12:17 ` Wei Liu 2021-11-02 14:31 ` Sean Christopherson 2021-11-04 12:10 ` Wei Liu 2021-11-04 12:24 ` Wei Liu 2021-10-28 22:21 ` [PATCH 2/2] x86/hyperv: Move required MSRs check to initial platform probing Sean Christopherson 2021-10-29 9:20 ` Vitaly Kuznetsov 2021-11-04 12:13 ` Wei Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).