From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 3/3] x86/HVM: don't crash guest upon problems occurring in user mode Date: Thu, 20 Nov 2014 11:06:28 +0000 Message-ID: <546DCB34.8030302@citrix.com> References: <546DCAB102000078000493E0@smtp.nue.novell.com> <546DCCC202000078000493F8@smtp.nue.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7262172203652596061==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XrPZJ-0001iJ-Sy for xen-devel@lists.xenproject.org; Thu, 20 Nov 2014 11:06:34 +0000 In-Reply-To: <546DCCC202000078000493F8@smtp.nue.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Keir Fraser , Tim Deegan List-Id: xen-devel@lists.xenproject.org --===============7262172203652596061== Content-Type: multipart/alternative; boundary="------------010300060402080808010406" --------------010300060402080808010406 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable On 20/11/14 10:13, Jan Beulich wrote: > This extends commit 5283b310 ("x86/HVM: only kill guest when unknown VM= > exit occurred in guest kernel mode") to further cases, including the > failed VM entry one that XSA-110 was needed to be issued for. > > Signed-off-by: Jan Beulich > > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -90,6 +90,15 @@ static bool_t amd_erratum383_found __rea > static uint64_t osvw_length, osvw_status; > static DEFINE_SPINLOCK(osvw_lock); > =20 > +/* Only crash the guest if the problem originates in kernel mode. */ > +static void svm_crash_or_gp(struct vcpu *v) > +{ > + if ( vmcb_get_cpl(v->arch.hvm_svm.vmcb) ) > + hvm_inject_hw_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_= CODE); This (and its VMX counterpart) should either deliver a #GP fault, or have its name changed to not imply a #GP fault. How about "crash_or_fault()" as an alternative which is slightly less specific? ~Andrew > + else > + domain_crash(v->domain); > +} > + > void __update_guest_eip(struct cpu_user_regs *regs, unsigned int inst_= len) > { > struct vcpu *curr =3D current; > @@ -100,7 +109,7 @@ void __update_guest_eip(struct cpu_user_ > if ( unlikely(inst_len > 15) ) > { > gdprintk(XENLOG_ERR, "Bad instruction length %u\n", inst_len);= > - domain_crash(curr->domain); > + svm_crash_or_gp(curr); > return; > } > =20 > @@ -1505,7 +1514,7 @@ static void svm_do_nested_pgfault(struct > gdprintk(XENLOG_ERR, > "SVM violation gpa %#"PRIpaddr", mfn %#lx, type %i\n", > gpa, mfn_x(mfn), p2mt); > - domain_crash(v->domain); > + svm_crash_or_gp(v); > } > =20 > static void svm_fpu_dirty_intercept(void) > @@ -2647,7 +2656,7 @@ void svm_vmexit_handler(struct cpu_user_ > printk(XENLOG_G_ERR > "%pv: Error %d handling NPF (gpa=3D%08lx ec=3D%04lx= )\n", > v, rc, vmcb->exitinfo2, vmcb->exitinfo1); > - domain_crash(v->domain); > + svm_crash_or_gp(v); > } > v->arch.hvm_svm.cached_insn_len =3D 0; > break; > @@ -2680,11 +2689,7 @@ void svm_vmexit_handler(struct cpu_user_ > "exitinfo1 =3D %#"PRIx64", exitinfo2 =3D %#"PRIx64"\n= ", > exit_reason,=20 > (u64)vmcb->exitinfo1, (u64)vmcb->exitinfo2); > - if ( vmcb_get_cpl(vmcb) ) > - hvm_inject_hw_exception(TRAP_invalid_op, > - HVM_DELIVER_NO_ERROR_CODE); > - else > - domain_crash(v->domain); > + svm_crash_or_gp(v); > break; > } > =20 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -134,6 +134,18 @@ static void vmx_vcpu_destroy(struct vcpu > passive_domain_destroy(v); > } > =20 > +/* Only crash the guest if the problem originates in kernel mode. */ > +static void vmx_crash_or_gp(struct vcpu *v) > +{ > + struct segment_register ss; > + > + vmx_get_segment_register(v, x86_seg_ss, &ss); > + if ( ss.attr.fields.dpl ) > + hvm_inject_hw_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_= CODE); > + else > + domain_crash(v->domain); > +} > + > static DEFINE_PER_CPU(struct vmx_msr_state, host_msr_state); > =20 > static const u32 msr_index[] =3D > @@ -2474,7 +2486,7 @@ static void ept_handle_violation(unsigne > if ( qualification & EPT_GLA_VALID ) > gdprintk(XENLOG_ERR, " --- GLA %#lx\n", gla); > =20 > - domain_crash(d); > + vmx_crash_or_gp(current); > } > =20 > static void vmx_failed_vmentry(unsigned int exit_reason, > @@ -2508,7 +2520,7 @@ static void vmx_failed_vmentry(unsigned=20 > vmcs_dump_vcpu(curr); > printk("**************************************\n"); > =20 > - domain_crash(curr->domain); > + vmx_crash_or_gp(curr); > } > =20 > void vmx_enter_realmode(struct cpu_user_regs *regs) > @@ -3161,19 +3173,8 @@ void vmx_vmexit_handler(struct cpu_user_ > /* fall through */ > default: > exit_and_crash: > - { > - struct segment_register ss; > - > - gdprintk(XENLOG_WARNING, "Bad vmexit (reason %#lx)\n", > - exit_reason); > - > - vmx_get_segment_register(v, x86_seg_ss, &ss); > - if ( ss.attr.fields.dpl ) > - hvm_inject_hw_exception(TRAP_invalid_op, > - HVM_DELIVER_NO_ERROR_CODE); > - else > - domain_crash(v->domain); > - } > + gdprintk(XENLOG_WARNING, "Bad vmexit (reason %#lx)\n", exit_re= ason); > + vmx_crash_or_gp(v); > break; > } > =20 > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------010300060402080808010406 Content-Type: text/html; charset="windows-1252" Content-Transfer-Encoding: 8bit
On 20/11/14 10:13, Jan Beulich wrote:
This extends commit 5283b310 ("x86/HVM: only kill guest when unknown VM
exit occurred in guest kernel mode") to further cases, including the
failed VM entry one that XSA-110 was needed to be issued for.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -90,6 +90,15 @@ static bool_t amd_erratum383_found __rea
 static uint64_t osvw_length, osvw_status;
 static DEFINE_SPINLOCK(osvw_lock);
 
+/* Only crash the guest if the problem originates in kernel mode. */
+static void svm_crash_or_gp(struct vcpu *v)
+{
+    if ( vmcb_get_cpl(v->arch.hvm_svm.vmcb) )
+        hvm_inject_hw_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE);

This (and its VMX counterpart) should either deliver a #GP fault, or have its name changed to not imply a #GP fault.

How about "crash_or_fault()" as an alternative which is slightly less specific?

~Andrew

+    else
+        domain_crash(v->domain);
+}
+
 void __update_guest_eip(struct cpu_user_regs *regs, unsigned int inst_len)
 {
     struct vcpu *curr = current;
@@ -100,7 +109,7 @@ void __update_guest_eip(struct cpu_user_
     if ( unlikely(inst_len > 15) )
     {
         gdprintk(XENLOG_ERR, "Bad instruction length %u\n", inst_len);
-        domain_crash(curr->domain);
+        svm_crash_or_gp(curr);
         return;
     }
 
@@ -1505,7 +1514,7 @@ static void svm_do_nested_pgfault(struct
     gdprintk(XENLOG_ERR,
          "SVM violation gpa %#"PRIpaddr", mfn %#lx, type %i\n",
          gpa, mfn_x(mfn), p2mt);
-    domain_crash(v->domain);
+    svm_crash_or_gp(v);
 }
 
 static void svm_fpu_dirty_intercept(void)
@@ -2647,7 +2656,7 @@ void svm_vmexit_handler(struct cpu_user_
             printk(XENLOG_G_ERR
                    "%pv: Error %d handling NPF (gpa=%08lx ec=%04lx)\n",
                    v, rc, vmcb->exitinfo2, vmcb->exitinfo1);
-            domain_crash(v->domain);
+            svm_crash_or_gp(v);
         }
         v->arch.hvm_svm.cached_insn_len = 0;
         break;
@@ -2680,11 +2689,7 @@ void svm_vmexit_handler(struct cpu_user_
                  "exitinfo1 = %#"PRIx64", exitinfo2 = %#"PRIx64"\n",
                  exit_reason, 
                  (u64)vmcb->exitinfo1, (u64)vmcb->exitinfo2);
-        if ( vmcb_get_cpl(vmcb) )
-            hvm_inject_hw_exception(TRAP_invalid_op,
-                                    HVM_DELIVER_NO_ERROR_CODE);
-        else
-            domain_crash(v->domain);
+        svm_crash_or_gp(v);
         break;
     }
 
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -134,6 +134,18 @@ static void vmx_vcpu_destroy(struct vcpu
     passive_domain_destroy(v);
 }
 
+/* Only crash the guest if the problem originates in kernel mode. */
+static void vmx_crash_or_gp(struct vcpu *v)
+{
+    struct segment_register ss;
+
+    vmx_get_segment_register(v, x86_seg_ss, &ss);
+    if ( ss.attr.fields.dpl )
+        hvm_inject_hw_exception(TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE);
+    else
+        domain_crash(v->domain);
+}
+
 static DEFINE_PER_CPU(struct vmx_msr_state, host_msr_state);
 
 static const u32 msr_index[] =
@@ -2474,7 +2486,7 @@ static void ept_handle_violation(unsigne
     if ( qualification & EPT_GLA_VALID )
         gdprintk(XENLOG_ERR, " --- GLA %#lx\n", gla);
 
-    domain_crash(d);
+    vmx_crash_or_gp(current);
 }
 
 static void vmx_failed_vmentry(unsigned int exit_reason,
@@ -2508,7 +2520,7 @@ static void vmx_failed_vmentry(unsigned 
     vmcs_dump_vcpu(curr);
     printk("**************************************\n");
 
-    domain_crash(curr->domain);
+    vmx_crash_or_gp(curr);
 }
 
 void vmx_enter_realmode(struct cpu_user_regs *regs)
@@ -3161,19 +3173,8 @@ void vmx_vmexit_handler(struct cpu_user_
     /* fall through */
     default:
     exit_and_crash:
-        {
-            struct segment_register ss;
-
-            gdprintk(XENLOG_WARNING, "Bad vmexit (reason %#lx)\n",
-                     exit_reason);
-
-            vmx_get_segment_register(v, x86_seg_ss, &ss);
-            if ( ss.attr.fields.dpl )
-                hvm_inject_hw_exception(TRAP_invalid_op,
-                                        HVM_DELIVER_NO_ERROR_CODE);
-            else
-                domain_crash(v->domain);
-        }
+        gdprintk(XENLOG_WARNING, "Bad vmexit (reason %#lx)\n", exit_reason);
+        vmx_crash_or_gp(v);
         break;
     }
 





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

--------------010300060402080808010406-- --===============7262172203652596061== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============7262172203652596061==--