RE: [PATCH v4 1/5] x86/hyperv: Add support for detecting nested hypervisor

[Michael Kelley (LINUX)]

From: Jinank Jain <jinankjain@xxxxxxxxxxxxxxxxxxx> Sent: Wednesday, November 16, 2022 7:28 PM
> 
> When Linux runs as a root partition for Microsoft Hypervisor. It is
> possible to detect if it is running as nested hypervisor using
> hints exposed by mshv. While at it expose a new variable called
> hv_nested which can be used later for making decisions specific to
> nested use case.

Make the commit statement a bit more direct, and avoid equivocating
words like "possible" and "can be" when there isn't anything that is
doubtful.  Here is my suggestion:

Detect if Linux is running as a nested hypervisor in the root partition
for Microsoft Hypervisor, using flags provided by MSHV.  Expose a new
variable hv_nested that is used later for decisions specific to the
nested use case.

> 
> Signed-off-by: Jinank Jain <jinankjain@xxxxxxxxxxxxxxxxxxx>
> ---
>  arch/arm64/hyperv/mshyperv.c       | 6 ++++++
>  arch/x86/include/asm/hyperv-tlfs.h | 3 +++
>  arch/x86/kernel/cpu/mshyperv.c     | 7 +++++++
>  drivers/hv/hv_common.c             | 7 +++++--
>  include/asm-generic/mshyperv.h     | 1 +
>  5 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
> index a406454578f0..2024b19dc514 100644
> --- a/arch/arm64/hyperv/mshyperv.c
> +++ b/arch/arm64/hyperv/mshyperv.c
> @@ -19,6 +19,9 @@
> 
>  static bool hyperv_initialized;
> 
> +/* Is Linux running on nested Microsoft Hypervisor */
> +bool hv_nested;
> +
>  static int __init hyperv_init(void)
>  {
>  	struct hv_get_vp_registers_output	result;
> @@ -63,6 +66,9 @@ static int __init hyperv_init(void)
>  	pr_info("Hyper-V: Host Build %d.%d.%d.%d-%d-%d\n",
>  		b >> 16, b & 0xFFFF, a,	d & 0xFFFFFF, c, d >> 24);
> 
> +	/* ARM64 does not support nested virtualization */
> +	hv_nested = false;
> +
>  	ret = hv_common_init();
>  	if (ret)
>  		return ret;

The above ARM64 additions aren't needed.  An architecture that works
with the default value of "0" (i.e., "false") doesn't have to do anything
as it uses the version in hv_common.c.   While explicitly coding it on
the ARM64 side doesn't break anything, one of intentions of the __weak
approach is that we don't have to update the ARM64 side every time
we add something that is x86 only.  To avoid irrelevant clutter on the
ARM64 side, the preference is to *not* add such code.  Similarly,
you'll notice that the ARM64 code doesn't initialize hv_root_partition.

> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index 6d9368ea3701..58c03d18c235 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -114,6 +114,9 @@
>  /* Recommend using the newer ExProcessorMasks interface */
>  #define HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED		BIT(11)
> 
> +/* Indicates that the hypervisor is nested within a Hyper-V partition. */
> +#define HV_X64_HYPERV_NESTED				BIT(12)
> +
>  /* Recommend using enlightened VMCS */
>  #define HV_X64_ENLIGHTENED_VMCS_RECOMMENDED		BIT(14)
> 
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 831613959a92..9a4204139490 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -37,6 +37,8 @@
> 
>  /* Is Linux running as the root partition? */
>  bool hv_root_partition;
> +/* Is Linux running on nested Microsoft Hypervisor */
> +bool hv_nested;
>  struct ms_hyperv_info ms_hyperv;
> 
>  #if IS_ENABLED(CONFIG_HYPERV)
> @@ -301,6 +303,11 @@ static void __init ms_hyperv_init_platform(void)
>  		pr_info("Hyper-V: running as root partition\n");
>  	}
> 
> +	if (ms_hyperv.hints & HV_X64_HYPERV_NESTED) {
> +		hv_nested = true;
> +		pr_info("Hyper-V: running on a nested hypervisor\n");
> +	}
> +
>  	/*
>  	 * Extract host information.
>  	 */
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index ae68298c0dca..dcb336ce374f 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -25,8 +25,8 @@
>  #include <asm/mshyperv.h>
> 
>  /*
> - * hv_root_partition and ms_hyperv are defined here with other Hyper-V
> - * specific globals so they are shared across all architectures and are
> + * hv_root_partition, ms_hyperv and hv_nested are defined here with other
> + * Hyper-V specific globals so they are shared across all architectures and are
>   * built only when CONFIG_HYPERV is defined.  But on x86,
>   * ms_hyperv_init_platform() is built even when CONFIG_HYPERV is not
>   * defined, and it uses these two variables.  So mark them as __weak

s/two/three/  (since we now have three such variables)

> @@ -36,6 +36,9 @@
>  bool __weak hv_root_partition;
>  EXPORT_SYMBOL_GPL(hv_root_partition);
> 
> +bool __weak hv_nested;
> +EXPORT_SYMBOL_GPL(hv_nested);
> +
>  struct ms_hyperv_info __weak ms_hyperv;
>  EXPORT_SYMBOL_GPL(ms_hyperv);
> 
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index bfb9eb9d7215..5df6e944e6a9 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -164,6 +164,7 @@ extern int vmbus_interrupt;
>  extern int vmbus_irq;
> 
>  extern bool hv_root_partition;
> +extern bool hv_nested;
> 
>  #if IS_ENABLED(CONFIG_HYPERV)
>  /*
> --
> 2.25.1