[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH for-4.5 RFC v2] x86/HVM: Unconditionally crash guests on repeated vmentry failures



At 08:42 +0000 on 27 Nov (1417074133), Jan Beulich wrote:
> >>> On 26.11.14 at 18:43, <andrew.cooper3@xxxxxxxxxx> wrote:
> > My v1 patch only fixes the VMX side of things.  SVM doesn't explicitly
> > identify a failed vmentry and lets it fall into the default case of the
> > vmexit handler.  As such, with v1, the infinite loop still affects AMD
> > hardware.
> 
> Right; I should have said "something along the lines of v1". An SVM
> part is needed, but that should probably extend beyond what you
> proposed in v2: There are a number of "goto exit_and_crash"
> statements ahead of where you place your addition. I think they
> all need to be treated similarly.
> 
> I therefore think we should revert the VMX part of 28b4baacd5
> and make SVM behavior consistent with what results for VMX:
> Crash the guest unconditionally on VMEXIT_INVALID, without
> looking for recurring VM entry failures. See below/attached.
> 
> Jan
> 
> x86/HVM: prevent infinite VM entry retries
> 
> This reverts the VMX side of commit 28b4baac ("x86/HVM: don't crash
> guest upon problems occurring in user mode") and gets SVM in line with
> the resulting VMX behavior. This is because Andrew validly says
> 
> "A failed vmentry is overwhelmingly likely to be caused by corrupt
>  VMC[SB] state.  As a result, injecting a fault and retrying the the
>  vmentry is likely to fail in the same way."
> 
> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Looking at the SVM side, AFAICT you're trying to filter out
VMEXIT_INVALID exits that couldn't be handled by nested SVM, but I
think it's fine just to always crash on nested-SVM failures: we know
the guest wasn't in user mode because it successfully executed VMRUN.
And looking at it, the other users of that label are for unexpected
debugging exits, which can't be caused by the guest userspace either.

So how about this for the SVM side, reverting to crashing for
everything except new, unsupported exit types?

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 8aca6e6..8d28578 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2675,16 +2675,18 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
         break;
 
     default:
-    exit_and_crash:
         gdprintk(XENLOG_ERR, "unexpected VMEXIT: exit reason = %#"PRIx64", "
                  "exitinfo1 = %#"PRIx64", exitinfo2 = %#"PRIx64"\n",
                  exit_reason, 
                  (u64)vmcb->exitinfo1, (u64)vmcb->exitinfo2);
-        if ( vmcb_get_cpl(vmcb) )
+        if ( vmcb_get_cpl(vmcb) ) {
             hvm_inject_hw_exception(TRAP_invalid_op,
                                     HVM_DELIVER_NO_ERROR_CODE);
-        else
-            domain_crash(v->domain);
+            break;
+        }
+        /* else fall through */
+    exit_and_crash:
+        domain_crash(v->domain);
         break;
     }

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.