On Tue, Feb 25, 2025, at 00:22, Roman Kisel wrote:

Hi Arnd,

[...]

quoted

quoted

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