RE: [RFC PATCH V3 01/16] x86/hyperv: Add sev-snp enlightened guest specific config

[Michael Kelley (LINUX)]

From: Tianyu Lan <ltykernel@xxxxxxxxx> Sent: Saturday, January 21, 2023 6:46 PM
> 
> Introduce static key isolation_type_en_snp for enlightened
> guest check and add some specific options in ms_hyperv_init_
> platform().
> 
> Signed-off-by: Tianyu Lan <tiala@xxxxxxxxxxxxx>
> ---
>  arch/x86/hyperv/ivm.c           | 10 ++++++++++
>  arch/x86/include/asm/mshyperv.h |  3 +++
>  arch/x86/kernel/cpu/mshyperv.c  | 16 +++++++++++++++-
>  drivers/hv/hv_common.c          |  6 ++++++
>  4 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
> index abca9431d068..8c5dd8e4eb1e 100644
> --- a/arch/x86/hyperv/ivm.c
> +++ b/arch/x86/hyperv/ivm.c
> @@ -386,6 +386,16 @@ bool hv_is_isolation_supported(void)
>  }
> 
>  DEFINE_STATIC_KEY_FALSE(isolation_type_snp);
> +DEFINE_STATIC_KEY_FALSE(isolation_type_en_snp);
> +
> +/*
> + * hv_isolation_type_en_snp - Check system runs in the AMD SEV-SNP based
> + * isolation enlightened VM.
> + */
> +bool hv_isolation_type_en_snp(void)
> +{
> +	return static_branch_unlikely(&isolation_type_en_snp);
> +}
> 
>  /*
>   * hv_isolation_type_snp - Check system runs in the AMD SEV-SNP based
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 010768d40155..285df71150e4 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -14,6 +14,7 @@
>  union hv_ghcb;
> 
>  DECLARE_STATIC_KEY_FALSE(isolation_type_snp);
> +DECLARE_STATIC_KEY_FALSE(isolation_type_en_snp);
> 
>  typedef int (*hyperv_fill_flush_list_func)(
>  		struct hv_guest_mapping_flush_list *flush,
> @@ -28,6 +29,8 @@ extern void *hv_hypercall_pg;
> 
>  extern u64 hv_current_partition_id;
> 
> +extern bool hv_isolation_type_en_snp(void);
> +
>  extern union hv_ghcb * __percpu *hv_ghcb_pg;
> 
>  int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 8f83ceec45dc..ace5901ba0fc 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -273,6 +273,18 @@ static void __init ms_hyperv_init_platform(void)
> 
>  	hv_max_functions_eax = cpuid_eax(HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS);
> 
> +	/*
> +	 * Add custom configuration for SEV-SNP Enlightened guest
> +	 */
> +	if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) {
> +		ms_hyperv.features |= HV_ACCESS_FREQUENCY_MSRS;
> +		ms_hyperv.misc_features |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
> +		ms_hyperv.misc_features &= ~HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
> +		ms_hyperv.hints |= HV_DEPRECATING_AEOI_RECOMMENDED;
> +		ms_hyperv.hints |= HV_X64_APIC_ACCESS_RECOMMENDED;
> +		ms_hyperv.hints |= HV_X64_CLUSTER_IPI_RECOMMENDED;

Two different things are happening in changing the above flags:

1)  Disabling certain feature that Hyper-V might offer to a guest, such
as the crash MSRs and Auto EOI.  (In some cases disabling the feature
means removing the flag.  In other cases in means adding the flag.  But
the net result is same -- other Hyper-V specific code will not use the
feature.)  This category is OK.

2)  Forcing certain features to be treated as enabled.  This category
is somewhat concerning.  Assuming that Hyper-V is accurately indicating
which features are available, it seems better to check that the flags
required by SNP are present, and refuse to boot in SNP mode if not.
Or is this code handling a different problem, where Hyper-V is not
indicating that the feature is available, even though it really is?

> +	}
> +
>  	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);
> @@ -331,7 +343,9 @@ static void __init ms_hyperv_init_platform(void)
>  		pr_info("Hyper-V: Isolation Config: Group A 0x%x, Group B 0x%x\n",
>  			ms_hyperv.isolation_config_a, ms_hyperv.isolation_config_b);
> 
> -		if (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP)
> +		if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
> +			static_branch_enable(&isolation_type_en_snp);
> +		else if (hv_get_isolation_type() == HV_ISOLATION_TYPE_SNP)
>  			static_branch_enable(&isolation_type_snp);
>  	}
> 
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 566735f35c28..f788c64de0bd 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -268,6 +268,12 @@ bool __weak hv_isolation_type_snp(void)
>  }
>  EXPORT_SYMBOL_GPL(hv_isolation_type_snp);
> 
> +bool __weak hv_isolation_type_en_snp(void)
> +{
> +	return false;
> +}
> +EXPORT_SYMBOL_GPL(hv_isolation_type_en_snp);
> +
>  void __weak hv_setup_vmbus_handler(void (*handler)(void))
>  {
>  }
> --
> 2.25.1