diff mbox series

[Part1,v5,07/38] x86/sev: Add support for hypervisor feature VMGEXIT

Message ID 20210820151933.22401-8-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 Aug. 20, 2021, 3:19 p.m. UTC
Version 2 of GHCB specification introduced advertisement of a features
that are supported by the hypervisor. Add support to query the HV
features on boot.

Version 2 of GHCB specification adds several new NAEs, most of them are
optional except the hypervisor feature. Now that hypervisor feature NAE
is implemented, so bump the GHCB maximum support protocol version.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 arch/x86/include/asm/mem_encrypt.h |  2 ++
 arch/x86/include/asm/sev-common.h  |  3 +++
 arch/x86/include/asm/sev.h         |  2 +-
 arch/x86/include/uapi/asm/svm.h    |  2 ++
 arch/x86/kernel/sev-shared.c       | 23 +++++++++++++++++++++++
 5 files changed, 31 insertions(+), 1 deletion(-)

Comments

Borislav Petkov Aug. 23, 2021, 9:47 a.m. UTC | #1
On Fri, Aug 20, 2021 at 10:19:02AM -0500, Brijesh Singh wrote:
> Version 2 of GHCB specification introduced advertisement of a features
> that are supported by the hypervisor. Add support to query the HV
> features on boot.
> 
> Version 2 of GHCB specification adds several new NAEs, most of them are
> optional except the hypervisor feature. Now that hypervisor feature NAE
> is implemented, so bump the GHCB maximum support protocol version.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  arch/x86/include/asm/mem_encrypt.h |  2 ++
>  arch/x86/include/asm/sev-common.h  |  3 +++
>  arch/x86/include/asm/sev.h         |  2 +-
>  arch/x86/include/uapi/asm/svm.h    |  2 ++
>  arch/x86/kernel/sev-shared.c       | 23 +++++++++++++++++++++++
>  5 files changed, 31 insertions(+), 1 deletion(-)

I think you can simplify more.

The HV features are read twice - once in the decompressor stub and again
in kernel proper - but I guess that's not such a big deal.

Also, sev_hv_features can be static.

Diff ontop:

---
diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index fb857f2e72cb..df14291d65de 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -26,7 +26,6 @@ enum sev_feature_type {
 
 extern u64 sme_me_mask;
 extern u64 sev_status;
-extern u64 sev_hv_features;
 
 void sme_encrypt_execute(unsigned long encrypted_kernel_vaddr,
 			 unsigned long decrypted_kernel_vaddr,
@@ -67,7 +66,6 @@ bool sev_feature_enabled(unsigned int feature_type);
 #else	/* !CONFIG_AMD_MEM_ENCRYPT */
 
 #define sme_me_mask	0ULL
-#define sev_hv_features	0ULL
 
 static inline void __init sme_early_encrypt(resource_size_t paddr,
 					    unsigned long size) { }
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 8bd67087d79e..d657c2c5a1ee 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -24,7 +24,7 @@
 static u16 __ro_after_init ghcb_version;
 
 /* Bitmap of SEV features supported by the hypervisor */
-u64 __ro_after_init sev_hv_features = 0;
+static u64 __ro_after_init sev_hv_features;
 
 static bool __init sev_es_check_cpu_features(void)
 {
@@ -51,10 +51,18 @@ static void __noreturn sev_es_terminate(unsigned int set, unsigned int reason)
 		asm volatile("hlt\n" : : : "memory");
 }
 
+/*
+ * The hypervisor features are available from GHCB version 2 onward.
+ */
 static bool get_hv_features(void)
 {
 	u64 val;
 
+	sev_hv_features = 0;
+
+	if (ghcb_version < 2)
+		return false;
+
 	sev_es_wr_ghcb_msr(GHCB_MSR_HV_FT_REQ);
 	VMGEXIT();
 
@@ -85,8 +93,7 @@ static bool sev_es_negotiate_protocol(void)
 
 	ghcb_version = min_t(size_t, GHCB_MSR_PROTO_MAX(val), GHCB_PROTOCOL_MAX);
 
-	/* The hypervisor features are available from version 2 onward. */
-	if (ghcb_version >= 2 && !get_hv_features())
+	if (!get_hv_features())
 		return false;
 
 	return true;
Brijesh Singh Aug. 23, 2021, 6:25 p.m. UTC | #2
On 8/23/21 4:47 AM, Borislav Petkov wrote:
> On Fri, Aug 20, 2021 at 10:19:02AM -0500, Brijesh Singh wrote:
>> Version 2 of GHCB specification introduced advertisement of a features
>> that are supported by the hypervisor. Add support to query the HV
>> features on boot.
>>
>> Version 2 of GHCB specification adds several new NAEs, most of them are
>> optional except the hypervisor feature. Now that hypervisor feature NAE
>> is implemented, so bump the GHCB maximum support protocol version.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   arch/x86/include/asm/mem_encrypt.h |  2 ++
>>   arch/x86/include/asm/sev-common.h  |  3 +++
>>   arch/x86/include/asm/sev.h         |  2 +-
>>   arch/x86/include/uapi/asm/svm.h    |  2 ++
>>   arch/x86/kernel/sev-shared.c       | 23 +++++++++++++++++++++++
>>   5 files changed, 31 insertions(+), 1 deletion(-)
> 
> I think you can simplify more.
> 
> The HV features are read twice - once in the decompressor stub and again
> in kernel proper - but I guess that's not such a big deal.
> 
> Also, sev_hv_features can be static.
> 
> Diff ontop:
> 

The sev_hv_features is also referred during the AP creation. By caching 
the value in sev-shared.c and exporting it to others, we wanted to 
minimize VMGEXITs during the AP creation.

If we go with your patch below, then we will need to cache the 
sev_hv_features in sev.c, so that it can be later used by the AP 
creation code (see patch#22).

thanks
Brijesh Singh Aug. 23, 2021, 6:34 p.m. UTC | #3
On 8/23/21 1:25 PM, Brijesh Singh wrote:
> 
> 
> On 8/23/21 4:47 AM, Borislav Petkov wrote:
>> On Fri, Aug 20, 2021 at 10:19:02AM -0500, Brijesh Singh wrote:
>>> Version 2 of GHCB specification introduced advertisement of a features
>>> that are supported by the hypervisor. Add support to query the HV
>>> features on boot.
>>>
>>> Version 2 of GHCB specification adds several new NAEs, most of them are
>>> optional except the hypervisor feature. Now that hypervisor feature NAE
>>> is implemented, so bump the GHCB maximum support protocol version.
>>>
>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>> ---
>>>   arch/x86/include/asm/mem_encrypt.h |  2 ++
>>>   arch/x86/include/asm/sev-common.h  |  3 +++
>>>   arch/x86/include/asm/sev.h         |  2 +-
>>>   arch/x86/include/uapi/asm/svm.h    |  2 ++
>>>   arch/x86/kernel/sev-shared.c       | 23 +++++++++++++++++++++++
>>>   5 files changed, 31 insertions(+), 1 deletion(-)
>>
>> I think you can simplify more.
>>
>> The HV features are read twice - once in the decompressor stub and again
>> in kernel proper - but I guess that's not such a big deal.
>>
>> Also, sev_hv_features can be static.
>>
>> Diff ontop:
>>
> 
> The sev_hv_features is also referred during the AP creation. By caching 
> the value in sev-shared.c and exporting it to others, we wanted to 
> minimize VMGEXITs during the AP creation.
> 
> If we go with your patch below, then we will need to cache the 
> sev_hv_features in sev.c, so that it can be later used by the AP 
> creation code (see patch#22).
> 

Let me take it back, I didn't realize that sev.c includes the 
sev-shared.c. So your patch will work fine. sorry about the noise.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index df14291d65de..fb857f2e72cb 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -26,6 +26,7 @@  enum sev_feature_type {
 
 extern u64 sme_me_mask;
 extern u64 sev_status;
+extern u64 sev_hv_features;
 
 void sme_encrypt_execute(unsigned long encrypted_kernel_vaddr,
 			 unsigned long decrypted_kernel_vaddr,
@@ -66,6 +67,7 @@  bool sev_feature_enabled(unsigned int feature_type);
 #else	/* !CONFIG_AMD_MEM_ENCRYPT */
 
 #define sme_me_mask	0ULL
+#define sev_hv_features	0ULL
 
 static inline void __init sme_early_encrypt(resource_size_t paddr,
 					    unsigned long size) { }
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index 3278ee578937..891569c07ed7 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -60,6 +60,9 @@ 
 /* GHCB Hypervisor Feature Request/Response */
 #define GHCB_MSR_HV_FT_REQ		0x080
 #define GHCB_MSR_HV_FT_RESP		0x081
+#define GHCB_MSR_HV_FT_RESP_VAL(v)			\
+	/* GHCBData[63:12] */				\
+	(((u64)(v) & GENMASK_ULL(63, 12)) >> 12)
 
 #define GHCB_MSR_TERM_REQ		0x100
 #define GHCB_MSR_TERM_REASON_SET_POS	12
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 7ec91b1359df..134a7c9d91b6 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -13,7 +13,7 @@ 
 #include <asm/sev-common.h>
 
 #define GHCB_PROTOCOL_MIN	1ULL
-#define GHCB_PROTOCOL_MAX	1ULL
+#define GHCB_PROTOCOL_MAX	2ULL
 #define GHCB_DEFAULT_USAGE	0ULL
 
 #define	VMGEXIT()			{ asm volatile("rep; vmmcall\n\r"); }
diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h
index efa969325ede..b0ad00f4c1e1 100644
--- a/arch/x86/include/uapi/asm/svm.h
+++ b/arch/x86/include/uapi/asm/svm.h
@@ -108,6 +108,7 @@ 
 #define SVM_VMGEXIT_AP_JUMP_TABLE		0x80000005
 #define SVM_VMGEXIT_SET_AP_JUMP_TABLE		0
 #define SVM_VMGEXIT_GET_AP_JUMP_TABLE		1
+#define SVM_VMGEXIT_HV_FEATURES			0x8000fffd
 #define SVM_VMGEXIT_UNSUPPORTED_EVENT		0x8000ffff
 
 /* Exit code reserved for hypervisor/software use */
@@ -218,6 +219,7 @@ 
 	{ SVM_VMGEXIT_NMI_COMPLETE,	"vmgexit_nmi_complete" }, \
 	{ SVM_VMGEXIT_AP_HLT_LOOP,	"vmgexit_ap_hlt_loop" }, \
 	{ SVM_VMGEXIT_AP_JUMP_TABLE,	"vmgexit_ap_jump_table" }, \
+	{ SVM_VMGEXIT_HV_FEATURES,	"vmgexit_hypervisor_feature" }, \
 	{ SVM_EXIT_ERR,         "invalid_guest_state" }
 
 
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 58a6efb1f327..8bd67087d79e 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -23,6 +23,9 @@ 
  */
 static u16 __ro_after_init ghcb_version;
 
+/* Bitmap of SEV features supported by the hypervisor */
+u64 __ro_after_init sev_hv_features = 0;
+
 static bool __init sev_es_check_cpu_features(void)
 {
 	if (!has_cpuflag(X86_FEATURE_RDRAND)) {
@@ -48,6 +51,22 @@  static void __noreturn sev_es_terminate(unsigned int set, unsigned int reason)
 		asm volatile("hlt\n" : : : "memory");
 }
 
+static bool get_hv_features(void)
+{
+	u64 val;
+
+	sev_es_wr_ghcb_msr(GHCB_MSR_HV_FT_REQ);
+	VMGEXIT();
+
+	val = sev_es_rd_ghcb_msr();
+	if (GHCB_RESP_CODE(val) != GHCB_MSR_HV_FT_RESP)
+		return false;
+
+	sev_hv_features = GHCB_MSR_HV_FT_RESP_VAL(val);
+
+	return true;
+}
+
 static bool sev_es_negotiate_protocol(void)
 {
 	u64 val;
@@ -66,6 +85,10 @@  static bool sev_es_negotiate_protocol(void)
 
 	ghcb_version = min_t(size_t, GHCB_MSR_PROTO_MAX(val), GHCB_PROTOCOL_MAX);
 
+	/* The hypervisor features are available from version 2 onward. */
+	if (ghcb_version >= 2 && !get_hv_features())
+		return false;
+
 	return true;
 }