All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/sev: Return an error on a returned non-zero SW_EXITINFO1[31:0]
@ 2021-10-01  4:42 Tom Lendacky
  2021-10-01  9:43 ` Borislav Petkov
  0 siblings, 1 reply; 5+ messages in thread
From: Tom Lendacky @ 2021-10-01  4:42 UTC (permalink / raw)
  To: linux-kernel, x86
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Joerg Roedel, Brijesh Singh, stable

After returning from a VMGEXIT NAE event, SW_EXITINFO1[31:0] is checked
for a value of 1, which indicates an error and that SW_EXITINFO2 contains
exception information. However, future versions of the GHCB specification
may define new values for SW_EXITINFO1[31:0], so really any non-zero value
should be treated as an error.

Fixes: 597cfe48212a ("x86/boot/compressed/64: Setup a GHCB-based VC Exception handler")
Cc: <stable@vger.kernel.org> # 5.10+
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/kernel/sev-shared.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 34f20e08dc46..ff1e82ff52d9 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -130,6 +130,8 @@ static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
 		} else {
 			ret = ES_VMM_ERROR;
 		}
+	} else if (ghcb->save.sw_exit_info_1 & 0xffffffff) {
+		ret = ES_VMM_ERROR;
 	} else {
 		ret = ES_OK;
 	}
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/sev: Return an error on a returned non-zero SW_EXITINFO1[31:0]
  2021-10-01  4:42 [PATCH] x86/sev: Return an error on a returned non-zero SW_EXITINFO1[31:0] Tom Lendacky
@ 2021-10-01  9:43 ` Borislav Petkov
  2021-10-01 13:45   ` Tom Lendacky
  0 siblings, 1 reply; 5+ messages in thread
From: Borislav Petkov @ 2021-10-01  9:43 UTC (permalink / raw)
  To: Tom Lendacky
  Cc: linux-kernel, x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Joerg Roedel, Brijesh Singh, stable

On Thu, Sep 30, 2021 at 11:42:01PM -0500, Tom Lendacky wrote:
> After returning from a VMGEXIT NAE event, SW_EXITINFO1[31:0] is checked
> for a value of 1, which indicates an error and that SW_EXITINFO2 contains
> exception information. However, future versions of the GHCB specification
> may define new values for SW_EXITINFO1[31:0], so really any non-zero value
> should be treated as an error.
> 
> Fixes: 597cfe48212a ("x86/boot/compressed/64: Setup a GHCB-based VC Exception handler")
> Cc: <stable@vger.kernel.org> # 5.10+
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  arch/x86/kernel/sev-shared.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
> index 34f20e08dc46..ff1e82ff52d9 100644
> --- a/arch/x86/kernel/sev-shared.c
> +++ b/arch/x86/kernel/sev-shared.c
> @@ -130,6 +130,8 @@ static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
>  		} else {
>  			ret = ES_VMM_ERROR;
>  		}
> +	} else if (ghcb->save.sw_exit_info_1 & 0xffffffff) {
> +		ret = ES_VMM_ERROR;
>  	} else {
>  		ret = ES_OK;
>  	}
> -- 

So I wanna do this ontop. Might wanna apply it and look at the result -
it shows better what the changes are.

---
From: Borislav Petkov <bp@suse.de>
Date: Fri, 1 Oct 2021 11:41:05 +0200
Subject: [PATCH] x86/sev: Carve out HV call return value verification

Carve out the verification of the HV call return value into a separate
helper and make it more readable.

No it more readable.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/sev-shared.c | 53 ++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index bf1033a62e48..f2933f740fa7 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -94,25 +94,15 @@ static void vc_finish_insn(struct es_em_ctxt *ctxt)
 	ctxt->regs->ip += ctxt->insn.length;
 }
 
-static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
-					  struct es_em_ctxt *ctxt,
-					  u64 exit_code, u64 exit_info_1,
-					  u64 exit_info_2)
+static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
 {
-	enum es_result ret;
+	int ret;
 
-	/* Fill in protocol and format specifiers */
-	ghcb->protocol_version = GHCB_PROTOCOL_MAX;
-	ghcb->ghcb_usage       = GHCB_DEFAULT_USAGE;
+	ret = ghcb->save.sw_exit_info_1 & 0xffffffff;
+	if (!ret)
+		return ES_OK;
 
-	ghcb_set_sw_exit_code(ghcb, exit_code);
-	ghcb_set_sw_exit_info_1(ghcb, exit_info_1);
-	ghcb_set_sw_exit_info_2(ghcb, exit_info_2);
-
-	sev_es_wr_ghcb_msr(__pa(ghcb));
-	VMGEXIT();
-
-	if ((ghcb->save.sw_exit_info_1 & 0xffffffff) == 1) {
+	if (ret == 1) {
 		u64 info = ghcb->save.sw_exit_info_2;
 		unsigned long v;
 
@@ -124,19 +114,34 @@ static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
 		    ((v == X86_TRAP_GP) || (v == X86_TRAP_UD)) &&
 		    ((info & SVM_EVTINJ_TYPE_MASK) == SVM_EVTINJ_TYPE_EXEPT)) {
 			ctxt->fi.vector = v;
+
 			if (info & SVM_EVTINJ_VALID_ERR)
 				ctxt->fi.error_code = info >> 32;
-			ret = ES_EXCEPTION;
-		} else {
-			ret = ES_VMM_ERROR;
+
+			return ES_EXCEPTION;
 		}
-	} else if (ghcb->save.sw_exit_info_1 & 0xffffffff) {
-		ret = ES_VMM_ERROR;
-	} else {
-		ret = ES_OK;
 	}
 
-	return ret;
+	return ES_VMM_ERROR;
+}
+
+static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
+					  struct es_em_ctxt *ctxt,
+					  u64 exit_code, u64 exit_info_1,
+					  u64 exit_info_2)
+{
+	/* Fill in protocol and format specifiers */
+	ghcb->protocol_version = GHCB_PROTOCOL_MAX;
+	ghcb->ghcb_usage       = GHCB_DEFAULT_USAGE;
+
+	ghcb_set_sw_exit_code(ghcb, exit_code);
+	ghcb_set_sw_exit_info_1(ghcb, exit_info_1);
+	ghcb_set_sw_exit_info_2(ghcb, exit_info_2);
+
+	sev_es_wr_ghcb_msr(__pa(ghcb));
+	VMGEXIT();
+
+	return verify_exception_info(ghcb, ctxt);
 }
 
 /*
-- 
2.29.2


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/sev: Return an error on a returned non-zero SW_EXITINFO1[31:0]
  2021-10-01  9:43 ` Borislav Petkov
@ 2021-10-01 13:45   ` Tom Lendacky
  2021-10-01 13:54     ` Borislav Petkov
  0 siblings, 1 reply; 5+ messages in thread
From: Tom Lendacky @ 2021-10-01 13:45 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-kernel, x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Joerg Roedel, Brijesh Singh, stable

On 10/1/21 4:43 AM, Borislav Petkov wrote:
> On Thu, Sep 30, 2021 at 11:42:01PM -0500, Tom Lendacky wrote:
>> After returning from a VMGEXIT NAE event, SW_EXITINFO1[31:0] is checked
>> for a value of 1, which indicates an error and that SW_EXITINFO2 contains
>> exception information. However, future versions of the GHCB specification
>> may define new values for SW_EXITINFO1[31:0], so really any non-zero value
>> should be treated as an error.
>>
> 
> So I wanna do this ontop. Might wanna apply it and look at the result -
> it shows better what the changes are.

Yup, looks good to me.

> 
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Fri, 1 Oct 2021 11:41:05 +0200
> Subject: [PATCH] x86/sev: Carve out HV call return value verification
> 
> Carve out the verification of the HV call return value into a separate
> helper and make it more readable.
> 
> No it more readable.

I'm assuming you don't want this last sentence...

Thanks,
Tom

> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/sev: Return an error on a returned non-zero SW_EXITINFO1[31:0]
  2021-10-01 13:45   ` Tom Lendacky
@ 2021-10-01 13:54     ` Borislav Petkov
  2021-10-11 17:13       ` [PATCH] x86/sev: Carve out HV call's return value verification Borislav Petkov
  0 siblings, 1 reply; 5+ messages in thread
From: Borislav Petkov @ 2021-10-01 13:54 UTC (permalink / raw)
  To: Tom Lendacky
  Cc: linux-kernel, x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Joerg Roedel, Brijesh Singh, stable

On Fri, Oct 01, 2021 at 08:45:17AM -0500, Tom Lendacky wrote:
> I'm assuming you don't want this last sentence...

Bah, that's me fat-fingering it. It was supposed to say "No functional
changes."

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] x86/sev: Carve out HV call's return value verification
  2021-10-01 13:54     ` Borislav Petkov
@ 2021-10-11 17:13       ` Borislav Petkov
  0 siblings, 0 replies; 5+ messages in thread
From: Borislav Petkov @ 2021-10-11 17:13 UTC (permalink / raw)
  To: Tom Lendacky
  Cc: linux-kernel, x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Joerg Roedel, Brijesh Singh, stable

Now as a proper patch with typo fixed and tested:

---
From: Borislav Petkov <bp@suse.de>

Carve out the verification of the HV call return value into a separate
helper and make it more readable.

No functional changes.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/sev-shared.c | 53 ++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index bf1033a62e48..4579c38a11c4 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c
@@ -94,25 +94,15 @@ static void vc_finish_insn(struct es_em_ctxt *ctxt)
 	ctxt->regs->ip += ctxt->insn.length;
 }
 
-static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
-					  struct es_em_ctxt *ctxt,
-					  u64 exit_code, u64 exit_info_1,
-					  u64 exit_info_2)
+static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
 {
-	enum es_result ret;
+	u32 ret;
 
-	/* Fill in protocol and format specifiers */
-	ghcb->protocol_version = GHCB_PROTOCOL_MAX;
-	ghcb->ghcb_usage       = GHCB_DEFAULT_USAGE;
+	ret = ghcb->save.sw_exit_info_1 & GENMASK_ULL(31, 0);
+	if (!ret)
+		return ES_OK;
 
-	ghcb_set_sw_exit_code(ghcb, exit_code);
-	ghcb_set_sw_exit_info_1(ghcb, exit_info_1);
-	ghcb_set_sw_exit_info_2(ghcb, exit_info_2);
-
-	sev_es_wr_ghcb_msr(__pa(ghcb));
-	VMGEXIT();
-
-	if ((ghcb->save.sw_exit_info_1 & 0xffffffff) == 1) {
+	if (ret == 1) {
 		u64 info = ghcb->save.sw_exit_info_2;
 		unsigned long v;
 
@@ -124,19 +114,34 @@ static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
 		    ((v == X86_TRAP_GP) || (v == X86_TRAP_UD)) &&
 		    ((info & SVM_EVTINJ_TYPE_MASK) == SVM_EVTINJ_TYPE_EXEPT)) {
 			ctxt->fi.vector = v;
+
 			if (info & SVM_EVTINJ_VALID_ERR)
 				ctxt->fi.error_code = info >> 32;
-			ret = ES_EXCEPTION;
-		} else {
-			ret = ES_VMM_ERROR;
+
+			return ES_EXCEPTION;
 		}
-	} else if (ghcb->save.sw_exit_info_1 & 0xffffffff) {
-		ret = ES_VMM_ERROR;
-	} else {
-		ret = ES_OK;
 	}
 
-	return ret;
+	return ES_VMM_ERROR;
+}
+
+static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
+					  struct es_em_ctxt *ctxt,
+					  u64 exit_code, u64 exit_info_1,
+					  u64 exit_info_2)
+{
+	/* Fill in protocol and format specifiers */
+	ghcb->protocol_version = GHCB_PROTOCOL_MAX;
+	ghcb->ghcb_usage       = GHCB_DEFAULT_USAGE;
+
+	ghcb_set_sw_exit_code(ghcb, exit_code);
+	ghcb_set_sw_exit_info_1(ghcb, exit_info_1);
+	ghcb_set_sw_exit_info_2(ghcb, exit_info_2);
+
+	sev_es_wr_ghcb_msr(__pa(ghcb));
+	VMGEXIT();
+
+	return verify_exception_info(ghcb, ctxt);
 }
 
 /*
-- 
2.29.2


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-10-11 17:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-01  4:42 [PATCH] x86/sev: Return an error on a returned non-zero SW_EXITINFO1[31:0] Tom Lendacky
2021-10-01  9:43 ` Borislav Petkov
2021-10-01 13:45   ` Tom Lendacky
2021-10-01 13:54     ` Borislav Petkov
2021-10-11 17:13       ` [PATCH] x86/sev: Carve out HV call's return value verification Borislav Petkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.