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

Re: [Xen-devel] [PATCH 4/6] x86/emulate: Support for emulating software event injection



On 09/24/2014 09:04 AM, Andrew Cooper wrote:
On 24/09/14 14:01, Boris Ostrovsky wrote:
On 09/23/2014 11:03 AM, Andrew Cooper wrote:
AMD SVM requires all software events to have their injection emulated if
hardware lacks NextRIP support.  In addition, `icebp` (opcode 0xf1)
injection
requires emulation in all cases, even with hardware NextRIP support.

Emulating full control transfers is overkill for our needs.  All that
matters
is that guest userspace can't bypass the descriptor DPL check.  Any
guest OS
which would incur other faults as part of injection is going to end
up with a
double fault instead, and won't be in a position to care that the
faulting eip
is wrong.

Reported-by: Andrei LUTAS <vlutas@xxxxxxxxxxxxxxx>
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
CC: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@xxxxxxx>
---
   xen/arch/x86/hvm/emulate.c             |    8 +++
   xen/arch/x86/hvm/svm/svm.c             |   57 +++++++++++++--
   xen/arch/x86/mm.c                      |    2 +
   xen/arch/x86/mm/shadow/common.c        |    1 +
   xen/arch/x86/x86_emulate/x86_emulate.c |  122
++++++++++++++++++++++++++++++--
   xen/arch/x86/x86_emulate/x86_emulate.h |   10 +++
   6 files changed, 191 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 7ee146b..463ccfb 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -21,6 +21,7 @@
   #include <asm/hvm/hvm.h>
   #include <asm/hvm/trace.h>
   #include <asm/hvm/support.h>
+#include <asm/hvm/svm/svm.h>
     static void hvmtrace_io_assist(int is_mmio, ioreq_t *p)
   {
@@ -1328,6 +1329,13 @@ static int _hvm_emulate_one(struct
hvm_emulate_ctxt *hvmemul_ctxt,
       vio->mmio_retrying = vio->mmio_retry;
       vio->mmio_retry = 0;
   +    if ( cpu_has_vmx )
+        hvmemul_ctxt->ctxt.swint_emulate = x86_swint_emulate_none;
+    else if ( cpu_has_svm_nrips )
+        hvmemul_ctxt->ctxt.swint_emulate = x86_swint_emulate_icebp;
+    else
+        hvmemul_ctxt->ctxt.swint_emulate = x86_swint_emulate_all;
+
       rc = x86_emulate(&hvmemul_ctxt->ctxt, ops);
         if ( rc == X86EMUL_OKAY && vio->mmio_retry )
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index de982fd..b6beefc 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1177,11 +1177,12 @@ static void svm_inject_trap(struct hvm_trap
*trap)
       struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb;
       eventinj_t event = vmcb->eventinj;
       struct hvm_trap _trap = *trap;
+    const struct cpu_user_regs *regs = guest_cpu_user_regs();
         switch ( _trap.vector )
       {
       case TRAP_debug:
-        if ( guest_cpu_user_regs()->eflags & X86_EFLAGS_TF )
+        if ( regs->eflags & X86_EFLAGS_TF )
           {
               __restore_debug_registers(vmcb, curr);
               vmcb_set_dr6(vmcb, vmcb_get_dr6(vmcb) | 0x4000);
@@ -1209,10 +1210,58 @@ static void svm_inject_trap(struct hvm_trap
*trap)
         event.bytes = 0;
       event.fields.v = 1;
-    event.fields.type = X86_EVENTTYPE_HW_EXCEPTION;
       event.fields.vector = _trap.vector;
-    event.fields.ev = (_trap.error_code != HVM_DELIVER_NO_ERROR_CODE);
-    event.fields.errorcode = _trap.error_code;
+
+    /* Refer to AMD Vol 2: System Programming, 15.20 Event
Injection. */
+    switch ( _trap.type )
+    {
+    case X86_EVENTTYPE_SW_INTERRUPT: /* int $n */
+        /*
+         * Injection type 4 (software interrupt) is only supported with
+         * NextRIP support.  Without NextRIP, the emulator will have
performed
+         * DPL and presence checks for us.
+         */
+        if ( cpu_has_svm_nrips )
+        {
+            vmcb->nextrip = regs->eip + _trap.insn_len;
+            event.fields.type = X86_EVENTTYPE_SW_INTERRUPT;
+        }
+        else
+            event.fields.type = X86_EVENTTYPE_HW_EXCEPTION;
+        break;
+
+    case X86_EVENTTYPE_PRI_SW_EXCEPTION: /* icebp */
+        /*
+         * icebp's injection must always be emulated.  Software
injection help
+         * in x86_emulate has moved eip forward, but NextRIP (if
used) still
+         * needs setting or execution will resume from 0.
+         */

Can you tell me where eip is updated? I don't see any difference
between how, for example, int3 is emulated differently from icebp.

-boris
The second hunk in this series, chooses between x86_swint_emulate_icebp
and x86_swint_emulate_all based on NRIP support.

This cause inject_swint() to either emulate the injection or not.

I was asking about why you are calculating nextrip differently (i.e not adding _trap.insn_len) for icebp vs. int3. I read the comment as if it was saying that x86_emulate() has already taken care of that. And I don't understand where that is done.

-boris

_______________________________________________
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®.