linux-kernel.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).