[PATCH v2 00/14] KVM: SVM: Misc SEV cleanups

[PATCH v2 00/14] KVM: SVM: Misc SEV cleanups

[Sean Christopherson]

Minor bug fixes and refactorings of SEV related code, mainly to clean up
the KVM code for tracking whether or not SEV and SEV-ES are enabled.  E.g.
KVM has both sev_es and svm_sev_enabled(), and a global 'sev' flag while
also using 'sev' as a local variable in several places.

Based on kvm/master, commit 872f36eb0b0f ("KVM: x86: __kvm_vcpu_halt can
be static").

v2:
 - Remove the kernel's sev_enabled instead of renaming it to sev_guest.
 - Fix various build issues. [Tom]
 - Remove stable tag from the patch to free sev_asid_bitmap.  Keeping the
   bitmap on failure is truly only a leak once svm_sev_enabled() is
   dropped later in the series.  It's still arguably a fix since KVM will
   unnecessarily keep memory, but it's not stable material. [Tom]
 - Collect one Ack. [Tom]

v1:
 - https://lkml.kernel.org/r/20210109004714.1341275-1-seanjc@google.com

Sean Christopherson (14):
  KVM: SVM: Zero out the VMCB array used to track SEV ASID association
  KVM: SVM: Free sev_asid_bitmap during init if SEV setup fails
  KVM: SVM: Move SEV module params/variables to sev.c
  x86/cpufeatures: Assign dedicated feature word for AMD mem encryption
  KVM: x86: Override reported SME/SEV feature flags with host mask
  x86/sev: Drop redundant and potentially misleading 'sev_enabled'
  KVM: SVM: Append "_enabled" to module-scoped SEV/SEV-ES control
    variables
  KVM: SVM: Condition sev_enabled and sev_es_enabled on
    CONFIG_KVM_AMD_SEV=y
  KVM: SVM: Unconditionally invoke sev_hardware_teardown()
  KVM: SVM: Explicitly check max SEV ASID during sev_hardware_setup()
  KVM: SVM: Move SEV VMCB tracking allocation to sev.c
  KVM: SVM: Drop redundant svm_sev_enabled() helper
  KVM: SVM: Remove an unnecessary prototype declaration of
    sev_flush_asids()
  KVM: SVM: Skip SEV cache flush if no ASIDs have been used

 arch/x86/include/asm/cpufeature.h             |  7 +-
 arch/x86/include/asm/cpufeatures.h            | 17 +++--
 arch/x86/include/asm/disabled-features.h      |  3 +-
 arch/x86/include/asm/mem_encrypt.h            |  1 -
 arch/x86/include/asm/required-features.h      |  3 +-
 arch/x86/kernel/cpu/common.c                  |  3 +
 arch/x86/kernel/cpu/scattered.c               |  5 --
 arch/x86/kvm/cpuid.c                          |  2 +
 arch/x86/kvm/cpuid.h                          |  1 +
 arch/x86/kvm/svm/sev.c                        | 71 +++++++++++++------
 arch/x86/kvm/svm/svm.c                        | 35 +++------
 arch/x86/kvm/svm/svm.h                        |  8 +--
 arch/x86/mm/mem_encrypt.c                     | 12 ++--
 arch/x86/mm/mem_encrypt_identity.c            |  1 -
 .../arch/x86/include/asm/disabled-features.h  |  3 +-
 .../arch/x86/include/asm/required-features.h  |  3 +-
 16 files changed, 96 insertions(+), 79 deletions(-)

-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 01/14] KVM: SVM: Zero out the VMCB array used to track SEV ASID association

[Sean Christopherson]

Zero out the array of VMCB pointers so that pre_sev_run() won't see
garbage when querying the array to detect when an SEV ASID is being
associated with a new VMCB.  In practice, reading random values is all
but guaranteed to be benign as a false negative (which is extremely
unlikely on its own) can only happen on CPU0 on the first VMRUN and would
only cause KVM to skip the ASID flush.  For anything bad to happen, a
previous instance of KVM would have to exit without flushing the ASID,
_and_ KVM would have to not flush the ASID at any time while building the
new SEV guest.

Cc: Borislav Petkov <bp@suse.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Fixes: 70cd94e60c73 ("KVM: SVM: VMRUN should use associated ASID when SEV is enabled")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 7ef171790d02..ccf52c5531fb 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -573,7 +573,7 @@ static int svm_cpu_init(int cpu)
 	if (svm_sev_enabled()) {
 		sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1,
 					      sizeof(void *),
-					      GFP_KERNEL);
+					      GFP_KERNEL | __GFP_ZERO);
 		if (!sd->sev_vmcbs)
 			goto free_save_area;
 	}
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 02/14] KVM: SVM: Free sev_asid_bitmap during init if SEV setup fails

[Sean Christopherson]

Free sev_asid_bitmap if the reclaim bitmap allocation fails, othwerise
KVM will unnecessarily keep the bitmap when SEV is not fully enabled.

Freeing the page is also necessary to avoid introducing a bug when a
future patch eliminates svm_sev_enabled() in favor of using the global
'sev' flag directly.  While sev_hardware_enabled() checks max_sev_asid,
which is true even if KVM setup fails, 'sev' will be true if and only
if KVM setup fully succeeds.

Fixes: 33af3a7ef9e6 ("KVM: SVM: Reduce WBINVD/DF_FLUSH invocations")
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index c8ffdbc81709..0eeb6e1b803d 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1274,8 +1274,10 @@ void __init sev_hardware_setup(void)
 		goto out;
 
 	sev_reclaim_asid_bitmap = bitmap_zalloc(max_sev_asid, GFP_KERNEL);
-	if (!sev_reclaim_asid_bitmap)
+	if (!sev_reclaim_asid_bitmap) {
+		bitmap_free(sev_asid_bitmap);
 		goto out;
+	}
 
 	pr_info("SEV supported: %u ASIDs\n", max_sev_asid - min_sev_asid + 1);
 	sev_supported = true;
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 03/14] KVM: SVM: Move SEV module params/variables to sev.c

[Sean Christopherson]

Unconditionally invoke sev_hardware_setup() when configuring SVM and
handle clearing the module params/variable 'sev' and 'sev_es' in
sev_hardware_setup().  This allows making said variables static within
sev.c and reduces the odds of a collision with guest code, e.g. the guest
side of things has already laid claim to 'sev_enabled'.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 11 +++++++++++
 arch/x86/kvm/svm/svm.c | 15 +--------------
 arch/x86/kvm/svm/svm.h |  2 --
 3 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 0eeb6e1b803d..8ba93b8fa435 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -27,6 +27,14 @@
 
 #define __ex(x) __kvm_handle_fault_on_reboot(x)
 
+/* enable/disable SEV support */
+static int sev = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
+module_param(sev, int, 0444);
+
+/* enable/disable SEV-ES support */
+static int sev_es = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
+module_param(sev_es, int, 0444);
+
 static u8 sev_enc_bit;
 static int sev_flush_asids(void);
 static DECLARE_RWSEM(sev_deactivate_lock);
@@ -1249,6 +1257,9 @@ void __init sev_hardware_setup(void)
 	bool sev_es_supported = false;
 	bool sev_supported = false;
 
+	if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev)
+		goto out;
+
 	/* Does the CPU support SEV? */
 	if (!boot_cpu_has(X86_FEATURE_SEV))
 		goto out;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index ccf52c5531fb..f89f702b2a58 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -189,14 +189,6 @@ module_param(vls, int, 0444);
 static int vgif = true;
 module_param(vgif, int, 0444);
 
-/* enable/disable SEV support */
-int sev = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
-module_param(sev, int, 0444);
-
-/* enable/disable SEV-ES support */
-int sev_es = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
-module_param(sev_es, int, 0444);
-
 bool __read_mostly dump_invalid_vmcb;
 module_param(dump_invalid_vmcb, bool, 0644);
 
@@ -976,12 +968,7 @@ static __init int svm_hardware_setup(void)
 		kvm_enable_efer_bits(EFER_SVME | EFER_LMSLE);
 	}
 
-	if (IS_ENABLED(CONFIG_KVM_AMD_SEV) && sev) {
-		sev_hardware_setup();
-	} else {
-		sev = false;
-		sev_es = false;
-	}
+	sev_hardware_setup();
 
 	svm_adjust_mmio_mask();
 
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 0fe874ae5498..8e169835f52a 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -408,8 +408,6 @@ static inline bool gif_set(struct vcpu_svm *svm)
 #define MSR_CR3_LONG_MBZ_MASK			0xfff0000000000000U
 #define MSR_INVALID				0xffffffffU
 
-extern int sev;
-extern int sev_es;
 extern bool dump_invalid_vmcb;
 
 u32 svm_msrpm_offset(u32 msr);
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 04/14] x86/cpufeatures: Assign dedicated feature word for AMD mem encryption

[Sean Christopherson]

Collect the scattered SME/SEV related feature flags into a dedicated
word.  There are now five recognized features in CPUID.0x8000001F.EAX,
with at least one more on the horizon (SEV-SNP).  Using a dedicated word
allows KVM to use its automagic CPUID adjustment logic when reporting
the set of supported features to userspace.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/cpufeature.h              |  7 +++++--
 arch/x86/include/asm/cpufeatures.h             | 17 +++++++++++------
 arch/x86/include/asm/disabled-features.h       |  3 ++-
 arch/x86/include/asm/required-features.h       |  3 ++-
 arch/x86/kernel/cpu/common.c                   |  3 +++
 arch/x86/kernel/cpu/scattered.c                |  5 -----
 tools/arch/x86/include/asm/disabled-features.h |  3 ++-
 tools/arch/x86/include/asm/required-features.h |  3 ++-
 8 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 59bf91c57aa8..1728d4ce5730 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -30,6 +30,7 @@ enum cpuid_leafs
 	CPUID_7_ECX,
 	CPUID_8000_0007_EBX,
 	CPUID_7_EDX,
+	CPUID_8000_001F_EAX,
 };
 
 #ifdef CONFIG_X86_FEATURE_NAMES
@@ -88,8 +89,9 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
 	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 16, feature_bit) ||	\
 	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 17, feature_bit) ||	\
 	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 18, feature_bit) ||	\
+	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 19, feature_bit) ||	\
 	   REQUIRED_MASK_CHECK					  ||	\
-	   BUILD_BUG_ON_ZERO(NCAPINTS != 19))
+	   BUILD_BUG_ON_ZERO(NCAPINTS != 20))
 
 #define DISABLED_MASK_BIT_SET(feature_bit)				\
 	 ( CHECK_BIT_IN_MASK_WORD(DISABLED_MASK,  0, feature_bit) ||	\
@@ -111,8 +113,9 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
 	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 16, feature_bit) ||	\
 	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 17, feature_bit) ||	\
 	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 18, feature_bit) ||	\
+	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 19, feature_bit) ||	\
 	   DISABLED_MASK_CHECK					  ||	\
-	   BUILD_BUG_ON_ZERO(NCAPINTS != 19))
+	   BUILD_BUG_ON_ZERO(NCAPINTS != 20))
 
 #define cpu_has(c, bit)							\
 	(__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 :	\
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 9f9e9511f7cd..7c0bb1a20050 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -13,7 +13,7 @@
 /*
  * Defines x86 CPU feature bits
  */
-#define NCAPINTS			19	   /* N 32-bit words worth of info */
+#define NCAPINTS			20	   /* N 32-bit words worth of info */
 #define NBUGINTS			1	   /* N 32-bit bug flags */
 
 /*
@@ -96,7 +96,7 @@
 #define X86_FEATURE_SYSCALL32		( 3*32+14) /* "" syscall in IA32 userspace */
 #define X86_FEATURE_SYSENTER32		( 3*32+15) /* "" sysenter in IA32 userspace */
 #define X86_FEATURE_REP_GOOD		( 3*32+16) /* REP microcode works well */
-#define X86_FEATURE_SME_COHERENT	( 3*32+17) /* "" AMD hardware-enforced cache coherency */
+/* FREE!                                ( 3*32+17) */
 #define X86_FEATURE_LFENCE_RDTSC	( 3*32+18) /* "" LFENCE synchronizes RDTSC */
 #define X86_FEATURE_ACC_POWER		( 3*32+19) /* AMD Accumulated Power Mechanism */
 #define X86_FEATURE_NOPL		( 3*32+20) /* The NOPL (0F 1F) instructions */
@@ -201,7 +201,7 @@
 #define X86_FEATURE_INVPCID_SINGLE	( 7*32+ 7) /* Effectively INVPCID && CR4.PCIDE=1 */
 #define X86_FEATURE_HW_PSTATE		( 7*32+ 8) /* AMD HW-PState */
 #define X86_FEATURE_PROC_FEEDBACK	( 7*32+ 9) /* AMD ProcFeedbackInterface */
-#define X86_FEATURE_SME			( 7*32+10) /* AMD Secure Memory Encryption */
+/* FREE!                                ( 7*32+10) */
 #define X86_FEATURE_PTI			( 7*32+11) /* Kernel Page Table Isolation enabled */
 #define X86_FEATURE_RETPOLINE		( 7*32+12) /* "" Generic Retpoline mitigation for Spectre variant 2 */
 #define X86_FEATURE_RETPOLINE_AMD	( 7*32+13) /* "" AMD Retpoline mitigation for Spectre variant 2 */
@@ -211,7 +211,7 @@
 #define X86_FEATURE_SSBD		( 7*32+17) /* Speculative Store Bypass Disable */
 #define X86_FEATURE_MBA			( 7*32+18) /* Memory Bandwidth Allocation */
 #define X86_FEATURE_RSB_CTXSW		( 7*32+19) /* "" Fill RSB on context switches */
-#define X86_FEATURE_SEV			( 7*32+20) /* AMD Secure Encrypted Virtualization */
+/* FREE!                                ( 7*32+20) */
 #define X86_FEATURE_USE_IBPB		( 7*32+21) /* "" Indirect Branch Prediction Barrier enabled */
 #define X86_FEATURE_USE_IBRS_FW		( 7*32+22) /* "" Use IBRS during runtime firmware calls */
 #define X86_FEATURE_SPEC_STORE_BYPASS_DISABLE	( 7*32+23) /* "" Disable Speculative Store Bypass. */
@@ -236,8 +236,6 @@
 #define X86_FEATURE_EPT_AD		( 8*32+17) /* Intel Extended Page Table access-dirty bit */
 #define X86_FEATURE_VMCALL		( 8*32+18) /* "" Hypervisor supports the VMCALL instruction */
 #define X86_FEATURE_VMW_VMMCALL		( 8*32+19) /* "" VMware prefers VMMCALL hypercall instruction */
-#define X86_FEATURE_SEV_ES		( 8*32+20) /* AMD Secure Encrypted Virtualization - Encrypted State */
-#define X86_FEATURE_VM_PAGE_FLUSH	( 8*32+21) /* "" VM Page Flush MSR is supported */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (EBX), word 9 */
 #define X86_FEATURE_FSGSBASE		( 9*32+ 0) /* RDFSBASE, WRFSBASE, RDGSBASE, WRGSBASE instructions*/
@@ -383,6 +381,13 @@
 #define X86_FEATURE_CORE_CAPABILITIES	(18*32+30) /* "" IA32_CORE_CAPABILITIES MSR */
 #define X86_FEATURE_SPEC_CTRL_SSBD	(18*32+31) /* "" Speculative Store Bypass Disable */
 
+/* AMD-defined memory encryption features, CPUID level 0x8000001f (EAX), word 19 */
+#define X86_FEATURE_SME			(19*32+ 0) /* AMD Secure Memory Encryption */
+#define X86_FEATURE_SEV			(19*32+ 1) /* AMD Secure Encrypted Virtualization */
+#define X86_FEATURE_VM_PAGE_FLUSH	(19*32+ 2) /* "" VM Page Flush MSR is supported */
+#define X86_FEATURE_SEV_ES		(19*32+ 3) /* AMD Secure Encrypted Virtualization - Encrypted State */
+#define X86_FEATURE_SME_COHERENT	(19*32+10) /* "" AMD hardware-enforced cache coherency */
+
 /*
  * BUG word(s)
  */
diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
index 5861d34f9771..2216077676c8 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -85,6 +85,7 @@
 			 DISABLE_ENQCMD)
 #define DISABLED_MASK17	0
 #define DISABLED_MASK18	0
-#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
+#define DISABLED_MASK19	0
+#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
 
 #endif /* _ASM_X86_DISABLED_FEATURES_H */
diff --git a/arch/x86/include/asm/required-features.h b/arch/x86/include/asm/required-features.h
index 3ff0d48469f2..b2d504f11937 100644
--- a/arch/x86/include/asm/required-features.h
+++ b/arch/x86/include/asm/required-features.h
@@ -101,6 +101,7 @@
 #define REQUIRED_MASK16	0
 #define REQUIRED_MASK17	0
 #define REQUIRED_MASK18	0
-#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
+#define REQUIRED_MASK19	0
+#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
 
 #endif /* _ASM_X86_REQUIRED_FEATURES_H */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 35ad8480c464..9215b91bc044 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -960,6 +960,9 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 	if (c->extended_cpuid_level >= 0x8000000a)
 		c->x86_capability[CPUID_8000_000A_EDX] = cpuid_edx(0x8000000a);
 
+	if (c->extended_cpuid_level >= 0x8000001f)
+		c->x86_capability[CPUID_8000_001F_EAX] = cpuid_eax(0x8000001f);
+
 	init_scattered_cpuid_features(c);
 	init_speculation_control(c);
 
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 236924930bf0..972ec3bfa9c0 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -40,11 +40,6 @@ static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_CPB,		CPUID_EDX,  9, 0x80000007, 0 },
 	{ X86_FEATURE_PROC_FEEDBACK,    CPUID_EDX, 11, 0x80000007, 0 },
 	{ X86_FEATURE_MBA,		CPUID_EBX,  6, 0x80000008, 0 },
-	{ X86_FEATURE_SME,		CPUID_EAX,  0, 0x8000001f, 0 },
-	{ X86_FEATURE_SEV,		CPUID_EAX,  1, 0x8000001f, 0 },
-	{ X86_FEATURE_SEV_ES,		CPUID_EAX,  3, 0x8000001f, 0 },
-	{ X86_FEATURE_SME_COHERENT,	CPUID_EAX, 10, 0x8000001f, 0 },
-	{ X86_FEATURE_VM_PAGE_FLUSH,	CPUID_EAX,  2, 0x8000001f, 0 },
 	{ 0, 0, 0, 0, 0 }
 };
 
diff --git a/tools/arch/x86/include/asm/disabled-features.h b/tools/arch/x86/include/asm/disabled-features.h
index 5861d34f9771..2216077676c8 100644
--- a/tools/arch/x86/include/asm/disabled-features.h
+++ b/tools/arch/x86/include/asm/disabled-features.h
@@ -85,6 +85,7 @@
 			 DISABLE_ENQCMD)
 #define DISABLED_MASK17	0
 #define DISABLED_MASK18	0
-#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
+#define DISABLED_MASK19	0
+#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
 
 #endif /* _ASM_X86_DISABLED_FEATURES_H */
diff --git a/tools/arch/x86/include/asm/required-features.h b/tools/arch/x86/include/asm/required-features.h
index 3ff0d48469f2..b2d504f11937 100644
--- a/tools/arch/x86/include/asm/required-features.h
+++ b/tools/arch/x86/include/asm/required-features.h
@@ -101,6 +101,7 @@
 #define REQUIRED_MASK16	0
 #define REQUIRED_MASK17	0
 #define REQUIRED_MASK18	0
-#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
+#define REQUIRED_MASK19	0
+#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
 
 #endif /* _ASM_X86_REQUIRED_FEATURES_H */
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 05/14] KVM: x86: Override reported SME/SEV feature flags with host mask

[Sean Christopherson]

Add a reverse-CPUID entry for the memory encryption word, 0x8000001F.EAX,
and use it to override the supported CPUID flags reported to userspace.
Masking the reported CPUID flags avoids over-reporting KVM support, e.g.
without the mask a SEV-SNP capable CPU may incorrectly advertise SNP
support to userspace.

Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/cpuid.c | 2 ++
 arch/x86/kvm/cpuid.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 13036cf0b912..b7618cdd06b5 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -855,6 +855,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 	case 0x8000001F:
 		if (!boot_cpu_has(X86_FEATURE_SEV))
 			entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
+		else
+			cpuid_entry_override(entry, CPUID_8000_001F_EAX);
 		break;
 	/*Add support for Centaur's CPUID instruction*/
 	case 0xC0000000:
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index dc921d76e42e..8b6fc9bde248 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -63,6 +63,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
 	[CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX},
 	[CPUID_7_EDX]         = {         7, 0, CPUID_EDX},
 	[CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
+	[CPUID_8000_001F_EAX] = {0x8000001f, 1, CPUID_EAX},
 };
 
 /*
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 06/14] x86/sev: Drop redundant and potentially misleading 'sev_enabled'

[Sean Christopherson]

Drop the sev_enabled flag and switch its one user over to sev_active().
sev_enabled was made redundant with the introduction of sev_status in
commit b57de6cd1639 ("x86/sev-es: Add SEV-ES Feature Detection").
sev_enabled and sev_active() are guaranteed to be equivalent, as each is
true iff 'sev_status & MSR_AMD64_SEV_ENABLED' is true, and are only ever
written in tandem (ignoring compressed boot's version of sev_status).

Removing sev_enabled avoids confusion over whether it refers to the guest
or the host, and will also allow KVM to usurp "sev_enabled" for its own
purposes.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/mem_encrypt.h |  1 -
 arch/x86/mm/mem_encrypt.c          | 12 +++++-------
 arch/x86/mm/mem_encrypt_identity.c |  1 -
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index 2f62bbdd9d12..88d624499411 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -20,7 +20,6 @@
 
 extern u64 sme_me_mask;
 extern u64 sev_status;
-extern bool sev_enabled;
 
 void sme_encrypt_execute(unsigned long encrypted_kernel_vaddr,
 			 unsigned long decrypted_kernel_vaddr,
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index bc0833713be9..b89bc03c63a2 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -44,8 +44,6 @@ EXPORT_SYMBOL(sme_me_mask);
 DEFINE_STATIC_KEY_FALSE(sev_enable_key);
 EXPORT_SYMBOL_GPL(sev_enable_key);
 
-bool sev_enabled __section(".data");
-
 /* Buffer used for early in-place encryption by BSP, no locking needed */
 static char sme_early_buffer[PAGE_SIZE] __initdata __aligned(PAGE_SIZE);
 
@@ -342,16 +340,16 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size)
  * up under SME the trampoline area cannot be encrypted, whereas under SEV
  * the trampoline area must be encrypted.
  */
-bool sme_active(void)
-{
-	return sme_me_mask && !sev_enabled;
-}
-
 bool sev_active(void)
 {
 	return sev_status & MSR_AMD64_SEV_ENABLED;
 }
 
+bool sme_active(void)
+{
+	return sme_me_mask && !sev_active();
+}
+
 /* Needs to be called from non-instrumentable code */
 bool noinstr sev_es_active(void)
 {
diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c
index 6c5eb6f3f14f..0c2759b7f03a 100644
--- a/arch/x86/mm/mem_encrypt_identity.c
+++ b/arch/x86/mm/mem_encrypt_identity.c
@@ -545,7 +545,6 @@ void __init sme_enable(struct boot_params *bp)
 
 		/* SEV state cannot be controlled by a command line option */
 		sme_me_mask = me_mask;
-		sev_enabled = true;
 		physical_mask &= ~sme_me_mask;
 		return;
 	}
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 07/14] KVM: SVM: Append "_enabled" to module-scoped SEV/SEV-ES control variables

[Sean Christopherson]

Rename sev and sev_es to sev_enabled and sev_es_enabled respectively to
better align with other KVM terminology, and to avoid pseudo-shadowing
when the variables are moved to sev.c in a future patch ('sev' is often
used for local struct kvm_sev_info pointers.

No functional change intended.

Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 8ba93b8fa435..a024edabaca5 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -28,12 +28,12 @@
 #define __ex(x) __kvm_handle_fault_on_reboot(x)
 
 /* enable/disable SEV support */
-static int sev = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
-module_param(sev, int, 0444);
+static bool sev_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
+module_param_named(sev, sev_enabled, bool, 0444);
 
 /* enable/disable SEV-ES support */
-static int sev_es = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
-module_param(sev_es, int, 0444);
+static bool sev_es_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
+module_param_named(sev_es, sev_es_enabled, bool, 0444);
 
 static u8 sev_enc_bit;
 static int sev_flush_asids(void);
@@ -213,7 +213,7 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
 
 static int sev_es_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
 {
-	if (!sev_es)
+	if (!sev_es_enabled)
 		return -ENOTTY;
 
 	to_kvm_svm(kvm)->sev_info.es_active = true;
@@ -1052,7 +1052,7 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	struct kvm_sev_cmd sev_cmd;
 	int r;
 
-	if (!svm_sev_enabled() || !sev)
+	if (!svm_sev_enabled() || !sev_enabled)
 		return -ENOTTY;
 
 	if (!argp)
@@ -1257,7 +1257,7 @@ void __init sev_hardware_setup(void)
 	bool sev_es_supported = false;
 	bool sev_supported = false;
 
-	if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev)
+	if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev_enabled)
 		goto out;
 
 	/* Does the CPU support SEV? */
@@ -1294,7 +1294,7 @@ void __init sev_hardware_setup(void)
 	sev_supported = true;
 
 	/* SEV-ES support requested? */
-	if (!sev_es)
+	if (!sev_es_enabled)
 		goto out;
 
 	/* Does the CPU support SEV-ES? */
@@ -1309,8 +1309,8 @@ void __init sev_hardware_setup(void)
 	sev_es_supported = true;
 
 out:
-	sev = sev_supported;
-	sev_es = sev_es_supported;
+	sev_enabled = sev_supported;
+	sev_es_enabled = sev_es_supported;
 }
 
 void sev_hardware_teardown(void)
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 08/14] KVM: SVM: Condition sev_enabled and sev_es_enabled on CONFIG_KVM_AMD_SEV=y

[Sean Christopherson]

Define sev_enabled and sev_es_enabled as 'false' and explicitly #ifdef
out all of sev_hardware_setup() if CONFIG_KVM_AMD_SEV=n.  This kills
three birds at once:

  - Makes sev_enabled and sev_es_enabled off by default if
    CONFIG_KVM_AMD_SEV=n.  Previously, they could be on by default if
    CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT=y, regardless of KVM SEV
    support.

  - Hides the sev and sev_es module params when CONFIG_KVM_AMD_SEV=n.

  - Resolves a false positive -Wnonnull in __sev_recycle_asids() that is
    currently masked by the equivalent IS_ENABLED(CONFIG_KVM_AMD_SEV)
    check in svm_sev_enabled(), which will be dropped in a future patch.

Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index a024edabaca5..02a66008e9b9 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -28,12 +28,17 @@
 #define __ex(x) __kvm_handle_fault_on_reboot(x)
 
 /* enable/disable SEV support */
+#ifdef CONFIG_KVM_AMD_SEV
 static bool sev_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
 module_param_named(sev, sev_enabled, bool, 0444);
 
 /* enable/disable SEV-ES support */
 static bool sev_es_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
 module_param_named(sev_es, sev_es_enabled, bool, 0444);
+#else
+#define sev_enabled false
+#define sev_es_enabled false
+#endif /* CONFIG_KVM_AMD_SEV */
 
 static u8 sev_enc_bit;
 static int sev_flush_asids(void);
@@ -1253,11 +1258,12 @@ void sev_vm_destroy(struct kvm *kvm)
 
 void __init sev_hardware_setup(void)
 {
+#ifdef CONFIG_KVM_AMD_SEV
 	unsigned int eax, ebx, ecx, edx;
 	bool sev_es_supported = false;
 	bool sev_supported = false;
 
-	if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev_enabled)
+	if (!sev_enabled)
 		goto out;
 
 	/* Does the CPU support SEV? */
@@ -1311,6 +1317,7 @@ void __init sev_hardware_setup(void)
 out:
 	sev_enabled = sev_supported;
 	sev_es_enabled = sev_es_supported;
+#endif
 }
 
 void sev_hardware_teardown(void)
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 09/14] KVM: SVM: Unconditionally invoke sev_hardware_teardown()

[Sean Christopherson]

Remove the redundant svm_sev_enabled() check when calling
sev_hardware_teardown(), the teardown helper itself does the check.
Removing the check from svm.c will eventually allow dropping
svm_sev_enabled() entirely.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/svm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index f89f702b2a58..bb7b99743bea 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -887,8 +887,7 @@ static void svm_hardware_teardown(void)
 {
 	int cpu;
 
-	if (svm_sev_enabled())
-		sev_hardware_teardown();
+	sev_hardware_teardown();
 
 	for_each_possible_cpu(cpu)
 		svm_cpu_uninit(cpu);
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 10/14] KVM: SVM: Explicitly check max SEV ASID during sev_hardware_setup()

[Sean Christopherson]

Query max_sev_asid directly after setting it instead of bouncing through
its wrapper, svm_sev_enabled().  Using the wrapper is unnecessary
obfuscation.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 02a66008e9b9..1a143340103e 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1278,8 +1278,7 @@ void __init sev_hardware_setup(void)
 
 	/* Maximum number of encrypted guests supported simultaneously */
 	max_sev_asid = ecx;
-
-	if (!svm_sev_enabled())
+	if (!max_sev_asid)
 		goto out;
 
 	/* Minimum ASID value that should be used for SEV guest */
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 11/14] KVM: SVM: Move SEV VMCB tracking allocation to sev.c

[Sean Christopherson]

Move the allocation of the SEV VMCB array to sev.c to help pave the way
toward encapsulating SEV enabling wholly within sev.c.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 13 +++++++++++++
 arch/x86/kvm/svm/svm.c | 17 ++++++++---------
 arch/x86/kvm/svm/svm.h |  1 +
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 1a143340103e..a2c3e2d42a7f 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1330,6 +1330,19 @@ void sev_hardware_teardown(void)
 	sev_flush_asids();
 }
 
+int sev_cpu_init(struct svm_cpu_data *sd)
+{
+	if (!svm_sev_enabled())
+		return 0;
+
+	sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1, sizeof(void *),
+				      GFP_KERNEL | __GFP_ZERO);
+	if (!sd->sev_vmcbs)
+		return -ENOMEM;
+
+	return 0;
+}
+
 /*
  * Pages used by hardware to hold guest encrypted state must be flushed before
  * returning them to the system.
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index bb7b99743bea..89b95fb87a0c 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -552,23 +552,22 @@ static void svm_cpu_uninit(int cpu)
 static int svm_cpu_init(int cpu)
 {
 	struct svm_cpu_data *sd;
+	int ret;
 
 	sd = kzalloc(sizeof(struct svm_cpu_data), GFP_KERNEL);
 	if (!sd)
 		return -ENOMEM;
 	sd->cpu = cpu;
 	sd->save_area = alloc_page(GFP_KERNEL);
-	if (!sd->save_area)
+	if (!sd->save_area) {
+		ret = -ENOMEM;
 		goto free_cpu_data;
+	}
 	clear_page(page_address(sd->save_area));
 
-	if (svm_sev_enabled()) {
-		sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1,
-					      sizeof(void *),
-					      GFP_KERNEL | __GFP_ZERO);
-		if (!sd->sev_vmcbs)
-			goto free_save_area;
-	}
+	ret = sev_cpu_init(sd);
+	if (ret)
+		goto free_save_area;
 
 	per_cpu(svm_data, cpu) = sd;
 
@@ -578,7 +577,7 @@ static int svm_cpu_init(int cpu)
 	__free_page(sd->save_area);
 free_cpu_data:
 	kfree(sd);
-	return -ENOMEM;
+	return ret;
 
 }
 
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 8e169835f52a..4eb4bab0ca3e 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -583,6 +583,7 @@ int svm_unregister_enc_region(struct kvm *kvm,
 void pre_sev_run(struct vcpu_svm *svm, int cpu);
 void __init sev_hardware_setup(void);
 void sev_hardware_teardown(void);
+int sev_cpu_init(struct svm_cpu_data *sd);
 void sev_free_vcpu(struct kvm_vcpu *vcpu);
 int sev_handle_vmgexit(struct vcpu_svm *svm);
 int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in);
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 12/14] KVM: SVM: Drop redundant svm_sev_enabled() helper

[Sean Christopherson]

Replace calls to svm_sev_enabled() with direct checks on sev_enabled, or
in the case of svm_mem_enc_op, simply drop the call to svm_sev_enabled().
This effectively replaces checks against a valid max_sev_asid with checks
against sev_enabled.  sev_enabled is forced off by sev_hardware_setup()
if max_sev_asid is invalid, all call sites are guaranteed to run after
sev_hardware_setup(), and all of the checks care about SEV being fully
enabled (as opposed to intentionally handling the scenario where
max_sev_asid is valid but SEV enabling fails due to OOM).

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 6 +++---
 arch/x86/kvm/svm/svm.h | 5 -----
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index a2c3e2d42a7f..7e14514dd083 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1057,7 +1057,7 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	struct kvm_sev_cmd sev_cmd;
 	int r;
 
-	if (!svm_sev_enabled() || !sev_enabled)
+	if (!sev_enabled)
 		return -ENOTTY;
 
 	if (!argp)
@@ -1321,7 +1321,7 @@ void __init sev_hardware_setup(void)
 
 void sev_hardware_teardown(void)
 {
-	if (!svm_sev_enabled())
+	if (!sev_enabled)
 		return;
 
 	bitmap_free(sev_asid_bitmap);
@@ -1332,7 +1332,7 @@ void sev_hardware_teardown(void)
 
 int sev_cpu_init(struct svm_cpu_data *sd)
 {
-	if (!svm_sev_enabled())
+	if (!sev_enabled)
 		return 0;
 
 	sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1, sizeof(void *),
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 4eb4bab0ca3e..8cb4395b58a0 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -569,11 +569,6 @@ void svm_vcpu_unblocking(struct kvm_vcpu *vcpu);
 
 extern unsigned int max_sev_asid;
 
-static inline bool svm_sev_enabled(void)
-{
-	return IS_ENABLED(CONFIG_KVM_AMD_SEV) ? max_sev_asid : 0;
-}
-
 void sev_vm_destroy(struct kvm *kvm);
 int svm_mem_enc_op(struct kvm *kvm, void __user *argp);
 int svm_register_enc_region(struct kvm *kvm,
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 13/14] KVM: SVM: Remove an unnecessary prototype declaration of sev_flush_asids()

[Sean Christopherson]

Remove the forward declaration of sev_flush_asids(), which is only a few
lines above the function itself.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 7e14514dd083..23a4bead4a82 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -41,7 +41,6 @@ module_param_named(sev_es, sev_es_enabled, bool, 0444);
 #endif /* CONFIG_KVM_AMD_SEV */
 
 static u8 sev_enc_bit;
-static int sev_flush_asids(void);
 static DECLARE_RWSEM(sev_deactivate_lock);
 static DEFINE_MUTEX(sev_bitmap_lock);
 unsigned int max_sev_asid;
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH v2 14/14] KVM: SVM: Skip SEV cache flush if no ASIDs have been used

[Sean Christopherson]

Skip SEV's expensive WBINVD and DF_FLUSH if there are no SEV ASIDs
waiting to be reclaimed, e.g. if SEV was never used.  This "fixes" an
issue where the DF_FLUSH fails during hardware teardown if the original
SEV_INIT failed.  Ideally, SEV wouldn't be marked as enabled in KVM if
SEV_INIT fails, but that's a problem for another day.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 23a4bead4a82..e71bc742d8da 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -56,9 +56,14 @@ struct enc_region {
 	unsigned long size;
 };
 
-static int sev_flush_asids(void)
+static int sev_flush_asids(int min_asid, int max_asid)
 {
-	int ret, error = 0;
+	int ret, pos, error = 0;
+
+	/* Check if there are any ASIDs to reclaim before performing a flush */
+	pos = find_next_bit(sev_reclaim_asid_bitmap, max_sev_asid, min_asid);
+	if (pos >= max_asid)
+		return -EBUSY;
 
 	/*
 	 * DEACTIVATE will clear the WBINVD indicator causing DF_FLUSH to fail,
@@ -80,14 +85,7 @@ static int sev_flush_asids(void)
 /* Must be called with the sev_bitmap_lock held */
 static bool __sev_recycle_asids(int min_asid, int max_asid)
 {
-	int pos;
-
-	/* Check if there are any ASIDs to reclaim before performing a flush */
-	pos = find_next_bit(sev_reclaim_asid_bitmap, max_sev_asid, min_asid);
-	if (pos >= max_asid)
-		return false;
-
-	if (sev_flush_asids())
+	if (sev_flush_asids(min_asid, max_asid))
 		return false;
 
 	/* The flush process will flush all reclaimable SEV and SEV-ES ASIDs */
@@ -1323,10 +1321,10 @@ void sev_hardware_teardown(void)
 	if (!sev_enabled)
 		return;
 
+	sev_flush_asids(0, max_sev_asid);
+
 	bitmap_free(sev_asid_bitmap);
 	bitmap_free(sev_reclaim_asid_bitmap);
-
-	sev_flush_asids();
 }
 
 int sev_cpu_init(struct svm_cpu_data *sd)
-- 
2.30.0.284.gd98b1dd5eaa7-goog

Re: [PATCH v2 04/14] x86/cpufeatures: Assign dedicated feature word for AMD mem encryption

[Borislav Petkov]

On Wed, Jan 13, 2021 at 04:36:58PM -0800, Sean Christopherson wrote:
> Collect the scattered SME/SEV related feature flags into a dedicated
> word.  There are now five recognized features in CPUID.0x8000001F.EAX,
> with at least one more on the horizon (SEV-SNP).  Using a dedicated word
> allows KVM to use its automagic CPUID adjustment logic when reporting
> the set of supported features to userspace.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---

Subject should be:

x86/cpufeatures: Assign dedicated feature word for CPUID_0x8000001F[EAX]

but other than that, LGTM.

Anything against me taking it through tip now?

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg

Re: [PATCH v2 02/14] KVM: SVM: Free sev_asid_bitmap during init if SEV setup fails

[Tom Lendacky]

On 1/13/21 6:36 PM, Sean Christopherson wrote:
> Free sev_asid_bitmap if the reclaim bitmap allocation fails, othwerise
> KVM will unnecessarily keep the bitmap when SEV is not fully enabled.
> 
> Freeing the page is also necessary to avoid introducing a bug when a
> future patch eliminates svm_sev_enabled() in favor of using the global
> 'sev' flag directly.  While sev_hardware_enabled() checks max_sev_asid,
> which is true even if KVM setup fails, 'sev' will be true if and only
> if KVM setup fully succeeds.
> 
> Fixes: 33af3a7ef9e6 ("KVM: SVM: Reduce WBINVD/DF_FLUSH invocations")
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/svm/sev.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index c8ffdbc81709..0eeb6e1b803d 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1274,8 +1274,10 @@ void __init sev_hardware_setup(void)
>   		goto out;
>   
>   	sev_reclaim_asid_bitmap = bitmap_zalloc(max_sev_asid, GFP_KERNEL);
> -	if (!sev_reclaim_asid_bitmap)
> +	if (!sev_reclaim_asid_bitmap) {
> +		bitmap_free(sev_asid_bitmap);

Until that future change, you probably need to do sev_asid_bitmap = NULL 
here to avoid an issue in sev_hardware_teardown() when it tries to free it 
again.

Thanks,
Tom

>   		goto out;
> +	}
>   
>   	pr_info("SEV supported: %u ASIDs\n", max_sev_asid - min_sev_asid + 1);
>   	sev_supported = true;
> 

Re: [PATCH v2 01/14] KVM: SVM: Zero out the VMCB array used to track SEV ASID association

[Tom Lendacky]

On 1/13/21 6:36 PM, Sean Christopherson wrote:
> Zero out the array of VMCB pointers so that pre_sev_run() won't see
> garbage when querying the array to detect when an SEV ASID is being
> associated with a new VMCB.  In practice, reading random values is all
> but guaranteed to be benign as a false negative (which is extremely
> unlikely on its own) can only happen on CPU0 on the first VMRUN and would
> only cause KVM to skip the ASID flush.  For anything bad to happen, a
> previous instance of KVM would have to exit without flushing the ASID,
> _and_ KVM would have to not flush the ASID at any time while building the
> new SEV guest.
> 
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Fixes: 70cd94e60c73 ("KVM: SVM: VMRUN should use associated ASID when SEV is enabled")
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/svm/svm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 7ef171790d02..ccf52c5531fb 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -573,7 +573,7 @@ static int svm_cpu_init(int cpu)
>   	if (svm_sev_enabled()) {
>   		sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1,
>   					      sizeof(void *),
> -					      GFP_KERNEL);
> +					      GFP_KERNEL | __GFP_ZERO);

Alternatively, this call could just be changed to kcalloc().

Either way,

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

>   		if (!sd->sev_vmcbs)
>   			goto free_save_area;
>   	}
> 

Re: [PATCH v2 04/14] x86/cpufeatures: Assign dedicated feature word for AMD mem encryption

[Sean Christopherson]

On Thu, Jan 14, 2021, Borislav Petkov wrote:
> On Wed, Jan 13, 2021 at 04:36:58PM -0800, Sean Christopherson wrote:
> > Collect the scattered SME/SEV related feature flags into a dedicated
> > word.  There are now five recognized features in CPUID.0x8000001F.EAX,
> > with at least one more on the horizon (SEV-SNP).  Using a dedicated word
> > allows KVM to use its automagic CPUID adjustment logic when reporting
> > the set of supported features to userspace.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> 
> Subject should be:
> 
> x86/cpufeatures: Assign dedicated feature word for CPUID_0x8000001F[EAX]
> 
> but other than that, LGTM.
> 
> Anything against me taking it through tip now?

Hmm, patch 05/14 depends on the existence of the new word.  That's a non-issue
if you're planning on taking this for 5.11.  If it's destined for 5.12, maybe
get an ack from Paolo on patch 05 and take both through tip?  I can drop them
from this series when I send v3.  In hindsight, I should have split these two
patches into a separate mini-series from the get-go.

Thanks!

Re: [PATCH v2 02/14] KVM: SVM: Free sev_asid_bitmap during init if SEV setup fails

[Sean Christopherson]

On Thu, Jan 14, 2021, Tom Lendacky wrote:
> On 1/13/21 6:36 PM, Sean Christopherson wrote:
> > Free sev_asid_bitmap if the reclaim bitmap allocation fails, othwerise
> > KVM will unnecessarily keep the bitmap when SEV is not fully enabled.
> > 
> > Freeing the page is also necessary to avoid introducing a bug when a
> > future patch eliminates svm_sev_enabled() in favor of using the global
> > 'sev' flag directly.  While sev_hardware_enabled() checks max_sev_asid,
> > which is true even if KVM setup fails, 'sev' will be true if and only
> > if KVM setup fully succeeds.
> > 
> > Fixes: 33af3a7ef9e6 ("KVM: SVM: Reduce WBINVD/DF_FLUSH invocations")
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >   arch/x86/kvm/svm/sev.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index c8ffdbc81709..0eeb6e1b803d 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -1274,8 +1274,10 @@ void __init sev_hardware_setup(void)
> >   		goto out;
> >   	sev_reclaim_asid_bitmap = bitmap_zalloc(max_sev_asid, GFP_KERNEL);
> > -	if (!sev_reclaim_asid_bitmap)
> > +	if (!sev_reclaim_asid_bitmap) {
> > +		bitmap_free(sev_asid_bitmap);
> 
> Until that future change, you probably need to do sev_asid_bitmap = NULL
> here to avoid an issue in sev_hardware_teardown() when it tries to free it
> again.

Argh, you're right.  Thanks!

Re: [PATCH v2 01/14] KVM: SVM: Zero out the VMCB array used to track SEV ASID association

[Sean Christopherson]

On Thu, Jan 14, 2021, Tom Lendacky wrote:
> On 1/13/21 6:36 PM, Sean Christopherson wrote:
> > Zero out the array of VMCB pointers so that pre_sev_run() won't see
> > garbage when querying the array to detect when an SEV ASID is being
> > associated with a new VMCB.  In practice, reading random values is all
> > but guaranteed to be benign as a false negative (which is extremely
> > unlikely on its own) can only happen on CPU0 on the first VMRUN and would
> > only cause KVM to skip the ASID flush.  For anything bad to happen, a
> > previous instance of KVM would have to exit without flushing the ASID,
> > _and_ KVM would have to not flush the ASID at any time while building the
> > new SEV guest.
> > 
> > Cc: Borislav Petkov <bp@suse.de>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Cc: Brijesh Singh <brijesh.singh@amd.com>
> > Fixes: 70cd94e60c73 ("KVM: SVM: VMRUN should use associated ASID when SEV is enabled")
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >   arch/x86/kvm/svm/svm.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 7ef171790d02..ccf52c5531fb 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -573,7 +573,7 @@ static int svm_cpu_init(int cpu)
> >   	if (svm_sev_enabled()) {
> >   		sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1,
> >   					      sizeof(void *),
> > -					      GFP_KERNEL);
> > +					      GFP_KERNEL | __GFP_ZERO);
> 
> Alternatively, this call could just be changed to kcalloc().

Agreed, kcalloc() is a better option.  I'll do that in v3.

Re: [PATCH v2 04/14] x86/cpufeatures: Assign dedicated feature word for AMD mem encryption

[Borislav Petkov]

On Thu, Jan 14, 2021 at 09:09:28AM -0800, Sean Christopherson wrote:
> Hmm, patch 05/14 depends on the existence of the new word.  That's a non-issue
> if you're planning on taking this for 5.11.  If it's destined for 5.12, maybe
> get an ack from Paolo on patch 05 and take both through tip? 

Yeah, I guess that. Both are not urgent 5.11 material to take 'em now.
So I guess I'll wait for Paolo's ACK.

> I can drop them from this series when I send v3. In hindsight, I
> should have split these two patches into a separate mini-series from
> the get-go.

Nah, no worries. We do patch acrobatics on a daily basis. :-)

Thx.

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg

Re: [PATCH v2 06/14] x86/sev: Drop redundant and potentially misleading 'sev_enabled'

[Tom Lendacky]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Drop the sev_enabled flag and switch its one user over to sev_active().
> sev_enabled was made redundant with the introduction of sev_status in
> commit b57de6cd1639 ("x86/sev-es: Add SEV-ES Feature Detection").
> sev_enabled and sev_active() are guaranteed to be equivalent, as each is
> true iff 'sev_status & MSR_AMD64_SEV_ENABLED' is true, and are only ever
> written in tandem (ignoring compressed boot's version of sev_status).
> 
> Removing sev_enabled avoids confusion over whether it refers to the guest
> or the host, and will also allow KVM to usurp "sev_enabled" for its own
> purposes.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   arch/x86/include/asm/mem_encrypt.h |  1 -
>   arch/x86/mm/mem_encrypt.c          | 12 +++++-------
>   arch/x86/mm/mem_encrypt_identity.c |  1 -
>   3 files changed, 5 insertions(+), 9 deletions(-)
> 

Re: [PATCH v2 02/14] KVM: SVM: Free sev_asid_bitmap during init if SEV setup fails

[Tom Lendacky]

On 1/14/21 11:12 AM, Sean Christopherson wrote:
> On Thu, Jan 14, 2021, Tom Lendacky wrote:
>> On 1/13/21 6:36 PM, Sean Christopherson wrote:
>>> Free sev_asid_bitmap if the reclaim bitmap allocation fails, othwerise
>>> KVM will unnecessarily keep the bitmap when SEV is not fully enabled.
>>>
>>> Freeing the page is also necessary to avoid introducing a bug when a
>>> future patch eliminates svm_sev_enabled() in favor of using the global
>>> 'sev' flag directly.  While sev_hardware_enabled() checks max_sev_asid,
>>> which is true even if KVM setup fails, 'sev' will be true if and only
>>> if KVM setup fully succeeds.
>>>
>>> Fixes: 33af3a7ef9e6 ("KVM: SVM: Reduce WBINVD/DF_FLUSH invocations")

Oops, missed this last time... I don't think the Fixes: tag is needed 
anymore unless you don't want the memory consumption of the first bitmap, 
should the allocation of the second bitmap fail, until kvm_amd is 
rmmod'ed. Up to you.

Thanks,
Tom

>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>> Signed-off-by: Sean Christopherson <seanjc@google.com>
>>> ---
>>>    arch/x86/kvm/svm/sev.c | 4 +++-
>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>>> index c8ffdbc81709..0eeb6e1b803d 100644
>>> --- a/arch/x86/kvm/svm/sev.c
>>> +++ b/arch/x86/kvm/svm/sev.c
>>> @@ -1274,8 +1274,10 @@ void __init sev_hardware_setup(void)
>>>    		goto out;
>>>    	sev_reclaim_asid_bitmap = bitmap_zalloc(max_sev_asid, GFP_KERNEL);
>>> -	if (!sev_reclaim_asid_bitmap)
>>> +	if (!sev_reclaim_asid_bitmap) {
>>> +		bitmap_free(sev_asid_bitmap);
>>
>> Until that future change, you probably need to do sev_asid_bitmap = NULL
>> here to avoid an issue in sev_hardware_teardown() when it tries to free it
>> again.
> 
> Argh, you're right.  Thanks!
> 

Re: [PATCH v2 03/14] KVM: SVM: Move SEV module params/variables to sev.c

[Tom Lendacky]

On 1/13/21 6:36 PM, Sean Christopherson wrote:
> Unconditionally invoke sev_hardware_setup() when configuring SVM and
> handle clearing the module params/variable 'sev' and 'sev_es' in
> sev_hardware_setup().  This allows making said variables static within
> sev.c and reduces the odds of a collision with guest code, e.g. the guest
> side of things has already laid claim to 'sev_enabled'.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   arch/x86/kvm/svm/sev.c | 11 +++++++++++
>   arch/x86/kvm/svm/svm.c | 15 +--------------
>   arch/x86/kvm/svm/svm.h |  2 --
>   3 files changed, 12 insertions(+), 16 deletions(-)
> 

Re: [PATCH v2 02/14] KVM: SVM: Free sev_asid_bitmap during init if SEV setup fails

[Sean Christopherson]

On Thu, Jan 14, 2021, Tom Lendacky wrote:
> On 1/14/21 11:12 AM, Sean Christopherson wrote:
> > On Thu, Jan 14, 2021, Tom Lendacky wrote:
> > > On 1/13/21 6:36 PM, Sean Christopherson wrote:
> > > > Free sev_asid_bitmap if the reclaim bitmap allocation fails, othwerise
> > > > KVM will unnecessarily keep the bitmap when SEV is not fully enabled.
> > > > 
> > > > Freeing the page is also necessary to avoid introducing a bug when a
> > > > future patch eliminates svm_sev_enabled() in favor of using the global
> > > > 'sev' flag directly.  While sev_hardware_enabled() checks max_sev_asid,
> > > > which is true even if KVM setup fails, 'sev' will be true if and only
> > > > if KVM setup fully succeeds.
> > > > 
> > > > Fixes: 33af3a7ef9e6 ("KVM: SVM: Reduce WBINVD/DF_FLUSH invocations")
> 
> Oops, missed this last time... I don't think the Fixes: tag is needed
> anymore unless you don't want the memory consumption of the first bitmap,

If Fixes is viewed as purely a "this needs to be backported", then yes, it
should be dropped.  But, since KVM policy is to backport only patches that are
explicitly tagged with stable@, I like to use to Fixes to create a paper trail
for bug fixes even if the bug is essentially benign.

That being said, I have no objection to dropping it if anyone feels strongly
about not playing fast and loose with Fixes.

> should the allocation of the second bitmap fail, until kvm_amd is rmmod'ed.
> Up to you.

Re: [PATCH v2 08/14] KVM: SVM: Condition sev_enabled and sev_es_enabled on CONFIG_KVM_AMD_SEV=y

[Tom Lendacky]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Define sev_enabled and sev_es_enabled as 'false' and explicitly #ifdef
> out all of sev_hardware_setup() if CONFIG_KVM_AMD_SEV=n.  This kills
> three birds at once:
> 
>    - Makes sev_enabled and sev_es_enabled off by default if
>      CONFIG_KVM_AMD_SEV=n.  Previously, they could be on by default if
>      CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT=y, regardless of KVM SEV
>      support.
> 
>    - Hides the sev and sev_es module params when CONFIG_KVM_AMD_SEV=n.
> 
>    - Resolves a false positive -Wnonnull in __sev_recycle_asids() that is
>      currently masked by the equivalent IS_ENABLED(CONFIG_KVM_AMD_SEV)
>      check in svm_sev_enabled(), which will be dropped in a future patch.
> 
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   arch/x86/kvm/svm/sev.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index a024edabaca5..02a66008e9b9 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -28,12 +28,17 @@
>   #define __ex(x) __kvm_handle_fault_on_reboot(x)
>   
>   /* enable/disable SEV support */
> +#ifdef CONFIG_KVM_AMD_SEV
>   static bool sev_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
>   module_param_named(sev, sev_enabled, bool, 0444);
>   
>   /* enable/disable SEV-ES support */
>   static bool sev_es_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
>   module_param_named(sev_es, sev_es_enabled, bool, 0444);
> +#else
> +#define sev_enabled false
> +#define sev_es_enabled false
> +#endif /* CONFIG_KVM_AMD_SEV */
>   
>   static u8 sev_enc_bit;
>   static int sev_flush_asids(void);
> @@ -1253,11 +1258,12 @@ void sev_vm_destroy(struct kvm *kvm)
>   
>   void __init sev_hardware_setup(void)
>   {
> +#ifdef CONFIG_KVM_AMD_SEV
>   	unsigned int eax, ebx, ecx, edx;
>   	bool sev_es_supported = false;
>   	bool sev_supported = false;
>   
> -	if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev_enabled)
> +	if (!sev_enabled)
>   		goto out;
>   
>   	/* Does the CPU support SEV? */
> @@ -1311,6 +1317,7 @@ void __init sev_hardware_setup(void)
>   out:
>   	sev_enabled = sev_supported;
>   	sev_es_enabled = sev_es_supported;
> +#endif
>   }
>   
>   void sev_hardware_teardown(void)
> 

Re: [PATCH v2 01/14] KVM: SVM: Zero out the VMCB array used to track SEV ASID association

[Brijesh Singh]

On 1/13/21 6:36 PM, Sean Christopherson wrote:
> Zero out the array of VMCB pointers so that pre_sev_run() won't see
> garbage when querying the array to detect when an SEV ASID is being
> associated with a new VMCB.  In practice, reading random values is all
> but guaranteed to be benign as a false negative (which is extremely
> unlikely on its own) can only happen on CPU0 on the first VMRUN and would
> only cause KVM to skip the ASID flush.  For anything bad to happen, a
> previous instance of KVM would have to exit without flushing the ASID,
> _and_ KVM would have to not flush the ASID at any time while building the
> new SEV guest.
>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Fixes: 70cd94e60c73 ("KVM: SVM: VMRUN should use associated ASID when SEV is enabled")
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/svm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 7ef171790d02..ccf52c5531fb 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -573,7 +573,7 @@ static int svm_cpu_init(int cpu)
>  	if (svm_sev_enabled()) {
>  		sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1,
>  					      sizeof(void *),
> -					      GFP_KERNEL);
> +					      GFP_KERNEL | __GFP_ZERO);
>  		if (!sd->sev_vmcbs)
>  			goto free_save_area;
>  	}


Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>

Re: [PATCH v2 03/14] KVM: SVM: Move SEV module params/variables to sev.c

[Brijesh Singh]

On 1/13/21 6:36 PM, Sean Christopherson wrote:
> Unconditionally invoke sev_hardware_setup() when configuring SVM and
> handle clearing the module params/variable 'sev' and 'sev_es' in
> sev_hardware_setup().  This allows making said variables static within
> sev.c and reduces the odds of a collision with guest code, e.g. the guest
> side of things has already laid claim to 'sev_enabled'.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/sev.c | 11 +++++++++++
>  arch/x86/kvm/svm/svm.c | 15 +--------------
>  arch/x86/kvm/svm/svm.h |  2 --
>  3 files changed, 12 insertions(+), 16 deletions(-)


Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>


>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 0eeb6e1b803d..8ba93b8fa435 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -27,6 +27,14 @@
>  
>  #define __ex(x) __kvm_handle_fault_on_reboot(x)
>  
> +/* enable/disable SEV support */
> +static int sev = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
> +module_param(sev, int, 0444);
> +
> +/* enable/disable SEV-ES support */
> +static int sev_es = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
> +module_param(sev_es, int, 0444);
> +
>  static u8 sev_enc_bit;
>  static int sev_flush_asids(void);
>  static DECLARE_RWSEM(sev_deactivate_lock);
> @@ -1249,6 +1257,9 @@ void __init sev_hardware_setup(void)
>  	bool sev_es_supported = false;
>  	bool sev_supported = false;
>  
> +	if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev)
> +		goto out;
> +
>  	/* Does the CPU support SEV? */
>  	if (!boot_cpu_has(X86_FEATURE_SEV))
>  		goto out;
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index ccf52c5531fb..f89f702b2a58 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -189,14 +189,6 @@ module_param(vls, int, 0444);
>  static int vgif = true;
>  module_param(vgif, int, 0444);
>  
> -/* enable/disable SEV support */
> -int sev = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
> -module_param(sev, int, 0444);
> -
> -/* enable/disable SEV-ES support */
> -int sev_es = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
> -module_param(sev_es, int, 0444);
> -
>  bool __read_mostly dump_invalid_vmcb;
>  module_param(dump_invalid_vmcb, bool, 0644);
>  
> @@ -976,12 +968,7 @@ static __init int svm_hardware_setup(void)
>  		kvm_enable_efer_bits(EFER_SVME | EFER_LMSLE);
>  	}
>  
> -	if (IS_ENABLED(CONFIG_KVM_AMD_SEV) && sev) {
> -		sev_hardware_setup();
> -	} else {
> -		sev = false;
> -		sev_es = false;
> -	}
> +	sev_hardware_setup();
>  
>  	svm_adjust_mmio_mask();
>  
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 0fe874ae5498..8e169835f52a 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -408,8 +408,6 @@ static inline bool gif_set(struct vcpu_svm *svm)
>  #define MSR_CR3_LONG_MBZ_MASK			0xfff0000000000000U
>  #define MSR_INVALID				0xffffffffU
>  
> -extern int sev;
> -extern int sev_es;
>  extern bool dump_invalid_vmcb;
>  
>  u32 svm_msrpm_offset(u32 msr);

Re: [PATCH v2 04/14] x86/cpufeatures: Assign dedicated feature word for AMD mem encryption

[Brijesh Singh]

On 1/13/21 6:36 PM, Sean Christopherson wrote:
> Collect the scattered SME/SEV related feature flags into a dedicated
> word.  There are now five recognized features in CPUID.0x8000001F.EAX,
> with at least one more on the horizon (SEV-SNP).  Using a dedicated word
> allows KVM to use its automagic CPUID adjustment logic when reporting
> the set of supported features to userspace.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/cpufeature.h              |  7 +++++--
>  arch/x86/include/asm/cpufeatures.h             | 17 +++++++++++------
>  arch/x86/include/asm/disabled-features.h       |  3 ++-
>  arch/x86/include/asm/required-features.h       |  3 ++-
>  arch/x86/kernel/cpu/common.c                   |  3 +++
>  arch/x86/kernel/cpu/scattered.c                |  5 -----
>  tools/arch/x86/include/asm/disabled-features.h |  3 ++-
>  tools/arch/x86/include/asm/required-features.h |  3 ++-
>  8 files changed, 27 insertions(+), 17 deletions(-)

Thanks

Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>

>
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 59bf91c57aa8..1728d4ce5730 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -30,6 +30,7 @@ enum cpuid_leafs
>  	CPUID_7_ECX,
>  	CPUID_8000_0007_EBX,
>  	CPUID_7_EDX,
> +	CPUID_8000_001F_EAX,
>  };
>  
>  #ifdef CONFIG_X86_FEATURE_NAMES
> @@ -88,8 +89,9 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
>  	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 16, feature_bit) ||	\
>  	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 17, feature_bit) ||	\
>  	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 18, feature_bit) ||	\
> +	   CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 19, feature_bit) ||	\
>  	   REQUIRED_MASK_CHECK					  ||	\
> -	   BUILD_BUG_ON_ZERO(NCAPINTS != 19))
> +	   BUILD_BUG_ON_ZERO(NCAPINTS != 20))
>  
>  #define DISABLED_MASK_BIT_SET(feature_bit)				\
>  	 ( CHECK_BIT_IN_MASK_WORD(DISABLED_MASK,  0, feature_bit) ||	\
> @@ -111,8 +113,9 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
>  	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 16, feature_bit) ||	\
>  	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 17, feature_bit) ||	\
>  	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 18, feature_bit) ||	\
> +	   CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 19, feature_bit) ||	\
>  	   DISABLED_MASK_CHECK					  ||	\
> -	   BUILD_BUG_ON_ZERO(NCAPINTS != 19))
> +	   BUILD_BUG_ON_ZERO(NCAPINTS != 20))
>  
>  #define cpu_has(c, bit)							\
>  	(__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 :	\
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 9f9e9511f7cd..7c0bb1a20050 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -13,7 +13,7 @@
>  /*
>   * Defines x86 CPU feature bits
>   */
> -#define NCAPINTS			19	   /* N 32-bit words worth of info */
> +#define NCAPINTS			20	   /* N 32-bit words worth of info */
>  #define NBUGINTS			1	   /* N 32-bit bug flags */
>  
>  /*
> @@ -96,7 +96,7 @@
>  #define X86_FEATURE_SYSCALL32		( 3*32+14) /* "" syscall in IA32 userspace */
>  #define X86_FEATURE_SYSENTER32		( 3*32+15) /* "" sysenter in IA32 userspace */
>  #define X86_FEATURE_REP_GOOD		( 3*32+16) /* REP microcode works well */
> -#define X86_FEATURE_SME_COHERENT	( 3*32+17) /* "" AMD hardware-enforced cache coherency */
> +/* FREE!                                ( 3*32+17) */
>  #define X86_FEATURE_LFENCE_RDTSC	( 3*32+18) /* "" LFENCE synchronizes RDTSC */
>  #define X86_FEATURE_ACC_POWER		( 3*32+19) /* AMD Accumulated Power Mechanism */
>  #define X86_FEATURE_NOPL		( 3*32+20) /* The NOPL (0F 1F) instructions */
> @@ -201,7 +201,7 @@
>  #define X86_FEATURE_INVPCID_SINGLE	( 7*32+ 7) /* Effectively INVPCID && CR4.PCIDE=1 */
>  #define X86_FEATURE_HW_PSTATE		( 7*32+ 8) /* AMD HW-PState */
>  #define X86_FEATURE_PROC_FEEDBACK	( 7*32+ 9) /* AMD ProcFeedbackInterface */
> -#define X86_FEATURE_SME			( 7*32+10) /* AMD Secure Memory Encryption */
> +/* FREE!                                ( 7*32+10) */
>  #define X86_FEATURE_PTI			( 7*32+11) /* Kernel Page Table Isolation enabled */
>  #define X86_FEATURE_RETPOLINE		( 7*32+12) /* "" Generic Retpoline mitigation for Spectre variant 2 */
>  #define X86_FEATURE_RETPOLINE_AMD	( 7*32+13) /* "" AMD Retpoline mitigation for Spectre variant 2 */
> @@ -211,7 +211,7 @@
>  #define X86_FEATURE_SSBD		( 7*32+17) /* Speculative Store Bypass Disable */
>  #define X86_FEATURE_MBA			( 7*32+18) /* Memory Bandwidth Allocation */
>  #define X86_FEATURE_RSB_CTXSW		( 7*32+19) /* "" Fill RSB on context switches */
> -#define X86_FEATURE_SEV			( 7*32+20) /* AMD Secure Encrypted Virtualization */
> +/* FREE!                                ( 7*32+20) */
>  #define X86_FEATURE_USE_IBPB		( 7*32+21) /* "" Indirect Branch Prediction Barrier enabled */
>  #define X86_FEATURE_USE_IBRS_FW		( 7*32+22) /* "" Use IBRS during runtime firmware calls */
>  #define X86_FEATURE_SPEC_STORE_BYPASS_DISABLE	( 7*32+23) /* "" Disable Speculative Store Bypass. */
> @@ -236,8 +236,6 @@
>  #define X86_FEATURE_EPT_AD		( 8*32+17) /* Intel Extended Page Table access-dirty bit */
>  #define X86_FEATURE_VMCALL		( 8*32+18) /* "" Hypervisor supports the VMCALL instruction */
>  #define X86_FEATURE_VMW_VMMCALL		( 8*32+19) /* "" VMware prefers VMMCALL hypercall instruction */
> -#define X86_FEATURE_SEV_ES		( 8*32+20) /* AMD Secure Encrypted Virtualization - Encrypted State */
> -#define X86_FEATURE_VM_PAGE_FLUSH	( 8*32+21) /* "" VM Page Flush MSR is supported */
>  
>  /* Intel-defined CPU features, CPUID level 0x00000007:0 (EBX), word 9 */
>  #define X86_FEATURE_FSGSBASE		( 9*32+ 0) /* RDFSBASE, WRFSBASE, RDGSBASE, WRGSBASE instructions*/
> @@ -383,6 +381,13 @@
>  #define X86_FEATURE_CORE_CAPABILITIES	(18*32+30) /* "" IA32_CORE_CAPABILITIES MSR */
>  #define X86_FEATURE_SPEC_CTRL_SSBD	(18*32+31) /* "" Speculative Store Bypass Disable */
>  
> +/* AMD-defined memory encryption features, CPUID level 0x8000001f (EAX), word 19 */
> +#define X86_FEATURE_SME			(19*32+ 0) /* AMD Secure Memory Encryption */
> +#define X86_FEATURE_SEV			(19*32+ 1) /* AMD Secure Encrypted Virtualization */
> +#define X86_FEATURE_VM_PAGE_FLUSH	(19*32+ 2) /* "" VM Page Flush MSR is supported */
> +#define X86_FEATURE_SEV_ES		(19*32+ 3) /* AMD Secure Encrypted Virtualization - Encrypted State */
> +#define X86_FEATURE_SME_COHERENT	(19*32+10) /* "" AMD hardware-enforced cache coherency */
> +
>  /*
>   * BUG word(s)
>   */
> diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
> index 5861d34f9771..2216077676c8 100644
> --- a/arch/x86/include/asm/disabled-features.h
> +++ b/arch/x86/include/asm/disabled-features.h
> @@ -85,6 +85,7 @@
>  			 DISABLE_ENQCMD)
>  #define DISABLED_MASK17	0
>  #define DISABLED_MASK18	0
> -#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
> +#define DISABLED_MASK19	0
> +#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
>  
>  #endif /* _ASM_X86_DISABLED_FEATURES_H */
> diff --git a/arch/x86/include/asm/required-features.h b/arch/x86/include/asm/required-features.h
> index 3ff0d48469f2..b2d504f11937 100644
> --- a/arch/x86/include/asm/required-features.h
> +++ b/arch/x86/include/asm/required-features.h
> @@ -101,6 +101,7 @@
>  #define REQUIRED_MASK16	0
>  #define REQUIRED_MASK17	0
>  #define REQUIRED_MASK18	0
> -#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
> +#define REQUIRED_MASK19	0
> +#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
>  
>  #endif /* _ASM_X86_REQUIRED_FEATURES_H */
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 35ad8480c464..9215b91bc044 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -960,6 +960,9 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
>  	if (c->extended_cpuid_level >= 0x8000000a)
>  		c->x86_capability[CPUID_8000_000A_EDX] = cpuid_edx(0x8000000a);
>  
> +	if (c->extended_cpuid_level >= 0x8000001f)
> +		c->x86_capability[CPUID_8000_001F_EAX] = cpuid_eax(0x8000001f);
> +
>  	init_scattered_cpuid_features(c);
>  	init_speculation_control(c);
>  
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index 236924930bf0..972ec3bfa9c0 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -40,11 +40,6 @@ static const struct cpuid_bit cpuid_bits[] = {
>  	{ X86_FEATURE_CPB,		CPUID_EDX,  9, 0x80000007, 0 },
>  	{ X86_FEATURE_PROC_FEEDBACK,    CPUID_EDX, 11, 0x80000007, 0 },
>  	{ X86_FEATURE_MBA,		CPUID_EBX,  6, 0x80000008, 0 },
> -	{ X86_FEATURE_SME,		CPUID_EAX,  0, 0x8000001f, 0 },
> -	{ X86_FEATURE_SEV,		CPUID_EAX,  1, 0x8000001f, 0 },
> -	{ X86_FEATURE_SEV_ES,		CPUID_EAX,  3, 0x8000001f, 0 },
> -	{ X86_FEATURE_SME_COHERENT,	CPUID_EAX, 10, 0x8000001f, 0 },
> -	{ X86_FEATURE_VM_PAGE_FLUSH,	CPUID_EAX,  2, 0x8000001f, 0 },
>  	{ 0, 0, 0, 0, 0 }
>  };
>  
> diff --git a/tools/arch/x86/include/asm/disabled-features.h b/tools/arch/x86/include/asm/disabled-features.h
> index 5861d34f9771..2216077676c8 100644
> --- a/tools/arch/x86/include/asm/disabled-features.h
> +++ b/tools/arch/x86/include/asm/disabled-features.h
> @@ -85,6 +85,7 @@
>  			 DISABLE_ENQCMD)
>  #define DISABLED_MASK17	0
>  #define DISABLED_MASK18	0
> -#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
> +#define DISABLED_MASK19	0
> +#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
>  
>  #endif /* _ASM_X86_DISABLED_FEATURES_H */
> diff --git a/tools/arch/x86/include/asm/required-features.h b/tools/arch/x86/include/asm/required-features.h
> index 3ff0d48469f2..b2d504f11937 100644
> --- a/tools/arch/x86/include/asm/required-features.h
> +++ b/tools/arch/x86/include/asm/required-features.h
> @@ -101,6 +101,7 @@
>  #define REQUIRED_MASK16	0
>  #define REQUIRED_MASK17	0
>  #define REQUIRED_MASK18	0
> -#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)
> +#define REQUIRED_MASK19	0
> +#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
>  
>  #endif /* _ASM_X86_REQUIRED_FEATURES_H */

Re: [PATCH v2 05/14] KVM: x86: Override reported SME/SEV feature flags with host mask

[Brijesh Singh]

On 1/13/21 6:36 PM, Sean Christopherson wrote:
> Add a reverse-CPUID entry for the memory encryption word, 0x8000001F.EAX,
> and use it to override the supported CPUID flags reported to userspace.
> Masking the reported CPUID flags avoids over-reporting KVM support, e.g.
> without the mask a SEV-SNP capable CPU may incorrectly advertise SNP
> support to userspace.
>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/cpuid.c | 2 ++
>  arch/x86/kvm/cpuid.h | 1 +
>  2 files changed, 3 insertions(+)

thanks

Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>

>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 13036cf0b912..b7618cdd06b5 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -855,6 +855,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>  	case 0x8000001F:
>  		if (!boot_cpu_has(X86_FEATURE_SEV))
>  			entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
> +		else
> +			cpuid_entry_override(entry, CPUID_8000_001F_EAX);
>  		break;
>  	/*Add support for Centaur's CPUID instruction*/
>  	case 0xC0000000:
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index dc921d76e42e..8b6fc9bde248 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -63,6 +63,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
>  	[CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX},
>  	[CPUID_7_EDX]         = {         7, 0, CPUID_EDX},
>  	[CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
> +	[CPUID_8000_001F_EAX] = {0x8000001f, 1, CPUID_EAX},
>  };
>  
>  /*

Re: [PATCH v2 06/14] x86/sev: Drop redundant and potentially misleading 'sev_enabled'

[Brijesh Singh]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Drop the sev_enabled flag and switch its one user over to sev_active().
> sev_enabled was made redundant with the introduction of sev_status in
> commit b57de6cd1639 ("x86/sev-es: Add SEV-ES Feature Detection").
> sev_enabled and sev_active() are guaranteed to be equivalent, as each is
> true iff 'sev_status & MSR_AMD64_SEV_ENABLED' is true, and are only ever
> written in tandem (ignoring compressed boot's version of sev_status).
>
> Removing sev_enabled avoids confusion over whether it refers to the guest
> or the host, and will also allow KVM to usurp "sev_enabled" for its own
> purposes.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/mem_encrypt.h |  1 -
>  arch/x86/mm/mem_encrypt.c          | 12 +++++-------
>  arch/x86/mm/mem_encrypt_identity.c |  1 -
>  3 files changed, 5 insertions(+), 9 deletions(-)

Thanks

Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>

> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> index 2f62bbdd9d12..88d624499411 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -20,7 +20,6 @@
>  
>  extern u64 sme_me_mask;
>  extern u64 sev_status;
> -extern bool sev_enabled;
>  
>  void sme_encrypt_execute(unsigned long encrypted_kernel_vaddr,
>  			 unsigned long decrypted_kernel_vaddr,
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index bc0833713be9..b89bc03c63a2 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -44,8 +44,6 @@ EXPORT_SYMBOL(sme_me_mask);
>  DEFINE_STATIC_KEY_FALSE(sev_enable_key);
>  EXPORT_SYMBOL_GPL(sev_enable_key);
>  
> -bool sev_enabled __section(".data");
> -
>  /* Buffer used for early in-place encryption by BSP, no locking needed */
>  static char sme_early_buffer[PAGE_SIZE] __initdata __aligned(PAGE_SIZE);
>  
> @@ -342,16 +340,16 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size)
>   * up under SME the trampoline area cannot be encrypted, whereas under SEV
>   * the trampoline area must be encrypted.
>   */
> -bool sme_active(void)
> -{
> -	return sme_me_mask && !sev_enabled;
> -}
> -
>  bool sev_active(void)
>  {
>  	return sev_status & MSR_AMD64_SEV_ENABLED;
>  }
>  
> +bool sme_active(void)
> +{
> +	return sme_me_mask && !sev_active();
> +}
> +
>  /* Needs to be called from non-instrumentable code */
>  bool noinstr sev_es_active(void)
>  {
> diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c
> index 6c5eb6f3f14f..0c2759b7f03a 100644
> --- a/arch/x86/mm/mem_encrypt_identity.c
> +++ b/arch/x86/mm/mem_encrypt_identity.c
> @@ -545,7 +545,6 @@ void __init sme_enable(struct boot_params *bp)
>  
>  		/* SEV state cannot be controlled by a command line option */
>  		sme_me_mask = me_mask;
> -		sev_enabled = true;
>  		physical_mask &= ~sme_me_mask;
>  		return;
>  	}

Re: [PATCH v2 09/14] KVM: SVM: Unconditionally invoke sev_hardware_teardown()

[Tom Lendacky]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Remove the redundant svm_sev_enabled() check when calling
> sev_hardware_teardown(), the teardown helper itself does the check.
> Removing the check from svm.c will eventually allow dropping
> svm_sev_enabled() entirely.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   arch/x86/kvm/svm/svm.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index f89f702b2a58..bb7b99743bea 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -887,8 +887,7 @@ static void svm_hardware_teardown(void)
>   {
>   	int cpu;
>   
> -	if (svm_sev_enabled())
> -		sev_hardware_teardown();
> +	sev_hardware_teardown();
>   
>   	for_each_possible_cpu(cpu)
>   		svm_cpu_uninit(cpu);
> 

Re: [PATCH v2 07/14] KVM: SVM: Append "_enabled" to module-scoped SEV/SEV-ES control variables

[Brijesh Singh]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Rename sev and sev_es to sev_enabled and sev_es_enabled respectively to
> better align with other KVM terminology, and to avoid pseudo-shadowing
> when the variables are moved to sev.c in a future patch ('sev' is often
> used for local struct kvm_sev_info pointers.
>
> No functional change intended.
>
> Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/sev.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

thanks

Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>


>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 8ba93b8fa435..a024edabaca5 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -28,12 +28,12 @@
>  #define __ex(x) __kvm_handle_fault_on_reboot(x)
>  
>  /* enable/disable SEV support */
> -static int sev = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
> -module_param(sev, int, 0444);
> +static bool sev_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
> +module_param_named(sev, sev_enabled, bool, 0444);
>  
>  /* enable/disable SEV-ES support */
> -static int sev_es = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
> -module_param(sev_es, int, 0444);
> +static bool sev_es_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
> +module_param_named(sev_es, sev_es_enabled, bool, 0444);
>  
>  static u8 sev_enc_bit;
>  static int sev_flush_asids(void);
> @@ -213,7 +213,7 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  
>  static int sev_es_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  {
> -	if (!sev_es)
> +	if (!sev_es_enabled)
>  		return -ENOTTY;
>  
>  	to_kvm_svm(kvm)->sev_info.es_active = true;
> @@ -1052,7 +1052,7 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
>  	struct kvm_sev_cmd sev_cmd;
>  	int r;
>  
> -	if (!svm_sev_enabled() || !sev)
> +	if (!svm_sev_enabled() || !sev_enabled)
>  		return -ENOTTY;
>  
>  	if (!argp)
> @@ -1257,7 +1257,7 @@ void __init sev_hardware_setup(void)
>  	bool sev_es_supported = false;
>  	bool sev_supported = false;
>  
> -	if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev)
> +	if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev_enabled)
>  		goto out;
>  
>  	/* Does the CPU support SEV? */
> @@ -1294,7 +1294,7 @@ void __init sev_hardware_setup(void)
>  	sev_supported = true;
>  
>  	/* SEV-ES support requested? */
> -	if (!sev_es)
> +	if (!sev_es_enabled)
>  		goto out;
>  
>  	/* Does the CPU support SEV-ES? */
> @@ -1309,8 +1309,8 @@ void __init sev_hardware_setup(void)
>  	sev_es_supported = true;
>  
>  out:
> -	sev = sev_supported;
> -	sev_es = sev_es_supported;
> +	sev_enabled = sev_supported;
> +	sev_es_enabled = sev_es_supported;
>  }
>  
>  void sev_hardware_teardown(void)

Re: [PATCH v2 08/14] KVM: SVM: Condition sev_enabled and sev_es_enabled on CONFIG_KVM_AMD_SEV=y

[Brijesh Singh]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Define sev_enabled and sev_es_enabled as 'false' and explicitly #ifdef
> out all of sev_hardware_setup() if CONFIG_KVM_AMD_SEV=n.  This kills
> three birds at once:
>
>   - Makes sev_enabled and sev_es_enabled off by default if
>     CONFIG_KVM_AMD_SEV=n.  Previously, they could be on by default if
>     CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT=y, regardless of KVM SEV
>     support.
>
>   - Hides the sev and sev_es module params when CONFIG_KVM_AMD_SEV=n.
>
>   - Resolves a false positive -Wnonnull in __sev_recycle_asids() that is
>     currently masked by the equivalent IS_ENABLED(CONFIG_KVM_AMD_SEV)
>     check in svm_sev_enabled(), which will be dropped in a future patch.
>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/sev.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

thanks

Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>


>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index a024edabaca5..02a66008e9b9 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -28,12 +28,17 @@
>  #define __ex(x) __kvm_handle_fault_on_reboot(x)
>  
>  /* enable/disable SEV support */
> +#ifdef CONFIG_KVM_AMD_SEV
>  static bool sev_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
>  module_param_named(sev, sev_enabled, bool, 0444);
>  
>  /* enable/disable SEV-ES support */
>  static bool sev_es_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
>  module_param_named(sev_es, sev_es_enabled, bool, 0444);
> +#else
> +#define sev_enabled false
> +#define sev_es_enabled false
> +#endif /* CONFIG_KVM_AMD_SEV */
>  
>  static u8 sev_enc_bit;
>  static int sev_flush_asids(void);
> @@ -1253,11 +1258,12 @@ void sev_vm_destroy(struct kvm *kvm)
>  
>  void __init sev_hardware_setup(void)
>  {
> +#ifdef CONFIG_KVM_AMD_SEV
>  	unsigned int eax, ebx, ecx, edx;
>  	bool sev_es_supported = false;
>  	bool sev_supported = false;
>  
> -	if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev_enabled)
> +	if (!sev_enabled)
>  		goto out;
>  
>  	/* Does the CPU support SEV? */
> @@ -1311,6 +1317,7 @@ void __init sev_hardware_setup(void)
>  out:
>  	sev_enabled = sev_supported;
>  	sev_es_enabled = sev_es_supported;
> +#endif
>  }
>  
>  void sev_hardware_teardown(void)

Re: [PATCH v2 09/14] KVM: SVM: Unconditionally invoke sev_hardware_teardown()

[Brijesh Singh]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Remove the redundant svm_sev_enabled() check when calling
> sev_hardware_teardown(), the teardown helper itself does the check.
> Removing the check from svm.c will eventually allow dropping
> svm_sev_enabled() entirely.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/svm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)


Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>


>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index f89f702b2a58..bb7b99743bea 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -887,8 +887,7 @@ static void svm_hardware_teardown(void)
>  {
>  	int cpu;
>  
> -	if (svm_sev_enabled())
> -		sev_hardware_teardown();
> +	sev_hardware_teardown();
>  
>  	for_each_possible_cpu(cpu)
>  		svm_cpu_uninit(cpu);

Re: [PATCH v2 10/14] KVM: SVM: Explicitly check max SEV ASID during sev_hardware_setup()

[Brijesh Singh]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Query max_sev_asid directly after setting it instead of bouncing through
> its wrapper, svm_sev_enabled().  Using the wrapper is unnecessary
> obfuscation.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/sev.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

thanks

Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>

>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 02a66008e9b9..1a143340103e 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1278,8 +1278,7 @@ void __init sev_hardware_setup(void)
>  
>  	/* Maximum number of encrypted guests supported simultaneously */
>  	max_sev_asid = ecx;
> -
> -	if (!svm_sev_enabled())
> +	if (!max_sev_asid)
>  		goto out;
>  
>  	/* Minimum ASID value that should be used for SEV guest */

Re: [PATCH v2 11/14] KVM: SVM: Move SEV VMCB tracking allocation to sev.c

[Brijesh Singh]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Move the allocation of the SEV VMCB array to sev.c to help pave the way
> toward encapsulating SEV enabling wholly within sev.c.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/sev.c | 13 +++++++++++++
>  arch/x86/kvm/svm/svm.c | 17 ++++++++---------
>  arch/x86/kvm/svm/svm.h |  1 +
>  3 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 1a143340103e..a2c3e2d42a7f 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1330,6 +1330,19 @@ void sev_hardware_teardown(void)
>  	sev_flush_asids();
>  }
>  
> +int sev_cpu_init(struct svm_cpu_data *sd)
> +{
> +	if (!svm_sev_enabled())
> +		return 0;
> +
> +	sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1, sizeof(void *),
> +				      GFP_KERNEL | __GFP_ZERO);


I saw Tom recommended to use kzalloc.. instead of __GFP_ZERO in previous
patch. With that fixed,

Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>


> +	if (!sd->sev_vmcbs)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
>  /*
>   * Pages used by hardware to hold guest encrypted state must be flushed before
>   * returning them to the system.
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index bb7b99743bea..89b95fb87a0c 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -552,23 +552,22 @@ static void svm_cpu_uninit(int cpu)
>  static int svm_cpu_init(int cpu)
>  {
>  	struct svm_cpu_data *sd;
> +	int ret;
>  
>  	sd = kzalloc(sizeof(struct svm_cpu_data), GFP_KERNEL);
>  	if (!sd)
>  		return -ENOMEM;
>  	sd->cpu = cpu;
>  	sd->save_area = alloc_page(GFP_KERNEL);
> -	if (!sd->save_area)
> +	if (!sd->save_area) {
> +		ret = -ENOMEM;
>  		goto free_cpu_data;
> +	}
>  	clear_page(page_address(sd->save_area));
>  
> -	if (svm_sev_enabled()) {
> -		sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1,
> -					      sizeof(void *),
> -					      GFP_KERNEL | __GFP_ZERO);
> -		if (!sd->sev_vmcbs)
> -			goto free_save_area;
> -	}
> +	ret = sev_cpu_init(sd);
> +	if (ret)
> +		goto free_save_area;
>  
>  	per_cpu(svm_data, cpu) = sd;
>  
> @@ -578,7 +577,7 @@ static int svm_cpu_init(int cpu)
>  	__free_page(sd->save_area);
>  free_cpu_data:
>  	kfree(sd);
> -	return -ENOMEM;
> +	return ret;
>  
>  }
>  
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 8e169835f52a..4eb4bab0ca3e 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -583,6 +583,7 @@ int svm_unregister_enc_region(struct kvm *kvm,
>  void pre_sev_run(struct vcpu_svm *svm, int cpu);
>  void __init sev_hardware_setup(void);
>  void sev_hardware_teardown(void);
> +int sev_cpu_init(struct svm_cpu_data *sd);
>  void sev_free_vcpu(struct kvm_vcpu *vcpu);
>  int sev_handle_vmgexit(struct vcpu_svm *svm);
>  int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in);

Re: [PATCH v2 12/14] KVM: SVM: Drop redundant svm_sev_enabled() helper

[Brijesh Singh]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Replace calls to svm_sev_enabled() with direct checks on sev_enabled, or
> in the case of svm_mem_enc_op, simply drop the call to svm_sev_enabled().
> This effectively replaces checks against a valid max_sev_asid with checks
> against sev_enabled.  sev_enabled is forced off by sev_hardware_setup()
> if max_sev_asid is invalid, all call sites are guaranteed to run after
> sev_hardware_setup(), and all of the checks care about SEV being fully
> enabled (as opposed to intentionally handling the scenario where
> max_sev_asid is valid but SEV enabling fails due to OOM).
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/sev.c | 6 +++---
>  arch/x86/kvm/svm/svm.h | 5 -----
>  2 files changed, 3 insertions(+), 8 deletions(-)


Thanks

Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>


> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index a2c3e2d42a7f..7e14514dd083 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1057,7 +1057,7 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
>  	struct kvm_sev_cmd sev_cmd;
>  	int r;
>  
> -	if (!svm_sev_enabled() || !sev_enabled)
> +	if (!sev_enabled)
>  		return -ENOTTY;
>  
>  	if (!argp)
> @@ -1321,7 +1321,7 @@ void __init sev_hardware_setup(void)
>  
>  void sev_hardware_teardown(void)
>  {
> -	if (!svm_sev_enabled())
> +	if (!sev_enabled)
>  		return;
>  
>  	bitmap_free(sev_asid_bitmap);
> @@ -1332,7 +1332,7 @@ void sev_hardware_teardown(void)
>  
>  int sev_cpu_init(struct svm_cpu_data *sd)
>  {
> -	if (!svm_sev_enabled())
> +	if (!sev_enabled)
>  		return 0;
>  
>  	sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1, sizeof(void *),
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 4eb4bab0ca3e..8cb4395b58a0 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -569,11 +569,6 @@ void svm_vcpu_unblocking(struct kvm_vcpu *vcpu);
>  
>  extern unsigned int max_sev_asid;
>  
> -static inline bool svm_sev_enabled(void)
> -{
> -	return IS_ENABLED(CONFIG_KVM_AMD_SEV) ? max_sev_asid : 0;
> -}
> -
>  void sev_vm_destroy(struct kvm *kvm);
>  int svm_mem_enc_op(struct kvm *kvm, void __user *argp);
>  int svm_register_enc_region(struct kvm *kvm,

Re: [PATCH v2 13/14] KVM: SVM: Remove an unnecessary prototype declaration of sev_flush_asids()

[Brijesh Singh]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Remove the forward declaration of sev_flush_asids(), which is only a few
> lines above the function itself.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/sev.c | 1 -
>  1 file changed, 1 deletion(-)


Thanks

Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>


> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 7e14514dd083..23a4bead4a82 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -41,7 +41,6 @@ module_param_named(sev_es, sev_es_enabled, bool, 0444);
>  #endif /* CONFIG_KVM_AMD_SEV */
>  
>  static u8 sev_enc_bit;
> -static int sev_flush_asids(void);
>  static DECLARE_RWSEM(sev_deactivate_lock);
>  static DEFINE_MUTEX(sev_bitmap_lock);
>  unsigned int max_sev_asid;

Re: [PATCH v2 10/14] KVM: SVM: Explicitly check max SEV ASID during sev_hardware_setup()

[Tom Lendacky]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Query max_sev_asid directly after setting it instead of bouncing through
> its wrapper, svm_sev_enabled().  Using the wrapper is unnecessary
> obfuscation.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   arch/x86/kvm/svm/sev.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 02a66008e9b9..1a143340103e 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1278,8 +1278,7 @@ void __init sev_hardware_setup(void)
>   
>   	/* Maximum number of encrypted guests supported simultaneously */
>   	max_sev_asid = ecx;
> -
> -	if (!svm_sev_enabled())
> +	if (!max_sev_asid)
>   		goto out;
>   
>   	/* Minimum ASID value that should be used for SEV guest */
> 

Re: [PATCH v2 11/14] KVM: SVM: Move SEV VMCB tracking allocation to sev.c

[Tom Lendacky]

On 1/14/21 3:37 PM, Brijesh Singh wrote:
> 
> On 1/13/21 6:37 PM, Sean Christopherson wrote:
>> Move the allocation of the SEV VMCB array to sev.c to help pave the way
>> toward encapsulating SEV enabling wholly within sev.c.
>>
>> No functional change intended.
>>
>> Signed-off-by: Sean Christopherson <seanjc@google.com>
>> ---
>>   arch/x86/kvm/svm/sev.c | 13 +++++++++++++
>>   arch/x86/kvm/svm/svm.c | 17 ++++++++---------
>>   arch/x86/kvm/svm/svm.h |  1 +
>>   3 files changed, 22 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index 1a143340103e..a2c3e2d42a7f 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -1330,6 +1330,19 @@ void sev_hardware_teardown(void)
>>   	sev_flush_asids();
>>   }
>>   
>> +int sev_cpu_init(struct svm_cpu_data *sd)
>> +{
>> +	if (!svm_sev_enabled())
>> +		return 0;
>> +
>> +	sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1, sizeof(void *),
>> +				      GFP_KERNEL | __GFP_ZERO);
> 
> 
> I saw Tom recommended to use kzalloc.. instead of __GFP_ZERO in previous

kcalloc :)

Thanks,
Tom

> patch. With that fixed,
> 
> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
> 
> 
>> +	if (!sd->sev_vmcbs)
>> +		return -ENOMEM;
>> +
>> +	return 0;
>> +}
>> +
>>   /*
>>    * Pages used by hardware to hold guest encrypted state must be flushed before
>>    * returning them to the system.
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index bb7b99743bea..89b95fb87a0c 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -552,23 +552,22 @@ static void svm_cpu_uninit(int cpu)
>>   static int svm_cpu_init(int cpu)
>>   {
>>   	struct svm_cpu_data *sd;
>> +	int ret;
>>   
>>   	sd = kzalloc(sizeof(struct svm_cpu_data), GFP_KERNEL);
>>   	if (!sd)
>>   		return -ENOMEM;
>>   	sd->cpu = cpu;
>>   	sd->save_area = alloc_page(GFP_KERNEL);
>> -	if (!sd->save_area)
>> +	if (!sd->save_area) {
>> +		ret = -ENOMEM;
>>   		goto free_cpu_data;
>> +	}
>>   	clear_page(page_address(sd->save_area));
>>   
>> -	if (svm_sev_enabled()) {
>> -		sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1,
>> -					      sizeof(void *),
>> -					      GFP_KERNEL | __GFP_ZERO);
>> -		if (!sd->sev_vmcbs)
>> -			goto free_save_area;
>> -	}
>> +	ret = sev_cpu_init(sd);
>> +	if (ret)
>> +		goto free_save_area;
>>   
>>   	per_cpu(svm_data, cpu) = sd;
>>   
>> @@ -578,7 +577,7 @@ static int svm_cpu_init(int cpu)
>>   	__free_page(sd->save_area);
>>   free_cpu_data:
>>   	kfree(sd);
>> -	return -ENOMEM;
>> +	return ret;
>>   
>>   }
>>   
>> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
>> index 8e169835f52a..4eb4bab0ca3e 100644
>> --- a/arch/x86/kvm/svm/svm.h
>> +++ b/arch/x86/kvm/svm/svm.h
>> @@ -583,6 +583,7 @@ int svm_unregister_enc_region(struct kvm *kvm,
>>   void pre_sev_run(struct vcpu_svm *svm, int cpu);
>>   void __init sev_hardware_setup(void);
>>   void sev_hardware_teardown(void);
>> +int sev_cpu_init(struct svm_cpu_data *sd);
>>   void sev_free_vcpu(struct kvm_vcpu *vcpu);
>>   int sev_handle_vmgexit(struct vcpu_svm *svm);
>>   int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in);

Re: [PATCH v2 11/14] KVM: SVM: Move SEV VMCB tracking allocation to sev.c

[Tom Lendacky]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Move the allocation of the SEV VMCB array to sev.c to help pave the way
> toward encapsulating SEV enabling wholly within sev.c.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   arch/x86/kvm/svm/sev.c | 13 +++++++++++++
>   arch/x86/kvm/svm/svm.c | 17 ++++++++---------
>   arch/x86/kvm/svm/svm.h |  1 +
>   3 files changed, 22 insertions(+), 9 deletions(-)
> 

Re: [PATCH v2 12/14] KVM: SVM: Drop redundant svm_sev_enabled() helper

[Tom Lendacky]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Replace calls to svm_sev_enabled() with direct checks on sev_enabled, or
> in the case of svm_mem_enc_op, simply drop the call to svm_sev_enabled().
> This effectively replaces checks against a valid max_sev_asid with checks
> against sev_enabled.  sev_enabled is forced off by sev_hardware_setup()
> if max_sev_asid is invalid, all call sites are guaranteed to run after
> sev_hardware_setup(), and all of the checks care about SEV being fully
> enabled (as opposed to intentionally handling the scenario where
> max_sev_asid is valid but SEV enabling fails due to OOM).
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Ultimately the #ifdef CONFIG_KVM_AMD_SEV that you added that #defines 
sev_enabled and sev_es_enabled to false, resolves the build issue when 
kvm_amd is built into the kernel and ccp is built as a module, for which 
svm_sev_enabled() was originally created.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   arch/x86/kvm/svm/sev.c | 6 +++---
>   arch/x86/kvm/svm/svm.h | 5 -----
>   2 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index a2c3e2d42a7f..7e14514dd083 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1057,7 +1057,7 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
>   	struct kvm_sev_cmd sev_cmd;
>   	int r;
>   
> -	if (!svm_sev_enabled() || !sev_enabled)
> +	if (!sev_enabled)
>   		return -ENOTTY;
>   
>   	if (!argp)
> @@ -1321,7 +1321,7 @@ void __init sev_hardware_setup(void)
>   
>   void sev_hardware_teardown(void)
>   {
> -	if (!svm_sev_enabled())
> +	if (!sev_enabled)
>   		return;
>   
>   	bitmap_free(sev_asid_bitmap);
> @@ -1332,7 +1332,7 @@ void sev_hardware_teardown(void)
>   
>   int sev_cpu_init(struct svm_cpu_data *sd)
>   {
> -	if (!svm_sev_enabled())
> +	if (!sev_enabled)
>   		return 0;
>   
>   	sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1, sizeof(void *),
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 4eb4bab0ca3e..8cb4395b58a0 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -569,11 +569,6 @@ void svm_vcpu_unblocking(struct kvm_vcpu *vcpu);
>   
>   extern unsigned int max_sev_asid;
>   
> -static inline bool svm_sev_enabled(void)
> -{
> -	return IS_ENABLED(CONFIG_KVM_AMD_SEV) ? max_sev_asid : 0;
> -}
> -
>   void sev_vm_destroy(struct kvm *kvm);
>   int svm_mem_enc_op(struct kvm *kvm, void __user *argp);
>   int svm_register_enc_region(struct kvm *kvm,
> 

Re: [PATCH v2 13/14] KVM: SVM: Remove an unnecessary prototype declaration of sev_flush_asids()

[Tom Lendacky]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Remove the forward declaration of sev_flush_asids(), which is only a few
> lines above the function itself.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   arch/x86/kvm/svm/sev.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 7e14514dd083..23a4bead4a82 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -41,7 +41,6 @@ module_param_named(sev_es, sev_es_enabled, bool, 0444);
>   #endif /* CONFIG_KVM_AMD_SEV */
>   
>   static u8 sev_enc_bit;
> -static int sev_flush_asids(void);
>   static DECLARE_RWSEM(sev_deactivate_lock);
>   static DEFINE_MUTEX(sev_bitmap_lock);
>   unsigned int max_sev_asid;
> 

Re: [PATCH v2 14/14] KVM: SVM: Skip SEV cache flush if no ASIDs have been used

[Tom Lendacky]

On 1/13/21 6:37 PM, Sean Christopherson wrote:
> Skip SEV's expensive WBINVD and DF_FLUSH if there are no SEV ASIDs
> waiting to be reclaimed, e.g. if SEV was never used.  This "fixes" an
> issue where the DF_FLUSH fails during hardware teardown if the original
> SEV_INIT failed.  Ideally, SEV wouldn't be marked as enabled in KVM if
> SEV_INIT fails, but that's a problem for another day.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/svm/sev.c | 22 ++++++++++------------
>   1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 23a4bead4a82..e71bc742d8da 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -56,9 +56,14 @@ struct enc_region {
>   	unsigned long size;
>   };
>   
> -static int sev_flush_asids(void)
> +static int sev_flush_asids(int min_asid, int max_asid)
>   {
> -	int ret, error = 0;
> +	int ret, pos, error = 0;
> +
> +	/* Check if there are any ASIDs to reclaim before performing a flush */
> +	pos = find_next_bit(sev_reclaim_asid_bitmap, max_sev_asid, min_asid);
> +	if (pos >= max_asid)
> +		return -EBUSY;
>   
>   	/*
>   	 * DEACTIVATE will clear the WBINVD indicator causing DF_FLUSH to fail,
> @@ -80,14 +85,7 @@ static int sev_flush_asids(void)
>   /* Must be called with the sev_bitmap_lock held */
>   static bool __sev_recycle_asids(int min_asid, int max_asid)
>   {
> -	int pos;
> -
> -	/* Check if there are any ASIDs to reclaim before performing a flush */
> -	pos = find_next_bit(sev_reclaim_asid_bitmap, max_sev_asid, min_asid);
> -	if (pos >= max_asid)
> -		return false;
> -
> -	if (sev_flush_asids())
> +	if (sev_flush_asids(min_asid, max_asid))
>   		return false;
>   
>   	/* The flush process will flush all reclaimable SEV and SEV-ES ASIDs */
> @@ -1323,10 +1321,10 @@ void sev_hardware_teardown(void)
>   	if (!sev_enabled)
>   		return;
>   
> +	sev_flush_asids(0, max_sev_asid);

I guess you could have called __sev_recycle_asids(0, max_sev_asid) here 
and left things unchanged up above. It would do the extra bitmap_xor() and 
bitmap_zero() operations, though. What do you think?

Also, maybe a comment about not needing the bitmap lock because this is 
during teardown.

Thanks,
Tom

> +
>   	bitmap_free(sev_asid_bitmap);
>   	bitmap_free(sev_reclaim_asid_bitmap);
> -
> -	sev_flush_asids();
>   }
>   
>   int sev_cpu_init(struct svm_cpu_data *sd)
> 

Re: [PATCH v2 14/14] KVM: SVM: Skip SEV cache flush if no ASIDs have been used

[Sean Christopherson]

On Fri, Jan 15, 2021, Tom Lendacky wrote:
> On 1/13/21 6:37 PM, Sean Christopherson wrote:
> > Skip SEV's expensive WBINVD and DF_FLUSH if there are no SEV ASIDs
> > waiting to be reclaimed, e.g. if SEV was never used.  This "fixes" an
> > issue where the DF_FLUSH fails during hardware teardown if the original
> > SEV_INIT failed.  Ideally, SEV wouldn't be marked as enabled in KVM if
> > SEV_INIT fails, but that's a problem for another day.
> > 
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >   arch/x86/kvm/svm/sev.c | 22 ++++++++++------------
> >   1 file changed, 10 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index 23a4bead4a82..e71bc742d8da 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -56,9 +56,14 @@ struct enc_region {
> >   	unsigned long size;
> >   };
> > -static int sev_flush_asids(void)
> > +static int sev_flush_asids(int min_asid, int max_asid)
> >   {
> > -	int ret, error = 0;
> > +	int ret, pos, error = 0;
> > +
> > +	/* Check if there are any ASIDs to reclaim before performing a flush */
> > +	pos = find_next_bit(sev_reclaim_asid_bitmap, max_sev_asid, min_asid);
> > +	if (pos >= max_asid)
> > +		return -EBUSY;
> >   	/*
> >   	 * DEACTIVATE will clear the WBINVD indicator causing DF_FLUSH to fail,
> > @@ -80,14 +85,7 @@ static int sev_flush_asids(void)
> >   /* Must be called with the sev_bitmap_lock held */
> >   static bool __sev_recycle_asids(int min_asid, int max_asid)
> >   {
> > -	int pos;
> > -
> > -	/* Check if there are any ASIDs to reclaim before performing a flush */
> > -	pos = find_next_bit(sev_reclaim_asid_bitmap, max_sev_asid, min_asid);
> > -	if (pos >= max_asid)
> > -		return false;
> > -
> > -	if (sev_flush_asids())
> > +	if (sev_flush_asids(min_asid, max_asid))
> >   		return false;
> >   	/* The flush process will flush all reclaimable SEV and SEV-ES ASIDs */
> > @@ -1323,10 +1321,10 @@ void sev_hardware_teardown(void)
> >   	if (!sev_enabled)
> >   		return;
> > +	sev_flush_asids(0, max_sev_asid);
> 
> I guess you could have called __sev_recycle_asids(0, max_sev_asid) here and
> left things unchanged up above. It would do the extra bitmap_xor() and
> bitmap_zero() operations, though. What do you think?

IMO, calling "recycle" from the teardown flow would be confusing without a
comment to explain that it's the flush that we really care about, and at that
point it's hard to argue against calling "flush" directly.

The cost of the extra operations is almost certainly negligible, but similar to
above it will leave readers wonder why the teardown flow bothers to xor/zero
the bitmap, only to immediately free it.

> Also, maybe a comment about not needing the bitmap lock because this is
> during teardown.

Ya, I'll add that.

Re: [PATCH v2 05/14] KVM: x86: Override reported SME/SEV feature flags with host mask

[Paolo Bonzini]

On 14/01/21 01:36, Sean Christopherson wrote:
> Add a reverse-CPUID entry for the memory encryption word, 0x8000001F.EAX,
> and use it to override the supported CPUID flags reported to userspace.
> Masking the reported CPUID flags avoids over-reporting KVM support, e.g.
> without the mask a SEV-SNP capable CPU may incorrectly advertise SNP
> support to userspace.
> 
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/cpuid.c | 2 ++
>   arch/x86/kvm/cpuid.h | 1 +
>   2 files changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 13036cf0b912..b7618cdd06b5 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -855,6 +855,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>   	case 0x8000001F:
>   		if (!boot_cpu_has(X86_FEATURE_SEV))
>   			entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
> +		else
> +			cpuid_entry_override(entry, CPUID_8000_001F_EAX);
>   		break;
>   	/*Add support for Centaur's CPUID instruction*/
>   	case 0xC0000000:
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index dc921d76e42e..8b6fc9bde248 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -63,6 +63,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
>   	[CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX},
>   	[CPUID_7_EDX]         = {         7, 0, CPUID_EDX},
>   	[CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
> +	[CPUID_8000_001F_EAX] = {0x8000001f, 1, CPUID_EAX},
>   };
>   
>   /*
> 

I don't understand, wouldn't this also need a kvm_cpu_cap_mask call 
somewhere else?  As it is, it doesn't do anything.

Paolo

Re: [PATCH v2 04/14] x86/cpufeatures: Assign dedicated feature word for AMD mem encryption

[Paolo Bonzini]

On 14/01/21 18:16, Borislav Petkov wrote:
> On Thu, Jan 14, 2021 at 09:09:28AM -0800, Sean Christopherson wrote:
>> Hmm, patch 05/14 depends on the existence of the new word.  That's a non-issue
>> if you're planning on taking this for 5.11.  If it's destined for 5.12, maybe
>> get an ack from Paolo on patch 05 and take both through tip?
> 
> Yeah, I guess that. Both are not urgent 5.11 material to take 'em now.
> So I guess I'll wait for Paolo's ACK.

If you think this patch is valuable go ahead and pick it.  I can wait 
until after the merge window to queue patch 5.  It is independent from 
the others and I had questions, so I am just queuing the others for 5.12.

Paolo

>> I can drop them from this series when I send v3. In hindsight, I
>> should have split these two patches into a separate mini-series from
>> the get-go.
> 
> Nah, no worries. We do patch acrobatics on a daily basis. :-)

Re: [PATCH v2 14/14] KVM: SVM: Skip SEV cache flush if no ASIDs have been used

[Paolo Bonzini]

On 14/01/21 01:37, Sean Christopherson wrote:
> Skip SEV's expensive WBINVD and DF_FLUSH if there are no SEV ASIDs
> waiting to be reclaimed, e.g. if SEV was never used.  This "fixes" an
> issue where the DF_FLUSH fails during hardware teardown if the original
> SEV_INIT failed.  Ideally, SEV wouldn't be marked as enabled in KVM if
> SEV_INIT fails, but that's a problem for another day.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/svm/sev.c | 22 ++++++++++------------
>   1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 23a4bead4a82..e71bc742d8da 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -56,9 +56,14 @@ struct enc_region {
>   	unsigned long size;
>   };
>   
> -static int sev_flush_asids(void)
> +static int sev_flush_asids(int min_asid, int max_asid)
>   {
> -	int ret, error = 0;
> +	int ret, pos, error = 0;
> +
> +	/* Check if there are any ASIDs to reclaim before performing a flush */
> +	pos = find_next_bit(sev_reclaim_asid_bitmap, max_sev_asid, min_asid);
> +	if (pos >= max_asid)
> +		return -EBUSY;
>   
>   	/*
>   	 * DEACTIVATE will clear the WBINVD indicator causing DF_FLUSH to fail,
> @@ -80,14 +85,7 @@ static int sev_flush_asids(void)
>   /* Must be called with the sev_bitmap_lock held */
>   static bool __sev_recycle_asids(int min_asid, int max_asid)
>   {
> -	int pos;
> -
> -	/* Check if there are any ASIDs to reclaim before performing a flush */
> -	pos = find_next_bit(sev_reclaim_asid_bitmap, max_sev_asid, min_asid);
> -	if (pos >= max_asid)
> -		return false;
> -
> -	if (sev_flush_asids())
> +	if (sev_flush_asids(min_asid, max_asid))
>   		return false;
>   
>   	/* The flush process will flush all reclaimable SEV and SEV-ES ASIDs */
> @@ -1323,10 +1321,10 @@ void sev_hardware_teardown(void)
>   	if (!sev_enabled)
>   		return;
>   
> +	sev_flush_asids(0, max_sev_asid);
> +
>   	bitmap_free(sev_asid_bitmap);
>   	bitmap_free(sev_reclaim_asid_bitmap);
> -
> -	sev_flush_asids();
>   }
>   
>   int sev_cpu_init(struct svm_cpu_data *sd)
> 

I can't find 00/14 in my inbox, so: queued 1-3 and 6-14, thanks.

Paolo

Re: [PATCH v2 14/14] KVM: SVM: Skip SEV cache flush if no ASIDs have been used

[Sean Christopherson]

On Thu, Jan 28, 2021, Paolo Bonzini wrote:
> I can't find 00/14 in my inbox, so: queued 1-3 and 6-14, thanks.

If it's not too late, v3 has a few tweaks that would be nice to have, as well as
a new patch to remove the CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT dependency.

https://lkml.kernel.org/r/20210122202144.2756381-1-seanjc@google.com

Re: [PATCH v2 14/14] KVM: SVM: Skip SEV cache flush if no ASIDs have been used

[Paolo Bonzini]

On 28/01/21 17:29, Sean Christopherson wrote:
> On Thu, Jan 28, 2021, Paolo Bonzini wrote:
>> I can't find 00/14 in my inbox, so: queued 1-3 and 6-14, thanks.
> 
> If it's not too late, v3 has a few tweaks that would be nice to have, as well as
> a new patch to remove the CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT dependency.
> 
> https://lkml.kernel.org/r/20210122202144.2756381-1-seanjc@google.com

Yes, will do (I had done all of them myself except the comment in 
sev_hardware_teardown() but it's better to match what was sent to LKML).

Paolo

Re: [PATCH v2 05/14] KVM: x86: Override reported SME/SEV feature flags with host mask

[Sean Christopherson]

On Thu, Jan 28, 2021, Paolo Bonzini wrote:
> On 14/01/21 01:36, Sean Christopherson wrote:
> > Add a reverse-CPUID entry for the memory encryption word, 0x8000001F.EAX,
> > and use it to override the supported CPUID flags reported to userspace.
> > Masking the reported CPUID flags avoids over-reporting KVM support, e.g.
> > without the mask a SEV-SNP capable CPU may incorrectly advertise SNP
> > support to userspace.
> > 
> > Cc: Brijesh Singh <brijesh.singh@amd.com>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >   arch/x86/kvm/cpuid.c | 2 ++
> >   arch/x86/kvm/cpuid.h | 1 +
> >   2 files changed, 3 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > index 13036cf0b912..b7618cdd06b5 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -855,6 +855,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
> >   	case 0x8000001F:
> >   		if (!boot_cpu_has(X86_FEATURE_SEV))
> >   			entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
> > +		else
> > +			cpuid_entry_override(entry, CPUID_8000_001F_EAX);
> >   		break;
> >   	/*Add support for Centaur's CPUID instruction*/
> >   	case 0xC0000000:
> > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> > index dc921d76e42e..8b6fc9bde248 100644
> > --- a/arch/x86/kvm/cpuid.h
> > +++ b/arch/x86/kvm/cpuid.h
> > @@ -63,6 +63,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
> >   	[CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX},
> >   	[CPUID_7_EDX]         = {         7, 0, CPUID_EDX},
> >   	[CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
> > +	[CPUID_8000_001F_EAX] = {0x8000001f, 1, CPUID_EAX},
> >   };
> >   /*
> > 
> 
> I don't understand, wouldn't this also need a kvm_cpu_cap_mask call
> somewhere else?  As it is, it doesn't do anything.

Ugh, yes, apparently I thought the kernel would magically clear bits it doesn't
care about.

Looking at this again, I think the kvm_cpu_cap_mask() invocation should always
mask off X86_FEATURE_SME.  SME cannot be virtualized, and AFAIK it's not
emulated by KVM.  This would fix an oddity where SME would be advertised if SEV
is also supported.

Boris has queue the kernel change to tip/x86/cpu, I'll spin v4 against that.

Re: [PATCH v2 05/14] KVM: x86: Override reported SME/SEV feature flags with host mask

[Paolo Bonzini]

On 28/01/21 18:09, Sean Christopherson wrote:
> On Thu, Jan 28, 2021, Paolo Bonzini wrote:
>> On 14/01/21 01:36, Sean Christopherson wrote:
>>> Add a reverse-CPUID entry for the memory encryption word, 0x8000001F.EAX,
>>> and use it to override the supported CPUID flags reported to userspace.
>>> Masking the reported CPUID flags avoids over-reporting KVM support, e.g.
>>> without the mask a SEV-SNP capable CPU may incorrectly advertise SNP
>>> support to userspace.
>>>
>>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>> Signed-off-by: Sean Christopherson <seanjc@google.com>
>>> ---
>>>    arch/x86/kvm/cpuid.c | 2 ++
>>>    arch/x86/kvm/cpuid.h | 1 +
>>>    2 files changed, 3 insertions(+)
>>>
>>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>>> index 13036cf0b912..b7618cdd06b5 100644
>>> --- a/arch/x86/kvm/cpuid.c
>>> +++ b/arch/x86/kvm/cpuid.c
>>> @@ -855,6 +855,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>>>    	case 0x8000001F:
>>>    		if (!boot_cpu_has(X86_FEATURE_SEV))
>>>    			entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
>>> +		else
>>> +			cpuid_entry_override(entry, CPUID_8000_001F_EAX);
>>>    		break;
>>>    	/*Add support for Centaur's CPUID instruction*/
>>>    	case 0xC0000000:
>>> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
>>> index dc921d76e42e..8b6fc9bde248 100644
>>> --- a/arch/x86/kvm/cpuid.h
>>> +++ b/arch/x86/kvm/cpuid.h
>>> @@ -63,6 +63,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
>>>    	[CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX},
>>>    	[CPUID_7_EDX]         = {         7, 0, CPUID_EDX},
>>>    	[CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
>>> +	[CPUID_8000_001F_EAX] = {0x8000001f, 1, CPUID_EAX},
>>>    };
>>>    /*
>>>
>>
>> I don't understand, wouldn't this also need a kvm_cpu_cap_mask call
>> somewhere else?  As it is, it doesn't do anything.
> 
> Ugh, yes, apparently I thought the kernel would magically clear bits it doesn't
> care about.
> 
> Looking at this again, I think the kvm_cpu_cap_mask() invocation should always
> mask off X86_FEATURE_SME.  SME cannot be virtualized, and AFAIK it's not
> emulated by KVM.  This would fix an oddity where SME would be advertised if SEV
> is also supported.
> 
> Boris has queue the kernel change to tip/x86/cpu, I'll spin v4 against that.

You can send it after the 5.12 merge window.

Paolo