Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 02 Aug 2021 16:55:13 +0100
Message-ID: <87lf5j265a.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Michael Kelley <mikelley@microsoft.com>
Cc:     will@kernel.org, catalin.marinas@arm.com, mark.rutland@arm.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-hyperv@vger.kernel.org, linux-efi@vger.kernel.org,
        wei.liu@kernel.org, kys@microsoft.com, sthemmin@microsoft.com,
        ardb@kernel.org
Subject: Re: [PATCH v11 3/5] arm64: hyperv: Initialize hypervisor on boot
In-Reply-To: <1626793023-13830-4-git-send-email-mikelley@microsoft.com>
References: <1626793023-13830-1-git-send-email-mikelley@microsoft.com>
        <1626793023-13830-4-git-send-email-mikelley@microsoft.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Tue, 20 Jul 2021 15:57:01 +0100,
Michael Kelley <mikelley@microsoft.com> wrote:
> 
> Add ARM64-specific code to initialize the Hyper-V
> hypervisor when booting as a guest VM.
> 
> This code is built only when CONFIG_HYPERV is enabled.
> 
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
>  arch/arm64/hyperv/Makefile   |  2 +-
>  arch/arm64/hyperv/mshyperv.c | 83 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 84 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm64/hyperv/mshyperv.c
> 
> diff --git a/arch/arm64/hyperv/Makefile b/arch/arm64/hyperv/Makefile
> index 1697d30..87c31c0 100644
> --- a/arch/arm64/hyperv/Makefile
> +++ b/arch/arm64/hyperv/Makefile
> @@ -1,2 +1,2 @@
>  # SPDX-License-Identifier: GPL-2.0
> -obj-y		:= hv_core.o
> +obj-y		:= hv_core.o mshyperv.o
> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
> new file mode 100644
> index 0000000..2811fd0
> --- /dev/null
> +++ b/arch/arm64/hyperv/mshyperv.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Core routines for interacting with Microsoft's Hyper-V hypervisor,
> + * including hypervisor initialization.
> + *
> + * Copyright (C) 2021, Microsoft, Inc.
> + *
> + * Author : Michael Kelley <mikelley@microsoft.com>
> + */
> +
> +#include <linux/types.h>
> +#include <linux/acpi.h>
> +#include <linux/export.h>
> +#include <linux/errno.h>
> +#include <linux/version.h>
> +#include <linux/cpuhotplug.h>
> +#include <asm/mshyperv.h>
> +
> +static bool hyperv_initialized;
> +
> +static int __init hyperv_init(void)
> +{
> +	struct hv_get_vp_registers_output	result;
> +	u32	a, b, c, d;
> +	u64	guest_id;
> +	int	ret;
> +
> +	/*
> +	 * If we're in a VM on Hyper-V, the ACPI hypervisor_id field will
> +	 * have the string "MsHyperV".
> +	 */
> +	if (strncmp((char *)&acpi_gbl_FADT.hypervisor_id, "MsHyperV", 8))
> +		return -EINVAL;

Before going ahead and parsing the local FADT copy, it would be
prudent to test that you are actually on an ACPI system, specially
given that this function is unconditionally called, even on DT
systems.  In the same vein, returning -EINVAL when failing to find
Hyper-V is a bit excessive.

> +
> +	/* Setup the guest ID */
> +	guest_id = generate_guest_id(0, LINUX_VERSION_CODE, 0);
> +	hv_set_vpreg(HV_REGISTER_GUEST_OSID, guest_id);
> +
> +	/* Get the features and hints from Hyper-V */
> +	hv_get_vpreg_128(HV_REGISTER_FEATURES, &result);
> +	ms_hyperv.features = result.as32.a;
> +	ms_hyperv.priv_high = result.as32.b;
> +	ms_hyperv.misc_features = result.as32.c;
> +
> +	hv_get_vpreg_128(HV_REGISTER_ENLIGHTENMENTS, &result);
> +	ms_hyperv.hints = result.as32.a;
> +
> +	pr_info("Hyper-V: privilege flags low 0x%x, high 0x%x, hints 0x%x, misc 0x%x\n",
> +		ms_hyperv.features, ms_hyperv.priv_high, ms_hyperv.hints,
> +		ms_hyperv.misc_features);
> +
> +	/* Get information about the Hyper-V host version */
> +	hv_get_vpreg_128(HV_REGISTER_HYPERVISOR_VERSION, &result);
> +	a = result.as32.a;
> +	b = result.as32.b;
> +	c = result.as32.c;
> +	d = result.as32.d;
> +	pr_info("Hyper-V: Host Build %d.%d.%d.%d-%d-%d\n",
> +		b >> 16, b & 0xFFFF, a,	d & 0xFFFFFF, c, d >> 24);
> +
> +	ret = hv_common_init();
> +	if (ret)
> +		return ret;
> +
> +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm64/hyperv_init:online",
> +				hv_common_cpu_init, hv_common_cpu_die);
> +	if (ret < 0) {
> +		hv_common_free();
> +		return ret;
> +	}
> +
> +	hyperv_initialized = true;
> +	return 0;
> +}
> +
> +early_initcall(hyperv_init);
> +
> +bool hv_is_hyperv_initialized(void)
> +{
> +	return hyperv_initialized;
> +}
> +EXPORT_SYMBOL_GPL(hv_is_hyperv_initialized);

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.