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?
