[PATCH v5 00/15] KVM: SVM: Misc SEV cleanups

[PATCH v5 00/15] 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 kvm/queue-ish, commit 0e91d1992235 ("KVM: SVM: Allocate SEV command
structures on local stack"), to avoid the conflicting CPUID.0x8000_001F
patch sitting in kvm/queue.

v5:
 - Use Paolo's version of the CPUID.0x8000_001F patch, with some of my
   goo on top.  Paolo gets credit by introducing fewer bugs; v4 missed
   the SEV/SEV-ES module params and used the wrong reverse-CPUID index...
 - Add a patch to disable SEV/SEV-ES if NPT is disabled.
 - Rebased, as above.
v4:
 - Reinstate the patch to override CPUID.0x8000_001F.
 - Properly configure the CPUID.0x8000_001F override. [Paolo]
 - Rebase to v5.12-rc1-dontuse.
v3:
 - Drop two patches: add a dedicated feature word for CPUID_0x8000001F,
   and use the new word to mask host CPUID in KVM.  I'll send these as a
   separate mini-series so that Boris can take them through tip.
 - Add a patch to remove dependency on
   CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT. [Boris / Paolo]
 - Use kcalloc() instead of an open-coded equivalent. [Tom]
 - Nullify sev_asid_bitmap when freeing it during setup. [Tom]
 - Add a comment in sev_hardware_teardown() to document why it's safe to
   query the ASID bitmap without taking the lock. [Tom]
 - Collect reviews. [Tom and Brijesh]
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

Paolo Bonzini (1):
  KVM: SEV: Mask CPUID[0x8000001F].eax according to supported features

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: Disable SEV/SEV-ES if NPT is disabled
  KVM: SVM: Move SEV module params/variables to sev.c
  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: Enable SEV/SEV-ES functionality by default (when supported)
  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/mem_encrypt.h |  1 -
 arch/x86/kvm/cpuid.c               |  8 ++-
 arch/x86/kvm/cpuid.h               |  1 +
 arch/x86/kvm/svm/sev.c             | 80 ++++++++++++++++++++++--------
 arch/x86/kvm/svm/svm.c             | 57 +++++++++------------
 arch/x86/kvm/svm/svm.h             |  9 +---
 arch/x86/mm/mem_encrypt.c          | 12 ++---
 arch/x86/mm/mem_encrypt_identity.c |  1 -
 8 files changed, 97 insertions(+), 72 deletions(-)

-- 
2.31.1.498.g6c1eba8ee3d-goog

[PATCH v5 01/15] 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>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: 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 | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index cd8c333ed2dc..fed153314aef 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -563,9 +563,8 @@ static int svm_cpu_init(int cpu)
 	clear_page(page_address(sd->save_area));
 
 	if (svm_sev_enabled()) {
-		sd->sev_vmcbs = kmalloc_array(max_sev_asid + 1,
-					      sizeof(void *),
-					      GFP_KERNEL);
+		sd->sev_vmcbs = kcalloc(max_sev_asid + 1, sizeof(void *),
+					GFP_KERNEL);
 		if (!sd->sev_vmcbs)
 			goto free_save_area;
 	}
-- 
2.31.1.498.g6c1eba8ee3d-goog

[PATCH v5 02/15] 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 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index b4e471b0a231..5ff8a202cc01 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1788,8 +1788,11 @@ 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);
+		sev_asid_bitmap = NULL;
 		goto out;
+	}
 
 	pr_info("SEV supported: %u ASIDs\n", max_sev_asid - min_sev_asid + 1);
 	sev_supported = true;
-- 
2.31.1.498.g6c1eba8ee3d-goog

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

[Tom Lendacky]

On 4/21/21 9:11 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>

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

> ---
>  arch/x86/kvm/svm/sev.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index b4e471b0a231..5ff8a202cc01 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1788,8 +1788,11 @@ 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);
> +		sev_asid_bitmap = NULL;
>  		goto out;
> +	}
>  
>  	pr_info("SEV supported: %u ASIDs\n", max_sev_asid - min_sev_asid + 1);
>  	sev_supported = true;
> 

[PATCH v5 03/15] KVM: SVM: Disable SEV/SEV-ES if NPT is disabled

[Sean Christopherson]

Disable SEV and SEV-ES if NPT is disabled.  While the APM doesn't clearly
state that NPT is mandatory, it's alluded to by:

  The guest page tables, managed by the guest, may mark data memory pages
  as either private or shared, thus allowing selected pages to be shared
  outside the guest.

And practically speaking, shadow paging can't work since KVM can't read
the guest's page tables.

Fixes: e9df09428996 ("KVM: SVM: Add sev module_param")
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/svm/svm.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index fed153314aef..0e8489908216 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -970,7 +970,21 @@ static __init int svm_hardware_setup(void)
 		kvm_enable_efer_bits(EFER_SVME | EFER_LMSLE);
 	}
 
-	if (IS_ENABLED(CONFIG_KVM_AMD_SEV) && sev) {
+	/*
+	 * KVM's MMU doesn't support using 2-level paging for itself, and thus
+	 * NPT isn't supported if the host is using 2-level paging since host
+	 * CR4 is unchanged on VMRUN.
+	 */
+	if (!IS_ENABLED(CONFIG_X86_64) && !IS_ENABLED(CONFIG_X86_PAE))
+		npt_enabled = false;
+
+	if (!boot_cpu_has(X86_FEATURE_NPT))
+		npt_enabled = false;
+
+	kvm_configure_mmu(npt_enabled, get_max_npt_level(), PG_LEVEL_1G);
+	pr_info("kvm: Nested Paging %sabled\n", npt_enabled ? "en" : "dis");
+
+	if (IS_ENABLED(CONFIG_KVM_AMD_SEV) && sev && npt_enabled) {
 		sev_hardware_setup();
 	} else {
 		sev = false;
@@ -985,20 +999,6 @@ static __init int svm_hardware_setup(void)
 			goto err;
 	}
 
-	/*
-	 * KVM's MMU doesn't support using 2-level paging for itself, and thus
-	 * NPT isn't supported if the host is using 2-level paging since host
-	 * CR4 is unchanged on VMRUN.
-	 */
-	if (!IS_ENABLED(CONFIG_X86_64) && !IS_ENABLED(CONFIG_X86_PAE))
-		npt_enabled = false;
-
-	if (!boot_cpu_has(X86_FEATURE_NPT))
-		npt_enabled = false;
-
-	kvm_configure_mmu(npt_enabled, get_max_npt_level(), PG_LEVEL_1G);
-	pr_info("kvm: Nested Paging %sabled\n", npt_enabled ? "en" : "dis");
-
 	if (nrips) {
 		if (!boot_cpu_has(X86_FEATURE_NRIPS))
 			nrips = false;
-- 
2.31.1.498.g6c1eba8ee3d-goog

Re: [PATCH v5 03/15] KVM: SVM: Disable SEV/SEV-ES if NPT is disabled

[Paolo Bonzini]

On 22/04/21 04:11, Sean Christopherson wrote:
> Disable SEV and SEV-ES if NPT is disabled.  While the APM doesn't clearly
> state that NPT is mandatory, it's alluded to by:
> 
>    The guest page tables, managed by the guest, may mark data memory pages
>    as either private or shared, thus allowing selected pages to be shared
>    outside the guest.
> 
> And practically speaking, shadow paging can't work since KVM can't read
> the guest's page tables.
> 
> Fixes: e9df09428996 ("KVM: SVM: Add sev module_param")
> 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/svm/svm.c | 30 +++++++++++++++---------------
>   1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index fed153314aef..0e8489908216 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -970,7 +970,21 @@ static __init int svm_hardware_setup(void)
>   		kvm_enable_efer_bits(EFER_SVME | EFER_LMSLE);
>   	}
>   
> -	if (IS_ENABLED(CONFIG_KVM_AMD_SEV) && sev) {
> +	/*
> +	 * KVM's MMU doesn't support using 2-level paging for itself, and thus
> +	 * NPT isn't supported if the host is using 2-level paging since host
> +	 * CR4 is unchanged on VMRUN.
> +	 */
> +	if (!IS_ENABLED(CONFIG_X86_64) && !IS_ENABLED(CONFIG_X86_PAE))
> +		npt_enabled = false;

Unrelated, but since you're moving this code: should we be pre-scient 
and tackle host 5-level paging as well?

Support for 5-level page tables on NPT is not hard to fix and could be 
tested by patching QEMU.  However, the !NPT case would also have to be 
fixed by extending the PDP and PML4 stacking trick to a PML5.

However, without real hardware to test on I'd be a bit wary to do it. 
Looking at 5-level EPT there might be other issues (e.g. what's the 
guest MAXPHYADDR) and I would prefer to see what AMD comes up with 
exactly in the APM.  So I would just block loading KVM on hypothetical 
AMD hosts with CR4.LA57=1.

Paolo

> +	if (!boot_cpu_has(X86_FEATURE_NPT))
> +		npt_enabled = false;
> +
> +	kvm_configure_mmu(npt_enabled, get_max_npt_level(), PG_LEVEL_1G);
> +	pr_info("kvm: Nested Paging %sabled\n", npt_enabled ? "en" : "dis");
> +
> +	if (IS_ENABLED(CONFIG_KVM_AMD_SEV) && sev && npt_enabled) {
>   		sev_hardware_setup();
>   	} else {
>   		sev = false;
> @@ -985,20 +999,6 @@ static __init int svm_hardware_setup(void)
>   			goto err;
>   	}
>   
> -	/*
> -	 * KVM's MMU doesn't support using 2-level paging for itself, and thus
> -	 * NPT isn't supported if the host is using 2-level paging since host
> -	 * CR4 is unchanged on VMRUN.
> -	 */
> -	if (!IS_ENABLED(CONFIG_X86_64) && !IS_ENABLED(CONFIG_X86_PAE))
> -		npt_enabled = false;
> -
> -	if (!boot_cpu_has(X86_FEATURE_NPT))
> -		npt_enabled = false;
> -
> -	kvm_configure_mmu(npt_enabled, get_max_npt_level(), PG_LEVEL_1G);
> -	pr_info("kvm: Nested Paging %sabled\n", npt_enabled ? "en" : "dis");
> -
>   	if (nrips) {
>   		if (!boot_cpu_has(X86_FEATURE_NRIPS))
>   			nrips = false;
> 

Re: [PATCH v5 03/15] KVM: SVM: Disable SEV/SEV-ES if NPT is disabled

[Sean Christopherson]

On Thu, Apr 22, 2021, Paolo Bonzini wrote:
> On 22/04/21 04:11, Sean Christopherson wrote:
> > Disable SEV and SEV-ES if NPT is disabled.  While the APM doesn't clearly
> > state that NPT is mandatory, it's alluded to by:
> > 
> >    The guest page tables, managed by the guest, may mark data memory pages
> >    as either private or shared, thus allowing selected pages to be shared
> >    outside the guest.
> > 
> > And practically speaking, shadow paging can't work since KVM can't read
> > the guest's page tables.
> > 
> > Fixes: e9df09428996 ("KVM: SVM: Add sev module_param")
> > 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/svm/svm.c | 30 +++++++++++++++---------------
> >   1 file changed, 15 insertions(+), 15 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index fed153314aef..0e8489908216 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -970,7 +970,21 @@ static __init int svm_hardware_setup(void)
> >   		kvm_enable_efer_bits(EFER_SVME | EFER_LMSLE);
> >   	}
> > -	if (IS_ENABLED(CONFIG_KVM_AMD_SEV) && sev) {
> > +	/*
> > +	 * KVM's MMU doesn't support using 2-level paging for itself, and thus
> > +	 * NPT isn't supported if the host is using 2-level paging since host
> > +	 * CR4 is unchanged on VMRUN.
> > +	 */
> > +	if (!IS_ENABLED(CONFIG_X86_64) && !IS_ENABLED(CONFIG_X86_PAE))
> > +		npt_enabled = false;
> 
> Unrelated, but since you're moving this code: should we be pre-scient and
> tackle host 5-level paging as well?
> 
> Support for 5-level page tables on NPT is not hard to fix and could be
> tested by patching QEMU.  However, the !NPT case would also have to be fixed
> by extending the PDP and PML4 stacking trick to a PML5.

Isn't that backwards?  It's the nested NPT case that requires the stacking trick.
When !NPT is disabled in L0 KVM, 32-bit guests are run with PAE paging.  Maybe
I'm misunderstanding what you're suggesting.
 
> However, without real hardware to test on I'd be a bit wary to do it.
> Looking at 5-level EPT there might be other issues (e.g. what's the guest
> MAXPHYADDR) and I would prefer to see what AMD comes up with exactly in the
> APM.  So I would just block loading KVM on hypothetical AMD hosts with
> CR4.LA57=1.

Agreed, I think blocking KVM makes the most sense.

Re: [PATCH v5 03/15] KVM: SVM: Disable SEV/SEV-ES if NPT is disabled

[Paolo Bonzini]

On 22/04/21 18:15, Sean Christopherson wrote:
>> Support for 5-level page tables on NPT is not hard to fix and could be
>> tested by patching QEMU.  However, the !NPT case would also have to be fixed
>> by extending the PDP and PML4 stacking trick to a PML5.
>   
> Isn't that backwards?  It's the nested NPT case that requires the stacking trick.
> When !NPT is disabled in L0 KVM, 32-bit guests are run with PAE paging.  Maybe
> I'm misunderstanding what you're suggesting.

Yes, you're right.  NPT is easy but we would have to guess what the spec 
would say about MAXPHYADDR, while nNPT would require the stacking of a 
PML5.  Either way, blocking KVM is the easiest thing todo.

Paolo

Re: [PATCH v5 03/15] KVM: SVM: Disable SEV/SEV-ES if NPT is disabled

[Sean Christopherson]

On Thu, Apr 22, 2021, Paolo Bonzini wrote:
> On 22/04/21 18:15, Sean Christopherson wrote:
> > > Support for 5-level page tables on NPT is not hard to fix and could be
> > > tested by patching QEMU.  However, the !NPT case would also have to be fixed
> > > by extending the PDP and PML4 stacking trick to a PML5.
> > Isn't that backwards?  It's the nested NPT case that requires the stacking trick.
> > When !NPT is disabled in L0 KVM, 32-bit guests are run with PAE paging.  Maybe
> > I'm misunderstanding what you're suggesting.
> 
> Yes, you're right.  NPT is easy but we would have to guess what the spec
> would say about MAXPHYADDR, while nNPT would require the stacking of a PML5.
> Either way, blocking KVM is the easiest thing todo.

How about I fold that into the s/lm_root/pml4_root rename[*]?  I.e. make the
blocking of PML5 a functional change, and the rename an opportunistic change?

[*] https://lkml.kernel.org/r/20210318201131.3242619-1-seanjc@google.com

Re: [PATCH v5 03/15] KVM: SVM: Disable SEV/SEV-ES if NPT is disabled

[Paolo Bonzini]

On 22/04/21 20:11, Sean Christopherson wrote:
>> Yes, you're right.  NPT is easy but we would have to guess what the spec
>> would say about MAXPHYADDR, while nNPT would require the stacking of a PML5.
>> Either way, blocking KVM is the easiest thing todo.
> How about I fold that into the s/lm_root/pml4_root rename[*]?  I.e. make the
> blocking of PML5 a functional change, and the rename an opportunistic change?
> 
> [*]https://lkml.kernel.org/r/20210318201131.3242619-1-seanjc@google.com
> 

Yes, that's a good plan.  Thanks,

Paolo

[PATCH v5 04/15] 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'.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 11 +++++++++++
 arch/x86/kvm/svm/svm.c | 16 ++--------------
 arch/x86/kvm/svm/svm.h |  2 --
 3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 5ff8a202cc01..fb32b93e325c 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -28,6 +28,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);
@@ -1762,6 +1770,9 @@ void __init sev_hardware_setup(void)
 	bool sev_es_supported = false;
 	bool sev_supported = false;
 
+	if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev || !npt_enabled)
+		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 0e8489908216..12b2c04076bb 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -185,14 +185,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);
 
@@ -984,12 +976,8 @@ static __init int svm_hardware_setup(void)
 	kvm_configure_mmu(npt_enabled, get_max_npt_level(), PG_LEVEL_1G);
 	pr_info("kvm: Nested Paging %sabled\n", npt_enabled ? "en" : "dis");
 
-	if (IS_ENABLED(CONFIG_KVM_AMD_SEV) && sev && npt_enabled) {
-		sev_hardware_setup();
-	} else {
-		sev = false;
-		sev_es = false;
-	}
+	/* Note, SEV setup consumes npt_enabled. */
+	sev_hardware_setup();
 
 	svm_adjust_mmio_mask();
 
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 454da1c1d9b7..ec0407f41458 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -397,8 +397,6 @@ static inline bool gif_set(struct vcpu_svm *svm)
 /* svm.c */
 #define MSR_INVALID				0xffffffffU
 
-extern int sev;
-extern int sev_es;
 extern bool dump_invalid_vmcb;
 
 u32 svm_msrpm_offset(u32 msr);
-- 
2.31.1.498.g6c1eba8ee3d-goog

[PATCH v5 05/15] KVM: SEV: Mask CPUID[0x8000001F].eax according to supported features

[Sean Christopherson]

From: Paolo Bonzini <pbonzini@redhat.com>

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.

Clear SEV/SEV-ES if their corresponding module parameters are disabled,
and clear the memory encryption leaf completely if SEV is not fully
supported in KVM.  Advertise SME_COHERENT in addition to SEV and SEV-ES,
as the guest can use SME_COHERENT to avoid CLFLUSH operations.

Explicitly omit SME and VM_PAGE_FLUSH from the reporting.  These features
are used by KVM, but are not exposed to the guest, e.g. guest access to
related MSRs will fault.

Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/cpuid.c   | 8 +++++++-
 arch/x86/kvm/cpuid.h   | 1 +
 arch/x86/kvm/svm/sev.c | 8 ++++++++
 arch/x86/kvm/svm/svm.c | 3 +++
 arch/x86/kvm/svm/svm.h | 1 +
 5 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 2ae061586677..96e41e1a1bde 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -557,6 +557,10 @@ void kvm_set_cpu_caps(void)
 	 */
 	kvm_cpu_cap_mask(CPUID_8000_000A_EDX, 0);
 
+	kvm_cpu_cap_mask(CPUID_8000_001F_EAX,
+		0 /* SME */ | F(SEV) | 0 /* VM_PAGE_FLUSH */ | F(SEV_ES) |
+		F(SME_COHERENT));
+
 	kvm_cpu_cap_mask(CPUID_C000_0001_EDX,
 		F(XSTORE) | F(XSTORE_EN) | F(XCRYPT) | F(XCRYPT_EN) |
 		F(ACE2) | F(ACE2_EN) | F(PHE) | F(PHE_EN) |
@@ -944,8 +948,10 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 		break;
 	/* Support memory encryption cpuid if host supports it */
 	case 0x8000001F:
-		if (!boot_cpu_has(X86_FEATURE_SEV))
+		if (!kvm_cpu_cap_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 888e88b42e8d..eeb4a3020e1b 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -99,6 +99,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
 	[CPUID_7_EDX]         = {         7, 0, CPUID_EDX},
 	[CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
 	[CPUID_12_EAX]        = {0x00000012, 0, CPUID_EAX},
+	[CPUID_8000_001F_EAX] = {0x8000001f, 0, CPUID_EAX},
 };
 
 /*
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index fb32b93e325c..e54eff6dfbbe 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1764,6 +1764,14 @@ void sev_vm_destroy(struct kvm *kvm)
 	sev_asid_free(sev->asid);
 }
 
+void __init sev_set_cpu_caps(void)
+{
+	if (!sev)
+		kvm_cpu_cap_clear(X86_FEATURE_SEV);
+	if (!sev_es)
+		kvm_cpu_cap_clear(X86_FEATURE_SEV_ES);
+}
+
 void __init sev_hardware_setup(void)
 {
 	unsigned int eax, ebx, ecx, edx;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 12b2c04076bb..cb227e90dffb 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -914,6 +914,9 @@ static __init void svm_set_cpu_caps(void)
 	if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) ||
 	    boot_cpu_has(X86_FEATURE_AMD_SSBD))
 		kvm_cpu_cap_set(X86_FEATURE_VIRT_SSBD);
+
+	/* CPUID 0x8000001F (SME/SEV features) */
+	sev_set_cpu_caps();
 }
 
 static __init int svm_hardware_setup(void)
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index ec0407f41458..39d1412f2c45 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -581,6 +581,7 @@ int svm_unregister_enc_region(struct kvm *kvm,
 			      struct kvm_enc_region *range);
 int svm_vm_copy_asid_from(struct kvm *kvm, unsigned int source_fd);
 void pre_sev_run(struct vcpu_svm *svm, int cpu);
+void __init sev_set_cpu_caps(void);
 void __init sev_hardware_setup(void);
 void sev_hardware_teardown(void);
 void sev_free_vcpu(struct kvm_vcpu *vcpu);
-- 
2.31.1.498.g6c1eba8ee3d-goog

[PATCH v5 06/15] 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.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
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 31c4df123aa0..9c80c68d75b5 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 4b01f7dbaf30..be384d8d0543 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);
 
@@ -373,15 +371,15 @@ 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();
+}
 EXPORT_SYMBOL_GPL(sev_active);
 
 /* Needs to be called from non-instrumentable code */
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.31.1.498.g6c1eba8ee3d-goog

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

[Paolo Bonzini]

On 22/04/21 04:11, 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.
> 
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Boris or another x86 maintainer, can you ack this small patch?  We would 
like to use sev_enabled as a static variable in KVM.

Paolo

> ---
>   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 31c4df123aa0..9c80c68d75b5 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 4b01f7dbaf30..be384d8d0543 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);
>   
> @@ -373,15 +371,15 @@ 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();
> +}
>   EXPORT_SYMBOL_GPL(sev_active);
>   
>   /* Needs to be called from non-instrumentable code */
> 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 v5 06/15] x86/sev: Drop redundant and potentially misleading 'sev_enabled'

[Borislav Petkov]

On Thu, Apr 22, 2021 at 02:05:46PM +0200, Paolo Bonzini wrote:
> Boris or another x86 maintainer, can you ack this small patch?  We would
> like to use sev_enabled as a static variable in KVM.

Yeah, all those "is anything SEV-like enabled" mechanisms would need
refactoring before it goes nuts. I think we should do this

bool sev_feature_enabled(enum sev_feature)

thing at some point:

https://lkml.kernel.org/r/20210421144402.GB5004@zn.tnic

And TDX would probably need something similar.

Thx.

-- 
Regards/Gruss,
    Boris.

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

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

[Borislav Petkov]

On Wed, Apr 21, 2021 at 07:11:16PM -0700, 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.
> 
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
> 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(-)

Acked-by: Borislav Petkov <bp@suse.de>

Thx.

-- 
Regards/Gruss,
    Boris.

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

[PATCH v5 07/15] 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>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index e54eff6dfbbe..9b6adc493cc8 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -29,12 +29,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);
@@ -1452,7 +1452,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)
@@ -1471,7 +1471,7 @@ int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 
 	switch (sev_cmd.id) {
 	case KVM_SEV_ES_INIT:
-		if (!sev_es) {
+		if (!sev_es_enabled) {
 			r = -ENOTTY;
 			goto out;
 		}
@@ -1766,9 +1766,9 @@ void sev_vm_destroy(struct kvm *kvm)
 
 void __init sev_set_cpu_caps(void)
 {
-	if (!sev)
+	if (!sev_enabled)
 		kvm_cpu_cap_clear(X86_FEATURE_SEV);
-	if (!sev_es)
+	if (!sev_es_enabled)
 		kvm_cpu_cap_clear(X86_FEATURE_SEV_ES);
 }
 
@@ -1778,7 +1778,7 @@ void __init sev_hardware_setup(void)
 	bool sev_es_supported = false;
 	bool sev_supported = false;
 
-	if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev || !npt_enabled)
+	if (!IS_ENABLED(CONFIG_KVM_AMD_SEV) || !sev_enabled || !npt_enabled)
 		goto out;
 
 	/* Does the CPU support SEV? */
@@ -1817,7 +1817,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? */
@@ -1832,8 +1832,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.31.1.498.g6c1eba8ee3d-goog

[PATCH v5 08/15] 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 modules 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.

Reviewed by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@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 9b6adc493cc8..2fe545102d12 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -28,6 +28,7 @@
 
 #define __ex(x) __kvm_handle_fault_on_reboot(x)
 
+#ifdef CONFIG_KVM_AMD_SEV
 /* enable/disable SEV support */
 static bool sev_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
 module_param_named(sev, sev_enabled, bool, 0444);
@@ -35,6 +36,10 @@ 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);
@@ -1774,11 +1779,12 @@ void __init sev_set_cpu_caps(void)
 
 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 || !npt_enabled)
+	if (!sev_enabled || !npt_enabled)
 		goto out;
 
 	/* Does the CPU support SEV? */
@@ -1834,6 +1840,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.31.1.498.g6c1eba8ee3d-goog

[PATCH v5 09/15] KVM: SVM: Enable SEV/SEV-ES functionality by default (when supported)

[Sean Christopherson]

Enable the 'sev' and 'sev_es' module params by default instead of having
them conditioned on CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT.  The extra
Kconfig is pointless as KVM SEV/SEV-ES support is already controlled via
CONFIG_KVM_AMD_SEV, and CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT has the
unfortunate side effect of enabling all the SEV-ES _guest_ code due to
it being dependent on CONFIG_AMD_MEM_ENCRYPT=y.

Cc: Borislav Petkov <bp@suse.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 2fe545102d12..bd26e564549c 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -30,11 +30,11 @@
 
 #ifdef CONFIG_KVM_AMD_SEV
 /* enable/disable SEV support */
-static bool sev_enabled = IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT);
+static bool sev_enabled = true;
 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);
+static bool sev_es_enabled = true;
 module_param_named(sev_es, sev_es_enabled, bool, 0444);
 #else
 #define sev_enabled false
-- 
2.31.1.498.g6c1eba8ee3d-goog

[PATCH v5 10/15] 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.

Reviewed by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
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 cb227e90dffb..f5684d24e333 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -879,8 +879,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.31.1.498.g6c1eba8ee3d-goog

[PATCH v5 11/15] 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.

Reviewed by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
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 bd26e564549c..8efbd23f771b 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1799,8 +1799,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.31.1.498.g6c1eba8ee3d-goog

[PATCH v5 12/15] 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.

Reviewed by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 12 ++++++++++++
 arch/x86/kvm/svm/svm.c | 16 ++++++++--------
 arch/x86/kvm/svm/svm.h |  1 +
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 8efbd23f771b..68999085db6e 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1853,6 +1853,18 @@ 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 = kcalloc(max_sev_asid + 1, sizeof(void *), GFP_KERNEL);
+	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 f5684d24e333..a5f994e1ca50 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -544,22 +544,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 = kcalloc(max_sev_asid + 1, sizeof(void *),
-					GFP_KERNEL);
-		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;
 
@@ -569,7 +569,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 39d1412f2c45..0af638f97b5f 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -584,6 +584,7 @@ void pre_sev_run(struct vcpu_svm *svm, int cpu);
 void __init sev_set_cpu_caps(void);
 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 kvm_vcpu *vcpu);
 int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in);
-- 
2.31.1.498.g6c1eba8ee3d-goog

[PATCH v5 13/15] 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).

Reviewed by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
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 68999085db6e..4440459cf8e3 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1457,7 +1457,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)
@@ -1844,7 +1844,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);
@@ -1855,7 +1855,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 = kcalloc(max_sev_asid + 1, sizeof(void *), GFP_KERNEL);
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 0af638f97b5f..f455784519d7 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -568,11 +568,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.31.1.498.g6c1eba8ee3d-goog

[PATCH v5 14/15] 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.

Reviewed by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
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 4440459cf8e3..5cdfea8b1c47 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -42,7 +42,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.31.1.498.g6c1eba8ee3d-goog

[PATCH v5 15/15] 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 | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 5cdfea8b1c47..d65193a4ea17 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -58,9 +58,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,
@@ -87,14 +92,7 @@ static inline bool is_mirroring_enc_context(struct kvm *kvm)
 /* 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 */
@@ -1846,10 +1844,11 @@ void sev_hardware_teardown(void)
 	if (!sev_enabled)
 		return;
 
+	/* No need to take sev_bitmap_lock, all VMs have been destroyed. */
+	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.31.1.498.g6c1eba8ee3d-goog

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

[Paolo Bonzini]

On 22/04/21 04:11, Sean Christopherson wrote:
> +	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;

There's a tiny bug here which would cause sev_flush_asids to return 0
if there are reclaimed SEV ASIDs and the caller is looking for an SEV-ES
ASID, or vice versa.  The bug used to be in __sev_recycle_asids, you're
just moving the code.

It's not a big deal because sev_asid_new only retries once, but we might
as well fix it:

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 02b3426a9e39..403c6991e67c 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -74,12 +74,13 @@ struct enc_region {
  	unsigned long size;
  };
  
+/* Called with the sev_bitmap_lock held, or on shutdown  */
  static int sev_flush_asids(int min_asid, int max_asid)
  {
  	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);
+	pos = find_next_bit(sev_reclaim_asid_bitmap, max_asid, min_asid);
  	if (pos >= max_asid)
  		return -EBUSY;
  

Paolo

>   	/*
>   	 * DEACTIVATE will clear the WBINVD indicator causing DF_FLUSH to fail,
> @@ -87,14 +92,7 @@ static inline bool is_mirroring_enc_context(struct kvm *kvm)
>   /* 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 */
> @@ -1846,10 +1844,11 @@ void sev_hardware_teardown(void)
>   	if (!sev_enabled)
>   		return;
>   
> +	/* No need to take sev_bitmap_lock, all VMs have been destroyed. */
> +	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)
> 

Re: [PATCH v5 00/15] KVM: SVM: Misc SEV cleanups

[Paolo Bonzini]

On 22/04/21 04:11, Sean Christopherson wrote:
> 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 kvm/queue-ish, commit 0e91d1992235 ("KVM: SVM: Allocate SEV command
> structures on local stack"), to avoid the conflicting CPUID.0x8000_001F
> patch sitting in kvm/queue.
> 
> v5:
>   - Use Paolo's version of the CPUID.0x8000_001F patch, with some of my
>     goo on top.  Paolo gets credit by introducing fewer bugs; v4 missed
>     the SEV/SEV-ES module params and used the wrong reverse-CPUID index...
>   - Add a patch to disable SEV/SEV-ES if NPT is disabled.
>   - Rebased, as above.
> v4:
>   - Reinstate the patch to override CPUID.0x8000_001F.
>   - Properly configure the CPUID.0x8000_001F override. [Paolo]
>   - Rebase to v5.12-rc1-dontuse.
> v3:
>   - Drop two patches: add a dedicated feature word for CPUID_0x8000001F,
>     and use the new word to mask host CPUID in KVM.  I'll send these as a
>     separate mini-series so that Boris can take them through tip.
>   - Add a patch to remove dependency on
>     CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT. [Boris / Paolo]
>   - Use kcalloc() instead of an open-coded equivalent. [Tom]
>   - Nullify sev_asid_bitmap when freeing it during setup. [Tom]
>   - Add a comment in sev_hardware_teardown() to document why it's safe to
>     query the ASID bitmap without taking the lock. [Tom]
>   - Collect reviews. [Tom and Brijesh]
> 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
> 
> Paolo Bonzini (1):
>    KVM: SEV: Mask CPUID[0x8000001F].eax according to supported features
> 
> 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: Disable SEV/SEV-ES if NPT is disabled
>    KVM: SVM: Move SEV module params/variables to sev.c
>    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: Enable SEV/SEV-ES functionality by default (when supported)
>    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/mem_encrypt.h |  1 -
>   arch/x86/kvm/cpuid.c               |  8 ++-
>   arch/x86/kvm/cpuid.h               |  1 +
>   arch/x86/kvm/svm/sev.c             | 80 ++++++++++++++++++++++--------
>   arch/x86/kvm/svm/svm.c             | 57 +++++++++------------
>   arch/x86/kvm/svm/svm.h             |  9 +---
>   arch/x86/mm/mem_encrypt.c          | 12 ++---
>   arch/x86/mm/mem_encrypt_identity.c |  1 -
>   8 files changed, 97 insertions(+), 72 deletions(-)
> 

Queued except for patch 6, send that separately since it's purely x86 
and maintainers will likely not notice it inside this thread.  You 
probably want to avoid conflicts by waiting for the migration patches, 
though.

Paolo

Re: [PATCH v5 00/15] KVM: SVM: Misc SEV cleanups

[Sean Christopherson]

On Thu, Apr 22, 2021, Paolo Bonzini wrote:
> > Paolo Bonzini (1):
> >    KVM: SEV: Mask CPUID[0x8000001F].eax according to supported features
> > 
> > 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: Disable SEV/SEV-ES if NPT is disabled
> >    KVM: SVM: Move SEV module params/variables to sev.c
> >    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: Enable SEV/SEV-ES functionality by default (when supported)
> >    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/mem_encrypt.h |  1 -
> >   arch/x86/kvm/cpuid.c               |  8 ++-
> >   arch/x86/kvm/cpuid.h               |  1 +
> >   arch/x86/kvm/svm/sev.c             | 80 ++++++++++++++++++++++--------
> >   arch/x86/kvm/svm/svm.c             | 57 +++++++++------------
> >   arch/x86/kvm/svm/svm.h             |  9 +---
> >   arch/x86/mm/mem_encrypt.c          | 12 ++---
> >   arch/x86/mm/mem_encrypt_identity.c |  1 -
> >   8 files changed, 97 insertions(+), 72 deletions(-)
> > 
> 
> Queued except for patch 6, send that separately since it's purely x86 and
> maintainers will likely not notice it inside this thread.  You probably want
> to avoid conflicts by waiting for the migration patches, though.

It can't be sent separately, having both the kernel's sev_enabled and KVM's
sev_enabled doesn't build with CONFIG_AMD_MEM_ENCRYPT=y:

arch/x86/kvm/svm/sev.c:33:13: error: static declaration of ‘sev_enabled’ follows non-static declaration
   33 | static bool sev_enabled = true;
      |             ^~~~~~~~~~~
In file included from include/linux/mem_encrypt.h:17,
                 from arch/x86/include/asm/page_types.h:7,
                 from arch/x86/include/asm/page.h:9,
                 from arch/x86/include/asm/thread_info.h:12,
                 from include/linux/thread_info.h:58,
                 from arch/x86/include/asm/preempt.h:7,
                 from include/linux/preempt.h:78,
                 from include/linux/percpu.h:6,
                 from include/linux/context_tracking_state.h:5,
                 from include/linux/hardirq.h:5,
                 from include/linux/kvm_host.h:7,
                 from arch/x86/kvm/svm/sev.c:11:
arch/x86/include/asm/mem_encrypt.h:23:13: note: previous declaration of ‘sev_enabled’ was here
   23 | extern bool sev_enabled;
      |             ^~~~~~~~~~~
make[3]: *** [scripts/Makefile.build:271: arch/x86/kvm/svm/sev.o] Error 1

Re: [PATCH v5 00/15] KVM: SVM: Misc SEV cleanups

[Paolo Bonzini]

On 22/04/21 18:02, Sean Christopherson wrote:
> On Thu, Apr 22, 2021, Paolo Bonzini wrote:
>>> Paolo Bonzini (1):
>>>     KVM: SEV: Mask CPUID[0x8000001F].eax according to supported features
>>>
>>> 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: Disable SEV/SEV-ES if NPT is disabled
>>>     KVM: SVM: Move SEV module params/variables to sev.c
>>>     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: Enable SEV/SEV-ES functionality by default (when supported)
>>>     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/mem_encrypt.h |  1 -
>>>    arch/x86/kvm/cpuid.c               |  8 ++-
>>>    arch/x86/kvm/cpuid.h               |  1 +
>>>    arch/x86/kvm/svm/sev.c             | 80 ++++++++++++++++++++++--------
>>>    arch/x86/kvm/svm/svm.c             | 57 +++++++++------------
>>>    arch/x86/kvm/svm/svm.h             |  9 +---
>>>    arch/x86/mm/mem_encrypt.c          | 12 ++---
>>>    arch/x86/mm/mem_encrypt_identity.c |  1 -
>>>    8 files changed, 97 insertions(+), 72 deletions(-)
>>>
>>
>> Queued except for patch 6, send that separately since it's purely x86 and
>> maintainers will likely not notice it inside this thread.  You probably want
>> to avoid conflicts by waiting for the migration patches, though.
> 
> It can't be sent separately, having both the kernel's sev_enabled and KVM's
> sev_enabled doesn't build with CONFIG_AMD_MEM_ENCRYPT=y:

I discovered just as much a few hours later, but Boris has acked it 
already so it's all set.

Paolo

> arch/x86/kvm/svm/sev.c:33:13: error: static declaration of ‘sev_enabled’ follows non-static declaration
>     33 | static bool sev_enabled = true;
>        |             ^~~~~~~~~~~
> In file included from include/linux/mem_encrypt.h:17,
>                   from arch/x86/include/asm/page_types.h:7,
>                   from arch/x86/include/asm/page.h:9,
>                   from arch/x86/include/asm/thread_info.h:12,
>                   from include/linux/thread_info.h:58,
>                   from arch/x86/include/asm/preempt.h:7,
>                   from include/linux/preempt.h:78,
>                   from include/linux/percpu.h:6,
>                   from include/linux/context_tracking_state.h:5,
>                   from include/linux/hardirq.h:5,
>                   from include/linux/kvm_host.h:7,
>                   from arch/x86/kvm/svm/sev.c:11:
> arch/x86/include/asm/mem_encrypt.h:23:13: note: previous declaration of ‘sev_enabled’ was here
>     23 | extern bool sev_enabled;
>        |             ^~~~~~~~~~~
> make[3]: *** [scripts/Makefile.build:271: arch/x86/kvm/svm/sev.o] Error 1
>