linux-kernel - Re: [PATCH v1] x86: hyperv: report value of misc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <874ktu2csk.fsf@vitty.brq.redhat.com>
Date:   Wed, 08 Apr 2020 17:54:35 +0200
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Olaf Hering <olaf@...fle.de>
Cc:     "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
        Michael Kelley <mikelley@...rosoft.com>
Subject: Re: [PATCH v1] x86: hyperv: report value of misc_features

Olaf Hering <olaf@...fle.de> 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 <olaf@...fle.de>
> ---
>  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