From: Nuno Das Neves <[email protected]> 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 <[email protected]> > Acked-by: Wei Liu <[email protected]> > --- > 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 >
