Hi Tom:
      Thanks for your review.

On 10/1/2021 2:27 AM, Tom Lendacky wrote:

On 9/30/21 8:05 AM, Tianyu Lan wrote:

quoted

From: Tianyu Lan <redacted>

...

quoted

diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c
index 9f90f460a28c..dd7f37de640b 100644
--- a/arch/x86/kernel/sev-shared.c
+++ b/arch/x86/kernel/sev-shared.c

@@ -94,10 +94,9 @@ 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)
+enum es_result sev_es_ghcb_hv_call_simple(struct ghcb *ghcb,
+                   u64 exit_code, u64 exit_info_1,
+                   u64 exit_info_2)
  {
      enum es_result ret;

@@ -109,29 +108,45 @@ static enum es_result sev_es_ghcb_hv_call(struct 

ghcb *ghcb,
      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) {
-        u64 info = ghcb->save.sw_exit_info_2;
-        unsigned long v;
-
-        info = ghcb->save.sw_exit_info_2;
-        v = info & SVM_EVTINJ_VEC_MASK;
-
-        /* Check if exception information from hypervisor is sane. */
-        if ((info & SVM_EVTINJ_VALID) &&
-            ((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;
-        }
-    } else {
+    if ((ghcb->save.sw_exit_info_1 & 0xffffffff) == 1)

Really, any non-zero value indicates an error, so this should be:

     if (ghcb->save.sw_exit_info_1 & 0xffffffff) >

quoted

+        ret = ES_VMM_ERROR;
+    else
          ret = ES_OK;
+
+    return ret;
+}
+
+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)
+{
+    unsigned long v;
+    enum es_result ret;
+    u64 info;
+
+    sev_es_wr_ghcb_msr(__pa(ghcb));
+
+    ret = sev_es_ghcb_hv_call_simple(ghcb, exit_code, exit_info_1,
+                     exit_info_2);
+    if (ret == ES_OK)
+        return ret;
+

And then here, the explicit check for 1 should be performed and if not 
1, then return ES_VMM_ERROR. If it is 1, then check the event injection 
values. >
Thanks,
Tom

Good suggestion. Will update.

quoted

+    info = ghcb->save.sw_exit_info_2;
+    v = info & SVM_EVTINJ_VEC_MASK;
+
+    /* Check if exception information from hypervisor is sane. */
+    if ((info & SVM_EVTINJ_VALID) &&
+        ((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 ret;