On Wed, Feb 9, 2022 at 8:44 AM Varad Gautam <varad.gautam@xxxxxxxx> 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@xxxxxxxx>
> ---
> 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
>
