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: <[email protected]> # 5.10+
Signed-off-by: Tom Lendacky <[email protected]>
---
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
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: <[email protected]> # 5.10+
> Signed-off-by: Tom Lendacky <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
---
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
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 <[email protected]>
> 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
>
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
Now as a proper patch with typo fixed and tested:
---
From: Borislav Petkov <[email protected]>
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 <[email protected]>
---
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