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

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] [day] [month] [year] [list]

Message-Id: <55b65ba6-4abe-478c-a173-4622c30ddd7b@app.fastmail.com>
Date: Tue, 25 Feb 2025 08:24:51 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Roman Kisel" <romank@...ux.microsoft.com>
Cc: benhill@...rosoft.com, bperkins@...rosoft.com, sunilmut@...rosoft.com,
 bhelgaas@...gle.com, "Borislav Petkov" <bp@...en8.de>,
 "Catalin Marinas" <catalin.marinas@....com>,
 "Conor Dooley" <conor+dt@...nel.org>,
 "Dave Hansen" <dave.hansen@...ux.intel.com>,
 "Dexuan Cui" <decui@...rosoft.com>,
 "Haiyang Zhang" <haiyangz@...rosoft.com>,
 "H. Peter Anvin" <hpa@...or.com>, krzk+dt@...nel.org,
 Krzysztof Wilczyński <kw@...ux.com>,
 "K. Y. Srinivasan" <kys@...rosoft.com>,
 "Lorenzo Pieralisi" <lpieralisi@...nel.org>,
 "Manivannan Sadhasivam" <manivannan.sadhasivam@...aro.org>,
 "Ingo Molnar" <mingo@...hat.com>, "Rob Herring" <robh@...nel.org>,
 ssengar@...ux.microsoft.com, "Thomas Gleixner" <tglx@...utronix.de>,
 "Wei Liu" <wei.liu@...nel.org>, "Will Deacon" <will@...nel.org>,
 devicetree@...r.kernel.org, Linux-Arch <linux-arch@...r.kernel.org>,
 linux-arm-kernel@...ts.infradead.org, linux-hyperv@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH hyperv-next v4 1/6] arm64: hyperv: Use SMCCC to detect hypervisor
 presence

On Tue, Feb 25, 2025, at 00:22, Roman Kisel wrote:
> Hi Arnd,
>
> [...]
>
>>> I would suggest moving the UUID values into a variable next
>>> to the caller like
>>>
>>> #define ARM_SMCCC_VENDOR_HYP_UID_KVM \
>>>      UUID_INIT(0x28b46fb6, 0x2ec5, 0x11e9, 0xa9, 0xca, 0x4b, 0x56, 
>>> 0x4d, 0x00, 0x3a, 0x74)
>>>
>>> and then just pass that into arm_smccc_hyp_present(). (please
>>> double-check the endianess of the definition here, I probably
>>> got it wrong myself).
>
> I worked out a variation [1] of the change that you said looked good.
>
> Here, there is a helper macro for creating uuid_t's when checking
> for the hypervisor running via SMCCC to avoid using the bare UUID_INIT. 
> Valiadted with KVM/arm64 and Hyper-V/arm64. Do you think this is a
> better approach than converting by hand?
>
> If that looks too heavy, maybe could leave out converting the expected
> register values to UUID, and pass the expected register values to
> arm_smccc_hyp_present directly. That way, instead of
>
> bool arm_smccc_hyp_present(const uuid_t *hyp_uuid);
>
> we'd have
>
> bool arm_smccc_hyp_present(u32 reg0, u32 reg1, u32 reg2, u32 reg2);
>
>
> Please let me know what you think!

The patch looks correct to me, but I agree it's a little silly
to convert register values into uuid format on both sides.

>   static bool hyperv_detect_via_smccc(void)
>   {
> -	struct arm_smccc_res res = {};
> +	uuid_t hyperv_uuid = HYP_UUID_INIT(ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_0,
> +		ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_1,
> +		ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_2,
> +		ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_3);

If you want to declare a uuid here, I think you should remove the
ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_{0,1,2,3} macros and just
have UUID in normal UUID_INIT() notation as we do for
other UUIDs.

If you want to keep the four 32-bit values and pass them into
arm_smccc_hyp_present() directly, I think that is also fine,
but in that case, I would try to avoid calling it a UUID.

How are the kvm and hyperv values specified originally?
>From the SMCCC document it seems like they are meant to be
UUIDs, so I would expect them to be in canonical form rather
than the smccc return values, but I could not find a document
for them.

     Arnd