[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 9/23/2014 10: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/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;

Why remove the above line?
It's common for all cases below.
We can leave it as it is and set it selectively to X86_EVENTTYPE_SW_INTERRUPT as
you do in 'case X86_EVENTTYPE_SW_INTERRUPT ' right?

-Aravind.

      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.
+         */
+        if ( cpu_has_svm_nrips )
+            vmcb->nextrip = regs->eip;
+        event.fields.type = X86_EVENTTYPE_HW_EXCEPTION;
+        break;
+
+    case X86_EVENTTYPE_SW_EXCEPTION: /* int3, into */
+        /*
+         * The AMD manual states that .type=3 (HW exception), .vector=3 or 4,
+         * will perform DPL checks.  Experimentally, DPL and presence checks
+         * are indeed performed, even without NextRIP support.
+         *
+         * However without NextRIP support, the event injection still needs
+         * fully emulating to get the correct eip in the trap frame, yet get
+         * the correct faulting eip should a fault occur.
+         */
+        if ( cpu_has_svm_nrips )
+            vmcb->nextrip = regs->eip + _trap.insn_len;
+        event.fields.type = X86_EVENTTYPE_HW_EXCEPTION;
+        break;
+
+    default:
+        event.fields.type = X86_EVENTTYPE_HW_EXCEPTION;
+        event.fields.ev = (_trap.error_code != HVM_DELIVER_NO_ERROR_CODE);
+        event.fields.errorcode = _trap.error_code;
+        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®.