On 12/13/2022 1:56 AM, Michael Kelley (LINUX) wrote:
@@ -32,6 +33,7 @@ extern u64 hv_current_partition_id;
extern union hv_ghcb * __percpu *hv_ghcb_pg;
+extern bool hv_isolation_type_en_snp(void);
This file also has a declaration for hv_isolation_type_snp(). I
think this new declaration is near the beginning of this file so
that it can be referenced by hv_do_hypercall() and related
functions in Patch 6 of this series. So maybe move the
declaration of hv_isolation_type_snp() up so it is adjacent
to this one? It would make sense for the two to be together.
Agree. Will update in the next version.
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 831613959a92..2ea4f21c6172 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -273,6 +273,21 @@ static void __init ms_hyperv_init_platform(void)
ms_hyperv.misc_features = cpuid_edx(HYPERV_CPUID_FEATURES);
ms_hyperv.hints = cpuid_eax(HYPERV_CPUID_ENLIGHTMENT_INFO);
+ /*
+ * 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;
+ }
+
+ pr_info("Hyper-V: enlightment features 0x%x, hints 0x%x, misc 0x%x\n",
+ ms_hyperv.features, ms_hyperv.hints, ms_hyperv.misc_features);
+
What's the reason for this additional call to pr_info()? There's a call to pr_info()
a couple of lines below that displays the same information, and a little bit more.
It seems like the above call should be deleted, as I think we should try to be as
consistent as possible in the output.
Sorry for noise. This one should be redundant. Will remove in the next
version.
@@ -328,18 +343,22 @@ static void __init ms_hyperv_init_platform(void)
ms_hyperv.shared_gpa_boundary =
BIT_ULL(ms_hyperv.shared_gpa_boundary_bits);
- 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);
#ifdef CONFIG_SWIOTLB
swiotlb_unencrypted_base = ms_hyperv.shared_gpa_boundary;
#endif
}
+
+ 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);
+
Is there a reason for moving this pr_info() down a few lines? I can't see that the
intervening code changes any of the settings that are displayed, so it should be
good in the original location. Just trying to minimize changes that don't add value ...
Agree. Will keep previous order. Thanks.
