diff mbox series

[Part1,RFC,v2,09/20] x86/sev: check SEV-SNP features support

Message ID 20210430121616.2295-10-brijesh.singh@amd.com (mailing list archive)
State New, archived
Headers show
Series Add AMD Secure Nested Paging (SEV-SNP) Guest Support | expand

Commit Message

Brijesh Singh April 30, 2021, 12:16 p.m. UTC
Version 2 of the GHCB specification added the advertisement of features
that are supported by the hypervisor. If hypervisor supports the SEV-SNP
then it must set the SEV-SNP features bit to indicate that the base
SEV-SNP is supported.

Check the SEV-SNP feature while establishing the GHCB, if failed,
terminate the guest.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 arch/x86/boot/compressed/sev.c | 21 +++++++++++++++++++++
 arch/x86/kernel/sev-shared.c   | 13 ++++++++++++-
 arch/x86/kernel/sev.c          |  4 ++++
 3 files changed, 37 insertions(+), 1 deletion(-)

Comments

Borislav Petkov May 20, 2021, 4:02 p.m. UTC | #1
On Fri, Apr 30, 2021 at 07:16:05AM -0500, Brijesh Singh wrote:
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index 6d9055427f37..7badbeb6cb95 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -25,6 +25,8 @@
>  
>  struct ghcb boot_ghcb_page __aligned(PAGE_SIZE);
>  struct ghcb *boot_ghcb;
> +static u64 sev_status_val;

msr_sev_status should be more descriptive.

> +static bool sev_status_checked;

You don't need this one - you can simply do

	if (!msr_sev_status)
		read the MSR.

>  /*
>   * Copy a version of this function here - insn-eval.c can't be used in
> @@ -119,11 +121,30 @@ static enum es_result vc_read_mem(struct es_em_ctxt *ctxt,
>  /* Include code for early handlers */
>  #include "../../kernel/sev-shared.c"
>  
> +static inline bool sev_snp_enabled(void)
> +{
> +	unsigned long low, high;
> +
> +	if (!sev_status_checked) {
> +		asm volatile("rdmsr\n"
> +			     : "=a" (low), "=d" (high)
> +			     : "c" (MSR_AMD64_SEV));
> +		sev_status_val = (high << 32) | low;
> +		sev_status_checked = true;
> +	}
> +
> +	return sev_status_val & MSR_AMD64_SEV_SNP_ENABLED ? true : false;

	return msr_sev_status & MSR_AMD64_SEV_SNP_ENABLED;

is enough.

> +}
> +
>  static bool early_setup_sev_es(void)
>  {
>  	if (!sev_es_negotiate_protocol())
>  		sev_es_terminate(0, GHCB_SEV_ES_REASON_PROTOCOL_UNSUPPORTED);
>  
> +	/* If SEV-SNP is enabled then check if the hypervisor supports the SEV-SNP features. */

80 cols like the rest of this file pls.

> +	if (sev_snp_enabled() && !sev_snp_check_hypervisor_features())
> +		sev_es_terminate(0, GHCB_SEV_ES_REASON_SNP_UNSUPPORTED);
> +
>  	if (set_page_decrypted((unsigned long)&boot_ghcb_page))
>  		return false;

Thx.
Brijesh Singh May 20, 2021, 5:40 p.m. UTC | #2
On 5/20/21 11:02 AM, Borislav Petkov wrote:
> On Fri, Apr 30, 2021 at 07:16:05AM -0500, Brijesh Singh wrote:
>> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
>> index 6d9055427f37..7badbeb6cb95 100644
>> --- a/arch/x86/boot/compressed/sev.c
>> +++ b/arch/x86/boot/compressed/sev.c
>> @@ -25,6 +25,8 @@
>>  
>>  struct ghcb boot_ghcb_page __aligned(PAGE_SIZE);
>>  struct ghcb *boot_ghcb;
>> +static u64 sev_status_val;
> msr_sev_status should be more descriptive.
Noted.
>
>> +static bool sev_status_checked;
> You don't need this one - you can simply do
>
> 	if (!msr_sev_status)
> 		read the MSR.

Agreed.


>
>>  /*
>>   * Copy a version of this function here - insn-eval.c can't be used in
>> @@ -119,11 +121,30 @@ static enum es_result vc_read_mem(struct es_em_ctxt *ctxt,
>>  /* Include code for early handlers */
>>  #include "../../kernel/sev-shared.c"
>>  
>> +static inline bool sev_snp_enabled(void)
>> +{
>> +	unsigned long low, high;
>> +
>> +	if (!sev_status_checked) {
>> +		asm volatile("rdmsr\n"
>> +			     : "=a" (low), "=d" (high)
>> +			     : "c" (MSR_AMD64_SEV));
>> +		sev_status_val = (high << 32) | low;
>> +		sev_status_checked = true;
>> +	}
>> +
>> +	return sev_status_val & MSR_AMD64_SEV_SNP_ENABLED ? true : false;
> 	return msr_sev_status & MSR_AMD64_SEV_SNP_ENABLED;
>
> is enough.

Noted.


>> +}
>> +
>>  static bool early_setup_sev_es(void)
>>  {
>>  	if (!sev_es_negotiate_protocol())
>>  		sev_es_terminate(0, GHCB_SEV_ES_REASON_PROTOCOL_UNSUPPORTED);
>>  
>> +	/* If SEV-SNP is enabled then check if the hypervisor supports the SEV-SNP features. */
> 80 cols like the rest of this file pls.

Noted.


>
>> +	if (sev_snp_enabled() && !sev_snp_check_hypervisor_features())
>> +		sev_es_terminate(0, GHCB_SEV_ES_REASON_SNP_UNSUPPORTED);
>> +
>>  	if (set_page_decrypted((unsigned long)&boot_ghcb_page))
>>  		return false;
> Thx.
>
diff mbox series

Patch

diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index 6d9055427f37..7badbeb6cb95 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -25,6 +25,8 @@ 
 
 struct ghcb boot_ghcb_page __aligned(PAGE_SIZE);
 struct ghcb *boot_ghcb;
+static u64 sev_status_val;
+static bool sev_status_checked;
 
 /*
  * Copy a version of this function here - insn-eval.c can't be used in
@@ -119,11 +121,30 @@  static enum es_result vc_read_mem(struct es_em_ctxt *ctxt,
 /* Include code for early handlers */
 #include "../../kernel/sev-shared.c"
 
+static inline bool sev_snp_enabled(void)
+{
+	unsigned long low, high;
+
+	if (!sev_status_checked) {
+		asm volatile("rdmsr\n"
+			     : "=a" (low), "=d" (high)
+			     : "c" (MSR_AMD64_SEV));
+		sev_status_val = (high << 32) | low;
+		sev_status_checked = true;
+	}
+
+	return sev_status_val & MSR_AMD64_SEV_SNP_ENABLED ? true : false;
+}
+
 static bool early_setup_sev_es(void)
 {
 	if (!sev_es_negotiate_protocol())
 		sev_es_terminate(0, GHCB_SEV_ES_REASON_PROTOCOL_UNSUPPORTED);
 
+	/* If SEV-SNP is enabled then check if the hypervisor supports the SEV-SNP features. */
+	if (sev_snp_enabled() && !sev_snp_check_hypervisor_features())
+		sev_es_terminate(0, GHCB_SEV_ES_REASON_SNP_UNSUPPORTED);
+
 	if (set_page_decrypted((unsigned long)&boot_ghcb_page))
 		return false;
 
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 3f9b06a04395..085d3d724bc8 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -32,7 +32,18 @@  static bool __init sev_es_check_cpu_features(void)
 	return true;
 }
 
-+static void __noreturn sev_es_terminate(unsigned int set, unsigned int reason)
+static bool __init sev_snp_check_hypervisor_features(void)
+{
+	if (ghcb_version < 2)
+		return false;
+
+	if (!(hv_features & GHCB_HV_FEATURES_SNP))
+		return false;
+
+	return true;
+}
+
+static void __noreturn sev_es_terminate(unsigned int set, unsigned int reason)
 {
 	u64 val = GHCB_MSR_TERM_REQ;
 
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 97be0fe666ab..8c8c939a1754 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -609,6 +609,10 @@  static bool __init sev_es_setup_ghcb(void)
 	if (!sev_es_negotiate_protocol())
 		return false;
 
+	/* If SNP is active, make sure that hypervisor supports the feature. */
+	if (sev_snp_active() && !sev_snp_check_hypervisor_features())
+		sev_es_terminate(0, GHCB_SEV_ES_REASON_SNP_UNSUPPORTED);
+
 	/*
 	 * Clear the boot_ghcb. The first exception comes in before the bss
 	 * section is cleared.