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;--
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); }
/*