A few kernel features depend on ms_hyperv.misc_features, but unlike its
siblings ->features and ->hints, the value was never reported during boot.
Signed-off-by: Olaf Hering <[email protected]>
---
arch/x86/kernel/cpu/mshyperv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index caa032ce3fe3..53706fb56433 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -227,8 +227,8 @@ static void __init ms_hyperv_init_platform(void)
ms_hyperv.misc_features = cpuid_edx(HYPERV_CPUID_FEATURES);
ms_hyperv.hints = cpuid_eax(HYPERV_CPUID_ENLIGHTMENT_INFO);
- pr_info("Hyper-V: features 0x%x, hints 0x%x\n",
- ms_hyperv.features, ms_hyperv.hints);
+ pr_info("Hyper-V: features 0x%x, hints 0x%x, misc 0x%x\n",
+ ms_hyperv.features, ms_hyperv.hints, ms_hyperv.misc_features);
ms_hyperv.max_vp_index = cpuid_eax(HYPERV_CPUID_IMPLEMENT_LIMITS);
ms_hyperv.max_lp_index = cpuid_ebx(HYPERV_CPUID_IMPLEMENT_LIMITS);
Olaf Hering <[email protected]> writes:
> A few kernel features depend on ms_hyperv.misc_features, but unlike its
> siblings ->features and ->hints, the value was never reported during boot.
>
> Signed-off-by: Olaf Hering <[email protected]>
> ---
> arch/x86/kernel/cpu/mshyperv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index caa032ce3fe3..53706fb56433 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -227,8 +227,8 @@ static void __init ms_hyperv_init_platform(void)
> ms_hyperv.misc_features = cpuid_edx(HYPERV_CPUID_FEATURES);
> ms_hyperv.hints = cpuid_eax(HYPERV_CPUID_ENLIGHTMENT_INFO);
>
> - pr_info("Hyper-V: features 0x%x, hints 0x%x\n",
> - ms_hyperv.features, ms_hyperv.hints);
> + pr_info("Hyper-V: features 0x%x, hints 0x%x, misc 0x%x\n",
> + ms_hyperv.features, ms_hyperv.hints, ms_hyperv.misc_features);
Several spare thoughts:
I'm always struggling to remember what is what here as these names don't
correspond to TLFS, could we maybe come up with better naming, e.g.
"Features.EAX", "Features.EDX", "Recommendations.EAX",...
On the other hand, there is more, e.g. nested virtualization
features. How is it useful for all users to see this in dmesg every
time? If we need it for development purposes, we can always run 'cpuid'
so maybe let's drop this altogether or print only with pr_debug?
(Honestly: no strong opinion here, deferring to Microsoft folks).
>
> ms_hyperv.max_vp_index = cpuid_eax(HYPERV_CPUID_IMPLEMENT_LIMITS);
> ms_hyperv.max_lp_index = cpuid_ebx(HYPERV_CPUID_IMPLEMENT_LIMITS);
>
--
Vitaly
On Wed, Apr 08, 2020 at 05:54:35PM +0200, Vitaly Kuznetsov wrote:
> Olaf Hering <[email protected]> writes:
>
> > A few kernel features depend on ms_hyperv.misc_features, but unlike its
> > siblings ->features and ->hints, the value was never reported during boot.
> >
> > Signed-off-by: Olaf Hering <[email protected]>
> > ---
> > arch/x86/kernel/cpu/mshyperv.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> > index caa032ce3fe3..53706fb56433 100644
> > --- a/arch/x86/kernel/cpu/mshyperv.c
> > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > @@ -227,8 +227,8 @@ static void __init ms_hyperv_init_platform(void)
> > ms_hyperv.misc_features = cpuid_edx(HYPERV_CPUID_FEATURES);
> > ms_hyperv.hints = cpuid_eax(HYPERV_CPUID_ENLIGHTMENT_INFO);
> >
> > - pr_info("Hyper-V: features 0x%x, hints 0x%x\n",
> > - ms_hyperv.features, ms_hyperv.hints);
> > + pr_info("Hyper-V: features 0x%x, hints 0x%x, misc 0x%x\n",
> > + ms_hyperv.features, ms_hyperv.hints, ms_hyperv.misc_features);
>
> Several spare thoughts:
>
> I'm always struggling to remember what is what here as these names don't
> correspond to TLFS, could we maybe come up with better naming, e.g.
>
> "Features.EAX", "Features.EDX", "Recommendations.EAX",...
>
> On the other hand, there is more, e.g. nested virtualization
> features. How is it useful for all users to see this in dmesg every
> time? If we need it for development purposes, we can always run 'cpuid'
> so maybe let's drop this altogether or print only with pr_debug?
>
> (Honestly: no strong opinion here, deferring to Microsoft folks).
I think making them closer to TLFS is a good idea.
Leaving them always printed is not a big concern to me. Sometimes you
don't get to run `cpuid` if the kernel crashes early.
Olaf's patch is fine as-is. I'm happy to take this patch (if not already
taken by x86 maintainers) given it makes at least one developer's life
easier.
Wei.
>
> >
> > ms_hyperv.max_vp_index = cpuid_eax(HYPERV_CPUID_IMPLEMENT_LIMITS);
> > ms_hyperv.max_lp_index = cpuid_ebx(HYPERV_CPUID_IMPLEMENT_LIMITS);
> >
>
> --
> Vitaly
>