On 2/12/22 9:54 PM, Marc Orr wrote:
> 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.
>
Changed in v3.
>> +
>> + return;
>> }
>> --
>> 2.32.0
>>
>
