Re: [PATCH hyperv-next v4 1/6] arm64: hyperv: Use SMCCC to detect hypervisor presence

[Roman Kisel <romank@linux.microsoft.com>]

On 2/19/2025 3:13 PM, Michael Kelley wrote:
> From: Roman Kisel <romank@linux.microsoft.com> Sent: Tuesday, February 11, 2025 5:43 PM
>>

[...]

>>   }
>>
>> +static bool hyperv_detect_via_acpi(void)
>> +{
>> +	if (acpi_disabled)
>> +		return false;
>> +#if IS_ENABLED(CONFIG_ACPI)
>> +	/* Hypervisor ID is only available in ACPI v6+. */
> 
> The comment is correct, but to me doesn't tell the full story.
> I initially wondered why the revision test < 6 was being done,
> since Hyper-V for ARM64 always provides ACPI 6.x or greater.
> But the test is needed to catch running in some unknown
> non-Hyper-V environment that has ACPI 5.x or less. In such a
> case, it can't be Hyper-V, so just return false instead of testing
> a bogus hypervisor_id field. Maybe a comment would help
> explain it to someone like me who was confused. :-)
> 

Thanks, I'll extend the comment to tell the whole story!

[...]

>> +	ms_hyperv.vtl = get_vtl();
> 
> The above statement looks like it will fail to compile on
> arm64 since the get_vtl() function is entirely on the x86
> side until Patch 3 of this series. As of this Patch 1, there's
> no declaration of get_vtl() available to arm64.
> 

I used my working branch for testing
https://github.com/romank-msft/linux-hyperv/tree/vtl_mode_arm64_v4
and didn't move that code around when chunking into patches.
Will fix the workflow, thanks for catching this!

>> +	/* Report if non-default VTL */
>> +	if (ms_hyperv.vtl > 0)
>> +		pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
> 
> Could this message include the VTL number as well? In the long
> run, I think there will be code at non-zero VTLs other than VTL 2.
> 

Absolutely, will add!

>> +
>>   	ms_hyperv_late_init();
>>
>>   	hyperv_initialized = true;
>> diff --git a/arch/arm64/include/asm/mshyperv.h
>> b/arch/arm64/include/asm/mshyperv.h
>> index 2e2f83bafcfb..a6d7eb9e167b 100644
>> --- a/arch/arm64/include/asm/mshyperv.h
>> +++ b/arch/arm64/include/asm/mshyperv.h
>> @@ -50,4 +50,9 @@ static inline u64 hv_get_msr(unsigned int reg)
>>
>>   #include <asm-generic/mshyperv.h>
>>
>> +#define ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_0	0x4d32ba58U
>> +#define ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_1	0xcd244764U
>> +#define ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_2	0x8eef6c75U
>> +#define ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_3	0x16597024U
>> +
>>   #endif
>> --
>> 2.43.0
>>

-- 
Thank you,
Roman