Re: [PATCH v2 02/15] mshyperv: Introduce hv_get_hypervisor_version function

[Nuno Das Neves <nunodasneves@linux.microsoft.com>]

On 8/17/2023 4:14 PM, Dave Hansen wrote:
> On 8/17/23 15:01, Nuno Das Neves wrote:
>> +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;
>> +}
> ...
>> +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;
>> +	}
>> +
>> +	info->eax = cpuid_eax(HYPERV_CPUID_VERSION);
>> +	info->ebx = cpuid_ebx(HYPERV_CPUID_VERSION);
>> +	info->ecx = cpuid_ecx(HYPERV_CPUID_VERSION);
>> +	info->edx = cpuid_edx(HYPERV_CPUID_VERSION);
>> +
>> +	return 0;
>> +}
> 
> I can't help but notice that the ARM version does *one* call to the
> hardware while the x86 version does CPUID four different times, once to
> get *EACH* register and throwing away the other three registers each
> time.  Also recall that CPUID is a big, fat architecturally serializing
> instruction.  This isn't a fast path, but CPUID is about as slow as you get.
> 

Good point, it is quite wasteful. I just went and checked cpuid.h, and of
course there is a generic cpuid() function that can get all 4 at once.
I will update it to use this in the next version. Thanks!

> Is there any reason you can't just have an x86 version of
> hv_get_vpreg_128() that gets the 128 bits bytes of data that comes back
> in the 4 CPUID registers?
> 
> That might even let you share some more code.

Unfortunately, for some reason the hypervisor only lets you get this data via
CPUID on x86_64, and via hypercall - i.e. hv_get_vpreg_128() - on ARM64.