diff mbox series

[Part1,RFC,v4,02/36] x86/sev: Save the negotiated GHCB version

Message ID 20210707181506.30489-3-brijesh.singh@amd.com
State Superseded
Headers show
Series Add AMD Secure Nested Paging (SEV-SNP) Guest Support | expand

Commit Message

Brijesh Singh July 7, 2021, 6:14 p.m. UTC
The SEV-ES guest calls the sev_es_negotiate_protocol() to negotiate the
GHCB protocol version before establishing the GHCB. Cache the negotiated
GHCB version so that it can be used later.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 arch/x86/include/asm/sev.h   |  2 +-
 arch/x86/kernel/sev-shared.c | 17 ++++++++++++++---
 2 files changed, 15 insertions(+), 4 deletions(-)

Comments

Borislav Petkov Aug. 10, 2021, 9:17 a.m. UTC | #1
On Wed, Jul 07, 2021 at 01:14:32PM -0500, Brijesh Singh wrote:
> diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
> index 114f62fe2529..19c2306ac02d 100644
> --- a/arch/x86/kernel/sev-shared.c
> +++ b/arch/x86/kernel/sev-shared.c
> @@ -14,6 +14,15 @@
>  #define has_cpuflag(f)	boot_cpu_has(f)
>  #endif
>  
> +/*
> + * Since feature negotiation related variables are set early in the boot
> + * process they must reside in the .data section so as not to be zeroed
> + * out when the .bss section is later cleared.
> + *
> + * GHCB protocol version negotiated with the hypervisor.
> + */
> +static u16 ghcb_version __section(".data..ro_after_init");

There's a define for that section specifier: __ro_after_init
Brijesh Singh Aug. 10, 2021, 1:17 p.m. UTC | #2
On 8/10/21 5:01 AM, Borislav Petkov wrote:
> On Wed, Jul 07, 2021 at 01:14:32PM -0500, Brijesh Singh wrote:

>> The SEV-ES guest calls the sev_es_negotiate_protocol() to negotiate the

>> GHCB protocol version before establishing the GHCB. Cache the negotiated

>> GHCB version so that it can be used later.

>>

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

>> ---

>>   arch/x86/include/asm/sev.h   |  2 +-

>>   arch/x86/kernel/sev-shared.c | 17 ++++++++++++++---

>>   2 files changed, 15 insertions(+), 4 deletions(-)

> 

> Also, while looking at this, all those defines in sev-common.h were

> bothering me for a while now because they're an unreadable mess because

> I have to go look at the GHCB spec, decode the corresponding MSR

> protocol request and then go and piece together each request value by

> verifying the masks...

> 

> So I did the below which is, IMO, a lot more readable as you can follow

> it directly with the spec opened in parallel.

> 

> Thus, if you don't have a better idea, I'd ask you to please add this to

> your set and continue defining the new MSR protocol requests this way so

> that it can be readable.

> 


thanks Boris, I will include this in my series and rework the remaining 
patches to align with it.

-brijesh
diff mbox series

Patch

diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index fa5cd05d3b5b..7ec91b1359df 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -12,7 +12,7 @@ 
 #include <asm/insn.h>
 #include <asm/sev-common.h>
 
-#define GHCB_PROTO_OUR		0x0001UL
+#define GHCB_PROTOCOL_MIN	1ULL
 #define GHCB_PROTOCOL_MAX	1ULL
 #define GHCB_DEFAULT_USAGE	0ULL
 
diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 114f62fe2529..19c2306ac02d 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -14,6 +14,15 @@ 
 #define has_cpuflag(f)	boot_cpu_has(f)
 #endif
 
+/*
+ * Since feature negotiation related variables are set early in the boot
+ * process they must reside in the .data section so as not to be zeroed
+ * out when the .bss section is later cleared.
+ *
+ * GHCB protocol version negotiated with the hypervisor.
+ */
+static u16 ghcb_version __section(".data..ro_after_init");
+
 static bool __init sev_es_check_cpu_features(void)
 {
 	if (!has_cpuflag(X86_FEATURE_RDRAND)) {
@@ -54,10 +63,12 @@  static bool sev_es_negotiate_protocol(void)
 	if (GHCB_MSR_INFO(val) != GHCB_MSR_SEV_INFO_RESP)
 		return false;
 
-	if (GHCB_MSR_PROTO_MAX(val) < GHCB_PROTO_OUR ||
-	    GHCB_MSR_PROTO_MIN(val) > GHCB_PROTO_OUR)
+	if (GHCB_MSR_PROTO_MAX(val) < GHCB_PROTOCOL_MIN ||
+	    GHCB_MSR_PROTO_MIN(val) > GHCB_PROTOCOL_MAX)
 		return false;
 
+	ghcb_version = min_t(size_t, GHCB_MSR_PROTO_MAX(val), GHCB_PROTOCOL_MAX);
+
 	return true;
 }
 
@@ -102,7 +113,7 @@  static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
 	enum es_result ret;
 
 	/* Fill in protocol and format specifiers */
-	ghcb->protocol_version = GHCB_PROTOCOL_MAX;
+	ghcb->protocol_version = ghcb_version;
 	ghcb->ghcb_usage       = GHCB_DEFAULT_USAGE;
 
 	ghcb_set_sw_exit_code(ghcb, exit_code);