| Message ID | 20220209164420.8894-6-varad.gautam@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add #VC exception handling for AMD SEV-ES | expand |
On Wed, Feb 9, 2022 at 8:44 AM Varad Gautam <varad.gautam@suse.com> wrote: > > Lay the groundwork for processing #VC exceptions in the handler. > This includes clearing the GHCB, decoding the insn that triggered > this #VC, and continuing execution after the exception has been > processed. This description does not mention that this code is copied from Linux. Should we have a comment in this patch description, similar to the other patches? Also, in general, I wonder if we need to mention where this code came from in a comment header at the top of the file. > > Signed-off-by: Varad Gautam <varad.gautam@suse.com> > --- > lib/x86/amd_sev_vc.c | 78 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 78 insertions(+) > > diff --git a/lib/x86/amd_sev_vc.c b/lib/x86/amd_sev_vc.c > index 8226121..142f2cd 100644 > --- a/lib/x86/amd_sev_vc.c > +++ b/lib/x86/amd_sev_vc.c > @@ -1,14 +1,92 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > > #include "amd_sev.h" > +#include "svm.h" > > extern phys_addr_t ghcb_addr; > > +static void vc_ghcb_invalidate(struct ghcb *ghcb) > +{ > + ghcb->save.sw_exit_code = 0; > + memset(ghcb->save.valid_bitmap, 0, sizeof(ghcb->save.valid_bitmap)); > +} > + > +static bool vc_decoding_needed(unsigned long exit_code) > +{ > + /* Exceptions don't require to decode the instruction */ > + return !(exit_code >= SVM_EXIT_EXCP_BASE && > + exit_code <= SVM_EXIT_LAST_EXCP); > +} > + > +static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt) > +{ > + unsigned char buffer[MAX_INSN_SIZE]; > + int ret; > + > + memcpy(buffer, (unsigned char *)ctxt->regs->rip, MAX_INSN_SIZE); > + > + ret = insn_decode(&ctxt->insn, buffer, MAX_INSN_SIZE, INSN_MODE_64); > + if (ret < 0) > + return ES_DECODE_FAILED; > + else > + return ES_OK; > +} > + > +static enum es_result vc_init_em_ctxt(struct es_em_ctxt *ctxt, > + struct ex_regs *regs, > + unsigned long exit_code) > +{ > + enum es_result ret = ES_OK; > + > + memset(ctxt, 0, sizeof(*ctxt)); > + ctxt->regs = regs; > + > + if (vc_decoding_needed(exit_code)) > + ret = vc_decode_insn(ctxt); > + > + return ret; > +} > + > +static void vc_finish_insn(struct es_em_ctxt *ctxt) > +{ > + ctxt->regs->rip += ctxt->insn.length; > +} > + > +static enum es_result vc_handle_exitcode(struct es_em_ctxt *ctxt, > + struct ghcb *ghcb, > + unsigned long exit_code) > +{ > + enum es_result result; > + > + switch (exit_code) { > + default: > + /* > + * Unexpected #VC exception > + */ > + result = ES_UNSUPPORTED; > + } > + > + return result; > +} > + > void handle_sev_es_vc(struct ex_regs *regs) > { > struct ghcb *ghcb = (struct ghcb *) ghcb_addr; > + unsigned long exit_code = regs->error_code; > + struct es_em_ctxt ctxt; > + enum es_result result; > + > if (!ghcb) { > /* TODO: kill guest */ > return; > } > + > + vc_ghcb_invalidate(ghcb); > + result = vc_init_em_ctxt(&ctxt, regs, exit_code); > + if (result == ES_OK) > + result = vc_handle_exitcode(&ctxt, ghcb, exit_code); > + if (result == ES_OK) > + vc_finish_insn(&ctxt); Should we print an error if the result is not `ES_OK`, like the function `vc_raw_handle_exception()` does in Linux? Otherwise, this silent failure is going to be very confusing to whoever runs into it. > + > + return; > } > -- > 2.32.0 >
On 2/12/22 9:54 PM, Marc Orr wrote: > On Wed, Feb 9, 2022 at 8:44 AM Varad Gautam <varad.gautam@suse.com> wrote: >> >> Lay the groundwork for processing #VC exceptions in the handler. >> This includes clearing the GHCB, decoding the insn that triggered >> this #VC, and continuing execution after the exception has been >> processed. > > This description does not mention that this code is copied from Linux. > Should we have a comment in this patch description, similar to the > other patches? > > Also, in general, I wonder if we need to mention where this code came > from in a comment header at the top of the file. > >> >> Signed-off-by: Varad Gautam <varad.gautam@suse.com> >> --- >> lib/x86/amd_sev_vc.c | 78 ++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 78 insertions(+) >> >> diff --git a/lib/x86/amd_sev_vc.c b/lib/x86/amd_sev_vc.c >> index 8226121..142f2cd 100644 >> --- a/lib/x86/amd_sev_vc.c >> +++ b/lib/x86/amd_sev_vc.c >> @@ -1,14 +1,92 @@ >> /* SPDX-License-Identifier: GPL-2.0 */ >> >> #include "amd_sev.h" >> +#include "svm.h" >> >> extern phys_addr_t ghcb_addr; >> >> +static void vc_ghcb_invalidate(struct ghcb *ghcb) >> +{ >> + ghcb->save.sw_exit_code = 0; >> + memset(ghcb->save.valid_bitmap, 0, sizeof(ghcb->save.valid_bitmap)); >> +} >> + >> +static bool vc_decoding_needed(unsigned long exit_code) >> +{ >> + /* Exceptions don't require to decode the instruction */ >> + return !(exit_code >= SVM_EXIT_EXCP_BASE && >> + exit_code <= SVM_EXIT_LAST_EXCP); >> +} >> + >> +static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt) >> +{ >> + unsigned char buffer[MAX_INSN_SIZE]; >> + int ret; >> + >> + memcpy(buffer, (unsigned char *)ctxt->regs->rip, MAX_INSN_SIZE); >> + >> + ret = insn_decode(&ctxt->insn, buffer, MAX_INSN_SIZE, INSN_MODE_64); >> + if (ret < 0) >> + return ES_DECODE_FAILED; >> + else >> + return ES_OK; >> +} >> + >> +static enum es_result vc_init_em_ctxt(struct es_em_ctxt *ctxt, >> + struct ex_regs *regs, >> + unsigned long exit_code) >> +{ >> + enum es_result ret = ES_OK; >> + >> + memset(ctxt, 0, sizeof(*ctxt)); >> + ctxt->regs = regs; >> + >> + if (vc_decoding_needed(exit_code)) >> + ret = vc_decode_insn(ctxt); >> + >> + return ret; >> +} >> + >> +static void vc_finish_insn(struct es_em_ctxt *ctxt) >> +{ >> + ctxt->regs->rip += ctxt->insn.length; >> +} >> + >> +static enum es_result vc_handle_exitcode(struct es_em_ctxt *ctxt, >> + struct ghcb *ghcb, >> + unsigned long exit_code) >> +{ >> + enum es_result result; >> + >> + switch (exit_code) { >> + default: >> + /* >> + * Unexpected #VC exception >> + */ >> + result = ES_UNSUPPORTED; >> + } >> + >> + return result; >> +} >> + >> void handle_sev_es_vc(struct ex_regs *regs) >> { >> struct ghcb *ghcb = (struct ghcb *) ghcb_addr; >> + unsigned long exit_code = regs->error_code; >> + struct es_em_ctxt ctxt; >> + enum es_result result; >> + >> if (!ghcb) { >> /* TODO: kill guest */ >> return; >> } >> + >> + vc_ghcb_invalidate(ghcb); >> + result = vc_init_em_ctxt(&ctxt, regs, exit_code); >> + if (result == ES_OK) >> + result = vc_handle_exitcode(&ctxt, ghcb, exit_code); >> + if (result == ES_OK) >> + vc_finish_insn(&ctxt); > > Should we print an error if the result is not `ES_OK`, like the > function `vc_raw_handle_exception()` does in Linux? Otherwise, this > silent failure is going to be very confusing to whoever runs into it. > Changed in v3. >> + >> + return; >> } >> -- >> 2.32.0 >> >
diff --git a/lib/x86/amd_sev_vc.c b/lib/x86/amd_sev_vc.c index 8226121..142f2cd 100644 --- a/lib/x86/amd_sev_vc.c +++ b/lib/x86/amd_sev_vc.c @@ -1,14 +1,92 @@ /* SPDX-License-Identifier: GPL-2.0 */ #include "amd_sev.h" +#include "svm.h" extern phys_addr_t ghcb_addr; +static void vc_ghcb_invalidate(struct ghcb *ghcb) +{ + ghcb->save.sw_exit_code = 0; + memset(ghcb->save.valid_bitmap, 0, sizeof(ghcb->save.valid_bitmap)); +} + +static bool vc_decoding_needed(unsigned long exit_code) +{ + /* Exceptions don't require to decode the instruction */ + return !(exit_code >= SVM_EXIT_EXCP_BASE && + exit_code <= SVM_EXIT_LAST_EXCP); +} + +static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt) +{ + unsigned char buffer[MAX_INSN_SIZE]; + int ret; + + memcpy(buffer, (unsigned char *)ctxt->regs->rip, MAX_INSN_SIZE); + + ret = insn_decode(&ctxt->insn, buffer, MAX_INSN_SIZE, INSN_MODE_64); + if (ret < 0) + return ES_DECODE_FAILED; + else + return ES_OK; +} + +static enum es_result vc_init_em_ctxt(struct es_em_ctxt *ctxt, + struct ex_regs *regs, + unsigned long exit_code) +{ + enum es_result ret = ES_OK; + + memset(ctxt, 0, sizeof(*ctxt)); + ctxt->regs = regs; + + if (vc_decoding_needed(exit_code)) + ret = vc_decode_insn(ctxt); + + return ret; +} + +static void vc_finish_insn(struct es_em_ctxt *ctxt) +{ + ctxt->regs->rip += ctxt->insn.length; +} + +static enum es_result vc_handle_exitcode(struct es_em_ctxt *ctxt, + struct ghcb *ghcb, + unsigned long exit_code) +{ + enum es_result result; + + switch (exit_code) { + default: + /* + * Unexpected #VC exception + */ + result = ES_UNSUPPORTED; + } + + return result; +} + void handle_sev_es_vc(struct ex_regs *regs) { struct ghcb *ghcb = (struct ghcb *) ghcb_addr; + unsigned long exit_code = regs->error_code; + struct es_em_ctxt ctxt; + enum es_result result; + if (!ghcb) { /* TODO: kill guest */ return; } + + vc_ghcb_invalidate(ghcb); + result = vc_init_em_ctxt(&ctxt, regs, exit_code); + if (result == ES_OK) + result = vc_handle_exitcode(&ctxt, ghcb, exit_code); + if (result == ES_OK) + vc_finish_insn(&ctxt); + + return; }
Lay the groundwork for processing #VC exceptions in the handler. This includes clearing the GHCB, decoding the insn that triggered this #VC, and continuing execution after the exception has been processed. Signed-off-by: Varad Gautam <varad.gautam@suse.com> --- lib/x86/amd_sev_vc.c | 78 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+)