| 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
| ||
|
Message-ID:
<SN6PR02MB4157D470D0E46FA9B2ACD8EBD4202@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Thu, 7 Mar 2024 19:22:16 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Nuno Das Neves <nunodasneves@...ux.microsoft.com>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: "mhkelley58@...il.com" <mhkelley58@...il.com>, "haiyangz@...rosoft.com"
<haiyangz@...rosoft.com>, "kys@...rosoft.com" <kys@...rosoft.com>,
"wei.liu@...nel.org" <wei.liu@...nel.org>, "decui@...rosoft.com"
<decui@...rosoft.com>, "catalin.marinas@....com" <catalin.marinas@....com>,
"tglx@...utronix.de" <tglx@...utronix.de>, "dave.hansen@...ux.intel.com"
<dave.hansen@...ux.intel.com>, "arnd@...db.de" <arnd@...db.de>
Subject: RE: [PATCH] mshyperv: Introduce hv_get_hypervisor_version function
From: Nuno Das Neves <nunodasneves@...ux.microsoft.com> Sent: Wednesday, March 6, 2024 4:48 PM
>
> Introduce x86_64 and arm64 functions for getting the hypervisor version
> information and storing it in a structure for simpler parsing.
>
> Use the new function to get and parse the version at boot time. While at
> it, print the version in the same format for each architecture, and move
> the printing code to hv_common_init() so it is not duplicated.
Isn't the format already the same for x86 and ARM64? A couple of
years ago they didn't match. But that was fixed in commit eeda29db98f4.
>
> Signed-off-by: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
> Acked-by: Wei Liu <wei.liu@...nel.org>
> ---
> arch/arm64/hyperv/mshyperv.c | 19 ++++++++---------
> arch/x86/kernel/cpu/mshyperv.c | 35 ++++++++++++++-----------------
> drivers/hv/hv_common.c | 9 ++++++++
> include/asm-generic/hyperv-tlfs.h | 23 ++++++++++++++++++++
> include/asm-generic/mshyperv.h | 2 ++
> 5 files changed, 59 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm64/hyperv/mshyperv.c
> b/arch/arm64/hyperv/mshyperv.c
> index f1b8a04ee9f2..55dc224d466d 100644
> --- a/arch/arm64/hyperv/mshyperv.c
> +++ b/arch/arm64/hyperv/mshyperv.c
> @@ -19,10 +19,18 @@
>
> static bool hyperv_initialized;
>
> +int hv_get_hypervisor_version(union hv_hypervisor_version_info *info)
> +{
> + hv_get_vpreg_128(HV_REGISTER_HYPERVISOR_VERSION,
> + (struct hv_get_vp_registers_output *)info);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(hv_get_hypervisor_version);
I don't think this need to be exported, at least not for the usage in
this patch. The caller in hv_common.c is never part of a module -- it's
always built-in. But maybe you are anticipating future use cases
from a module?
> +
> static int __init hyperv_init(void)
> {
> struct hv_get_vp_registers_output result;
> - u32 a, b, c, d;
> u64 guest_id;
> int ret;
>
> @@ -54,15 +62,6 @@ static int __init hyperv_init(void)
> ms_hyperv.features, ms_hyperv.priv_high, ms_hyperv.hints,
> ms_hyperv.misc_features);
>
> - /* Get information about the Hyper-V host version */
> - hv_get_vpreg_128(HV_REGISTER_HYPERVISOR_VERSION, &result);
> - a = result.as32.a;
> - b = result.as32.b;
> - c = result.as32.c;
> - d = result.as32.d;
> - pr_info("Hyper-V: Host Build %d.%d.%d.%d-%d-%d\n",
> - b >> 16, b & 0xFFFF, a, d & 0xFFFFFF, c, d >> 24);
> -
> ret = hv_common_init();
> if (ret)
> return ret;
> diff --git a/arch/x86/kernel/cpu/mshyperv.c
> b/arch/x86/kernel/cpu/mshyperv.c
> index d306f6184cee..03a3445faf7a 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -350,13 +350,25 @@ static void __init reduced_hw_init(void)
> x86_init.irqs.pre_vector_init = x86_init_noop;
> }
>
> +int hv_get_hypervisor_version(union hv_hypervisor_version_info *info)
> +{
> + unsigned int hv_max_functions;
> +
> + hv_max_functions = cpuid_eax(HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS);
> + if (hv_max_functions < HYPERV_CPUID_VERSION) {
> + pr_err("%s: Could not detect Hyper-V version\n", __func__);
> + return -ENODEV;
> + }
> +
> + cpuid(HYPERV_CPUID_VERSION, &info->eax, &info->ebx, &info->ecx, &info->edx);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(hv_get_hypervisor_version);
Same for this EXPORT.
> +
> static void __init ms_hyperv_init_platform(void)
> {
> int hv_max_functions_eax;
> - int hv_host_info_eax;
> - int hv_host_info_ebx;
> - int hv_host_info_ecx;
> - int hv_host_info_edx;
>
> #ifdef CONFIG_PARAVIRT
> pv_info.name = "Hyper-V";
> @@ -407,21 +419,6 @@ static void __init ms_hyperv_init_platform(void)
> pr_info("Hyper-V: running on a nested hypervisor\n");
> }
>
> - /*
> - * Extract host information.
> - */
> - if (hv_max_functions_eax >= HYPERV_CPUID_VERSION) {
> - hv_host_info_eax = cpuid_eax(HYPERV_CPUID_VERSION);
> - hv_host_info_ebx = cpuid_ebx(HYPERV_CPUID_VERSION);
> - hv_host_info_ecx = cpuid_ecx(HYPERV_CPUID_VERSION);
> - hv_host_info_edx = cpuid_edx(HYPERV_CPUID_VERSION);
> -
> - pr_info("Hyper-V: Host Build %d.%d.%d.%d-%d-%d\n",
> - hv_host_info_ebx >> 16, hv_host_info_ebx & 0xFFFF,
> - hv_host_info_eax, hv_host_info_edx & 0xFFFFFF,
> - hv_host_info_ecx, hv_host_info_edx >> 24);
> - }
> -
> if (ms_hyperv.features & HV_ACCESS_FREQUENCY_MSRS &&
> ms_hyperv.misc_features & HV_FEATURE_FREQUENCY_MSRS_AVAILABLE) {
> x86_platform.calibrate_tsc = hv_get_tsc_khz;
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 2f1dd4b07f9a..4d72c528af68 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -278,6 +278,15 @@ static void hv_kmsg_dump_register(void)
> int __init hv_common_init(void)
> {
> int i;
> + union hv_hypervisor_version_info version;
> +
> + /* Get information about the Hyper-V host version */
> + if (hv_get_hypervisor_version(&version) == 0) {
The usual idiom would be:
if (!hv_get_hypervisor_version(&version)) {
> + pr_info("Hyper-V: Host Build %d.%d.%d.%d-%d-%d\n",
> + version.major_version, version.minor_version,
> + version.build_number, version.service_number,
> + version.service_pack, version.service_branch);
> + }
>
> if (hv_is_isolation_supported())
> sysctl_record_panic_msg = 0;
> diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
> index 3d1b31f90ed6..32514a870b98 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -817,6 +817,29 @@ struct hv_input_unmap_device_interrupt {
> #define HV_SOURCE_SHADOW_NONE 0x0
> #define HV_SOURCE_SHADOW_BRIDGE_BUS_RANGE 0x1
>
> +/*
> + * Version info reported by hypervisor
> + */
> +union hv_hypervisor_version_info {
> + struct {
> + u32 build_number;
> +
> + u32 minor_version : 16;
> + u32 major_version : 16;
> +
> + u32 service_pack;
> +
> + u32 service_number : 24;
> + u32 service_branch : 8;
> + };
> + struct {
> + u32 eax;
> + u32 ebx;
> + u32 ecx;
> + u32 edx;
Nit: These names are x86-isms appearing in the generic portion
of hyperv-tlfs.h. On the ARM64 side I had called the four parts
"a", "b", "c", and "d" to be slightly more generic. But if want to
keep the x86 register names, I won't object.
Michael
> + };
> +};
> +
> /*
> * The whole argument should fit in a page to be able to pass to the hypervisor
> * in one hypercall.
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-
> generic/mshyperv.h
> index 04424a446bb7..452b7c089b71 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -161,6 +161,8 @@ static inline void vmbus_signal_eom(struct
> hv_message *msg, u32 old_msg_type)
> }
> }
>
> +int hv_get_hypervisor_version(union hv_hypervisor_version_info *info);
> +
> void hv_setup_vmbus_handler(void (*handler)(void));
> void hv_remove_vmbus_handler(void);
> void hv_setup_stimer0_handler(void (*handler)(void));
> --
> 2.25.1
>
Powered by blists - more mailing lists