[PATCH 00/13] KVM: SVM: Misc SEV cleanups

[PATCH 00/13] 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").

Not super well tested, but AFAICT the feature detection is working as
expected.

Boris, this obviously touches on the KVM vs. kernel _cpu_has() stuff as
well.  My thought is that we can judge the SME/SEV features solely on
whether or the kernel wants to dedicated a word for 'em, and hash out what
to do with KVM at large in the SGX thread.

Sean Christopherson (13):
  KVM: SVM: Free sev_asid_bitmap during init if SEV setup fails
  KVM: SVM: Zero out the VMCB array used to track SEV ASID association
  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: Rename global "sev_enabled" flag to "sev_guest"
  KVM: SVM: Append "_enabled" to module-scoped SEV/SEV-ES control
    variables
  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            |  2 +-
 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                        | 64 +++++++++++++------
 arch/x86/kvm/svm/svm.c                        | 35 +++-------
 arch/x86/kvm/svm/svm.h                        |  8 +--
 arch/x86/mm/mem_encrypt.c                     |  4 +-
 arch/x86/mm/mem_encrypt_identity.c            |  2 +-
 .../arch/x86/include/asm/disabled-features.h  |  3 +-
 .../arch/x86/include/asm/required-features.h  |  3 +-
 16 files changed, 88 insertions(+), 74 deletions(-)

-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH 01/13] 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
it will be leaked as sev_hardware_teardown() frees the bitmaps if and
only if SEV is fully enabled (which obviously isn't the case if SEV
setup fails).

Fixes: 33af3a7ef9e6 ("KVM: SVM: Reduce WBINVD/DF_FLUSH invocations")
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: stable@vger.kernel.org
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

Re: [PATCH 01/13] KVM: SVM: Free sev_asid_bitmap during init if SEV setup fails

[Tom Lendacky]

On 1/8/21 6:47 PM, Sean Christopherson wrote:
> Free sev_asid_bitmap if the reclaim bitmap allocation fails, othwerise
> it will be leaked as sev_hardware_teardown() frees the bitmaps if and
> only if SEV is fully enabled (which obviously isn't the case if SEV
> setup fails).

The svm_sev_enabled() function is only based on CONFIG_KVM_AMD_SEV and 
max_sev_asid. So sev_hardware_teardown() should still free everything if 
it was allocated since we never change max_sev_asid, no?

Thanks,
Tom

> 
> Fixes: 33af3a7ef9e6 ("KVM: SVM: Reduce WBINVD/DF_FLUSH invocations")
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: stable@vger.kernel.org
> 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;
> 

Re: [PATCH 01/13] KVM: SVM: Free sev_asid_bitmap during init if SEV setup fails

[Sean Christopherson]

On Mon, Jan 11, 2021, Tom Lendacky wrote:
> On 1/8/21 6:47 PM, Sean Christopherson wrote:
> > Free sev_asid_bitmap if the reclaim bitmap allocation fails, othwerise
> > it will be leaked as sev_hardware_teardown() frees the bitmaps if and
> > only if SEV is fully enabled (which obviously isn't the case if SEV
> > setup fails).
> 
> The svm_sev_enabled() function is only based on CONFIG_KVM_AMD_SEV and
> max_sev_asid. So sev_hardware_teardown() should still free everything if it
> was allocated since we never change max_sev_asid, no?

Aha!  You're correct.  This is needed once svm_sev_enabled() is gone, but is not
an actual bug in upstream.

I created the commit before the long New Years weekend, but stupidly didn't
write a changelog.  I then forgot that my series effectively introduce the bug
and incorrectly moved this patch to the front and treated it as an upstream bug
fix when retroactively writing the changelog.

Thanks!

[PATCH 02/13] 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 03/13] 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

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

[Vitaly Kuznetsov]

Sean Christopherson <seanjc@google.com> writes:

> 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);

Two stupid questions (and not really related to your patch) for
self-eduacation if I may:

1) Why do we rely on CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT (which
sound like it control the guest side of things) to set defaults here? 

2) It appears to be possible to do 'modprobe kvm_amd sev=0 sev_es=1' and
this looks like a bogus configuration, should we make an effort to
validate the correctness upon module load?

> +
>  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);

-- 
Vitaly

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

[Tom Lendacky]

On 1/11/21 4:42 AM, Vitaly Kuznetsov wrote:
> Sean Christopherson <seanjc@google.com> writes:
> 
>> 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);
> 
> Two stupid questions (and not really related to your patch) for
> self-eduacation if I may:
> 
> 1) Why do we rely on CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT (which
> sound like it control the guest side of things) to set defaults here?

I thought it was a review comment, but I'm not able to find it now.

Brijesh probably remembers better than me.

> 
> 2) It appears to be possible to do 'modprobe kvm_amd sev=0 sev_es=1' and
> this looks like a bogus configuration, should we make an effort to
> validate the correctness upon module load?

This will still result in an overall sev=0 sev_es=0. Is the question just 
about issuing a message based on the initial values specified?

Thanks,
Tom

> 
>> +
>>   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 03/13] KVM: SVM: Move SEV module params/variables to sev.c

[Vitaly Kuznetsov]

Tom Lendacky <thomas.lendacky@amd.com> writes:

> On 1/11/21 4:42 AM, Vitaly Kuznetsov wrote:
>> Sean Christopherson <seanjc@google.com> writes:
>> 
>>> 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);
>> 
>> Two stupid questions (and not really related to your patch) for
>> self-eduacation if I may:
>> 
>> 1) Why do we rely on CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT (which
>> sound like it control the guest side of things) to set defaults here?
>
> I thought it was a review comment, but I'm not able to find it now.
>
> Brijesh probably remembers better than me.
>
>> 
>> 2) It appears to be possible to do 'modprobe kvm_amd sev=0 sev_es=1' and
>> this looks like a bogus configuration, should we make an effort to
>> validate the correctness upon module load?
>
> This will still result in an overall sev=0 sev_es=0. Is the question just 
> about issuing a message based on the initial values specified?
>

Yes, as one may expect the result will be that SEV-ES guests work and
plain SEV don't.

-- 
Vitaly

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

[Sean Christopherson]

On Mon, Jan 11, 2021, Vitaly Kuznetsov wrote:
> Tom Lendacky <thomas.lendacky@amd.com> writes:
> 
> > On 1/11/21 4:42 AM, Vitaly Kuznetsov wrote:
> >> Sean Christopherson <seanjc@google.com> writes:
> >> 
> >>> 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);
> >> 
> >> Two stupid questions (and not really related to your patch) for
> >> self-eduacation if I may:
> >> 
> >> 1) Why do we rely on CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT (which
> >> sound like it control the guest side of things) to set defaults here?
> >
> > I thought it was a review comment, but I'm not able to find it now.
> >
> > Brijesh probably remembers better than me.
> >
> >> 
> >> 2) It appears to be possible to do 'modprobe kvm_amd sev=0 sev_es=1' and
> >> this looks like a bogus configuration, should we make an effort to
> >> validate the correctness upon module load?
> >
> > This will still result in an overall sev=0 sev_es=0. Is the question just 
> > about issuing a message based on the initial values specified?
> >
> 
> Yes, as one may expect the result will be that SEV-ES guests work and
> plain SEV don't.

KVM doesn't issue messages when it overrides other module params due to
disable requirements, e.g. ept=0 unrestricted_guest=1 is roughly equivalent.
Not that what KVM currently does is right, but at least it's consistent. :-)

And on the other hand, I think it's reasonable to expect that specifying only
sev=0 is sufficient to disable both SEV and SEV-ES, e.g. to turn them off when
they're enabled by default.

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

[Tom Lendacky]

On 1/8/21 6:47 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(-)
> 
> 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();

I believe the reason for the original if statement was similar to:

  853c110982ea ("KVM: x86: support CONFIG_KVM_AMD=y with CONFIG_CRYPTO_DEV_CCP_DD=m")

But with the removal of sev_platform_status() from sev_hardware_setup(),
I think it's ok to call sev_hardware_setup() no matter what now.

Thanks,
Tom

>   
>   	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);
> 

[PATCH 04/13] 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 05/13] 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 06/13] x86/sev: Rename global "sev_enabled" flag to "sev_guest"

[Sean Christopherson]

Use "guest" instead of "enabled" for the global "running as an SEV guest"
flag to avoid confusion over whether "sev_enabled" refers to the guest or
the host.  This 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 | 2 +-
 arch/x86/mm/mem_encrypt.c          | 4 ++--
 arch/x86/mm/mem_encrypt_identity.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index 2f62bbdd9d12..9b3990928674 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -20,7 +20,7 @@
 
 extern u64 sme_me_mask;
 extern u64 sev_status;
-extern bool sev_enabled;
+extern bool sev_guest;
 
 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..0f798355de03 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -44,7 +44,7 @@ EXPORT_SYMBOL(sme_me_mask);
 DEFINE_STATIC_KEY_FALSE(sev_enable_key);
 EXPORT_SYMBOL_GPL(sev_enable_key);
 
-bool sev_enabled __section(".data");
+bool sev_guest __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);
@@ -344,7 +344,7 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size)
  */
 bool sme_active(void)
 {
-	return sme_me_mask && !sev_enabled;
+	return sme_me_mask && !sev_guest;
 }
 
 bool sev_active(void)
diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c
index 6c5eb6f3f14f..91b6b899c02b 100644
--- a/arch/x86/mm/mem_encrypt_identity.c
+++ b/arch/x86/mm/mem_encrypt_identity.c
@@ -545,7 +545,7 @@ 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;
+		sev_guest = true;
 		physical_mask &= ~sme_me_mask;
 		return;
 	}
-- 
2.30.0.284.gd98b1dd5eaa7-goog

Re: [PATCH 06/13] x86/sev: Rename global "sev_enabled" flag to "sev_guest"

[Tom Lendacky]

On 1/8/21 6:47 PM, Sean Christopherson wrote:
> Use "guest" instead of "enabled" for the global "running as an SEV guest"
> flag to avoid confusion over whether "sev_enabled" refers to the guest or
> the host.  This will also allow KVM to usurp "sev_enabled" for its own
> purposes.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

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

> ---
>   arch/x86/include/asm/mem_encrypt.h | 2 +-
>   arch/x86/mm/mem_encrypt.c          | 4 ++--
>   arch/x86/mm/mem_encrypt_identity.c | 2 +-
>   3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> index 2f62bbdd9d12..9b3990928674 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -20,7 +20,7 @@
>   
>   extern u64 sme_me_mask;
>   extern u64 sev_status;
> -extern bool sev_enabled;
> +extern bool sev_guest;
>   
>   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..0f798355de03 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -44,7 +44,7 @@ EXPORT_SYMBOL(sme_me_mask);
>   DEFINE_STATIC_KEY_FALSE(sev_enable_key);
>   EXPORT_SYMBOL_GPL(sev_enable_key);
>   
> -bool sev_enabled __section(".data");
> +bool sev_guest __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);
> @@ -344,7 +344,7 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size)
>    */
>   bool sme_active(void)
>   {
> -	return sme_me_mask && !sev_enabled;
> +	return sme_me_mask && !sev_guest;
>   }
>   
>   bool sev_active(void)
> diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c
> index 6c5eb6f3f14f..91b6b899c02b 100644
> --- a/arch/x86/mm/mem_encrypt_identity.c
> +++ b/arch/x86/mm/mem_encrypt_identity.c
> @@ -545,7 +545,7 @@ 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;
> +		sev_guest = true;
>   		physical_mask &= ~sme_me_mask;
>   		return;
>   	}
> 

Re: [PATCH 06/13] x86/sev: Rename global "sev_enabled" flag to "sev_guest"

[Tom Lendacky]

On 1/11/21 10:02 AM, Tom Lendacky wrote:
> On 1/8/21 6:47 PM, Sean Christopherson wrote:
>> Use "guest" instead of "enabled" for the global "running as an SEV guest"
>> flag to avoid confusion over whether "sev_enabled" refers to the guest or
>> the host.  This will also allow KVM to usurp "sev_enabled" for its own
>> purposes.
>>
>> No functional change intended.
>>
>> Signed-off-by: Sean Christopherson <seanjc@google.com>
> 
> Acked-by: Tom Lendacky <thomas.lendacky@amd.com>

Ah, I tried building with CONFIG_KVM=y and CONFIG_KVM_AMD=y and got a 
build error:

In file included from arch/x86/kvm/svm/svm.c:43:
arch/x86/kvm/svm/svm.h:222:20: error: ‘sev_guest’ redeclared as different 
kind of symbol
   222 | static inline bool sev_guest(struct kvm *kvm)
       |                    ^~~~~~~~~
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:38,
                  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/svm.c:3:
./arch/x86/include/asm/mem_encrypt.h:23:13: note: previous declaration of 
‘sev_guest’ was here
    23 | extern bool sev_guest;
       |             ^~~~~~~~~

Thanks,
Tom

> 
>> ---
>>   arch/x86/include/asm/mem_encrypt.h | 2 +-
>>   arch/x86/mm/mem_encrypt.c          | 4 ++--
>>   arch/x86/mm/mem_encrypt_identity.c | 2 +-
>>   3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/mem_encrypt.h 
>> b/arch/x86/include/asm/mem_encrypt.h
>> index 2f62bbdd9d12..9b3990928674 100644
>> --- a/arch/x86/include/asm/mem_encrypt.h
>> +++ b/arch/x86/include/asm/mem_encrypt.h
>> @@ -20,7 +20,7 @@
>>   extern u64 sme_me_mask;
>>   extern u64 sev_status;
>> -extern bool sev_enabled;
>> +extern bool sev_guest;
>>   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..0f798355de03 100644
>> --- a/arch/x86/mm/mem_encrypt.c
>> +++ b/arch/x86/mm/mem_encrypt.c
>> @@ -44,7 +44,7 @@ EXPORT_SYMBOL(sme_me_mask);
>>   DEFINE_STATIC_KEY_FALSE(sev_enable_key);
>>   EXPORT_SYMBOL_GPL(sev_enable_key);
>> -bool sev_enabled __section(".data");
>> +bool sev_guest __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);
>> @@ -344,7 +344,7 @@ int __init early_set_memory_encrypted(unsigned long 
>> vaddr, unsigned long size)
>>    */
>>   bool sme_active(void)
>>   {
>> -    return sme_me_mask && !sev_enabled;
>> +    return sme_me_mask && !sev_guest;
>>   }
>>   bool sev_active(void)
>> diff --git a/arch/x86/mm/mem_encrypt_identity.c 
>> b/arch/x86/mm/mem_encrypt_identity.c
>> index 6c5eb6f3f14f..91b6b899c02b 100644
>> --- a/arch/x86/mm/mem_encrypt_identity.c
>> +++ b/arch/x86/mm/mem_encrypt_identity.c
>> @@ -545,7 +545,7 @@ 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;
>> +        sev_guest = true;
>>           physical_mask &= ~sme_me_mask;
>>           return;
>>       }
>>

Re: [PATCH 06/13] x86/sev: Rename global "sev_enabled" flag to "sev_guest"

[Sean Christopherson]

On Mon, Jan 11, 2021, Tom Lendacky wrote:
> On 1/11/21 10:02 AM, Tom Lendacky wrote:
> > On 1/8/21 6:47 PM, Sean Christopherson wrote:
> > > Use "guest" instead of "enabled" for the global "running as an SEV guest"
> > > flag to avoid confusion over whether "sev_enabled" refers to the guest or
> > > the host.  This will also allow KVM to usurp "sev_enabled" for its own
> > > purposes.
> > > 
> > > No functional change intended.
> > > 
> > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > 
> > Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
> 
> Ah, I tried building with CONFIG_KVM=y and CONFIG_KVM_AMD=y and got a build
> error:
> 
> In file included from arch/x86/kvm/svm/svm.c:43:
> arch/x86/kvm/svm/svm.h:222:20: error: ‘sev_guest’ redeclared as different
> kind of symbol

Dang, didn't consider that scenario, obviously.  The irony of introducing a
conflict while trying to avoid conflicts...

Re: [PATCH 06/13] x86/sev: Rename global "sev_enabled" flag to "sev_guest"

[Sean Christopherson]

On Mon, Jan 11, 2021, Sean Christopherson wrote:
> On Mon, Jan 11, 2021, Tom Lendacky wrote:
> > On 1/11/21 10:02 AM, Tom Lendacky wrote:
> > > On 1/8/21 6:47 PM, Sean Christopherson wrote:
> > > > Use "guest" instead of "enabled" for the global "running as an SEV guest"
> > > > flag to avoid confusion over whether "sev_enabled" refers to the guest or
> > > > the host.  This will also allow KVM to usurp "sev_enabled" for its own
> > > > purposes.
> > > > 
> > > > No functional change intended.
> > > > 
> > > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > 
> > > Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
> > 
> > Ah, I tried building with CONFIG_KVM=y and CONFIG_KVM_AMD=y and got a build
> > error:
> > 
> > In file included from arch/x86/kvm/svm/svm.c:43:
> > arch/x86/kvm/svm/svm.h:222:20: error: ‘sev_guest’ redeclared as different
> > kind of symbol
> 
> Dang, didn't consider that scenario, obviously.  The irony of introducing a
> conflict while trying to avoid conflicts...

Even better than coming up with a new name, sev_enabled can be removed entirely
as it's really just sev_active() stored in a separate bool.

[PATCH 07/13] 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.

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

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

[Tom Lendacky]

On 1/8/21 6:47 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.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Acked-by: Tom Lendacky <thomas.lendacky@amd.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)
> 

[PATCH 08/13] 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 09/13] 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 a024edabaca5..3d25d24bcb48 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1272,8 +1272,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 10/13] 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 3d25d24bcb48..8c34c467a09d 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1323,6 +1323,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 11/13] 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 8c34c467a09d..1b9174a49b65 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -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_enabled)
+	if (!sev_enabled)
 		return -ENOTTY;
 
 	if (!argp)
@@ -1314,7 +1314,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);
@@ -1325,7 +1325,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

Re: [PATCH 11/13] KVM: SVM: Drop redundant svm_sev_enabled() helper

[Tom Lendacky]

On 1/8/21 6:47 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(-)
> 

With CONFIG_KVM=y, CONFIG_KVM_AMD=y and CONFIG_CRYPTO_DEV_CCP_DD=m, I get
the following build warning:

make: Entering directory '/root/kernels/kvm-build-x86_64'
  DESCEND  objtool               
  CALL    scripts/atomic/check-atomics.sh
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h                                                           
  CC      arch/x86/kvm/svm/svm.o                                                                                                                                                                
  CC      arch/x86/kvm/svm/nested.o                                                             
  CC      arch/x86/kvm/svm/avic.o                                                                                                                                                               
  CC      arch/x86/kvm/svm/sev.o          
In file included from ./include/linux/cpumask.h:12,     
                 from ./arch/x86/include/asm/cpumask.h:5,
                 from ./arch/x86/include/asm/msr.h:11,
                 from ./arch/x86/include/asm/processor.h:22,
                 from ./arch/x86/include/asm/cpufeature.h:5,                                                                                                                                    
                 from ./arch/x86/include/asm/thread_info.h:53,
                 from ./include/linux/thread_info.h:38,
                 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:
In function ‘bitmap_zero’,
    inlined from ‘__sev_recycle_asids’ at arch/x86/kvm/svm/sev.c:92:2,
    inlined from ‘sev_asid_new’ at arch/x86/kvm/svm/sev.c:113:16,
    inlined from ‘sev_guest_init’ at arch/x86/kvm/svm/sev.c:195:9:
./include/linux/bitmap.h:238:2: warning: argument 1 null where non-null expected [-Wnonnull]
  238 |  memset(dst, 0, len);
      |  ^~~~~~~~~~~~~~~~~~~
In file included from ./arch/x86/include/asm/string.h:5,
                 from ./include/linux/string.h:20,
                 from ./include/linux/bitmap.h:9,
                 from ./include/linux/cpumask.h:12,
                 from ./arch/x86/include/asm/cpumask.h:5,
                 from ./arch/x86/include/asm/msr.h:11,
                 from ./arch/x86/include/asm/processor.h:22,
                 from ./arch/x86/include/asm/cpufeature.h:5,
                 from ./arch/x86/include/asm/thread_info.h:53,
                 from ./include/linux/thread_info.h:38,
                 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/kvm/svm/sev.c: In function ‘sev_guest_init’:
./arch/x86/include/asm/string_64.h:18:7: note: in a call to function ‘memset’ declared here
   18 | void *memset(void *s, int c, size_t n);
      |       ^~~~~~

Thanks,
Tom

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 8c34c467a09d..1b9174a49b65 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -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_enabled)
> +	if (!sev_enabled)
>   		return -ENOTTY;
>   
>   	if (!argp)
> @@ -1314,7 +1314,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);
> @@ -1325,7 +1325,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 11/13] KVM: SVM: Drop redundant svm_sev_enabled() helper

[Sean Christopherson]

On Mon, Jan 11, 2021, Tom Lendacky wrote:
> On 1/8/21 6:47 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(-)
> > 
> 
> With CONFIG_KVM=y, CONFIG_KVM_AMD=y and CONFIG_CRYPTO_DEV_CCP_DD=m, I get
> the following build warning:

...

> In function ‘bitmap_zero’,
>     inlined from ‘__sev_recycle_asids’ at arch/x86/kvm/svm/sev.c:92:2,
>     inlined from ‘sev_asid_new’ at arch/x86/kvm/svm/sev.c:113:16,
>     inlined from ‘sev_guest_init’ at arch/x86/kvm/svm/sev.c:195:9:
> ./include/linux/bitmap.h:238:2: warning: argument 1 null where non-null expected [-Wnonnull]
>   238 |  memset(dst, 0, len);
>       |  ^~~~~~~~~~~~~~~~~~~

Ah, because that config "silently" disables CONFIG_KVM_AMD_SEV.  The warning
pops up because svm_sev_enabled() included !IS_ENABLED(CONFIG_KVM_AMD_SEV) and
that was enough for the compiler to understand that svm_mem_enc_op() was a nop.

That being said, unless I'm missing something, this is a false positive the
compiler's part, e.g. the warning occurs even if sev_enabled is false be default,
i.e. CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT=n.

Anyways, I'm leaning towards "fixing" this by defining sev_enabled and
sev_es_enabled to false if CONFIG_KVM_AMD_SEV=n.  It'd be a worthwhile change to
condition the default values on CONFIG_KVM_AMD_SEV anyways, so it'd kill two
birds with one stone.  Long term, I'm tempted to exporing conditioning all of
sev.c on CONFIG_KVM_AMD_SEV=y, but there are just enough functions exposed via
svm.h that make me think it wouldn't be worth the effort.

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 1b9174a49b65..7e14514dd083 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? */
@@ -1310,6 +1316,7 @@ void __init sev_hardware_setup(void)
 out:
        sev_enabled = sev_supported;
        sev_es_enabled = sev_es_supported;
+#endif
 }

 void sev_hardware_teardown(void)

[PATCH 12/13] 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 1b9174a49b65..b4a9c12cf8ce 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -36,7 +36,6 @@ 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);
 static DECLARE_RWSEM(sev_deactivate_lock);
 static DEFINE_MUTEX(sev_bitmap_lock);
 unsigned int max_sev_asid;
-- 
2.30.0.284.gd98b1dd5eaa7-goog

[PATCH 13/13] 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 b4a9c12cf8ce..eb8e4dca4bf2 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -51,9 +51,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,
@@ -75,14 +80,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 */
@@ -1316,10 +1314,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