[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 10:20 AM, Andrew Cooper wrote:
On 24/09/14 14:24, Boris Ostrovsky wrote:
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.
With NRIPS support, hardware can deal with injecting the trap and doing
DPL/presence checks, which means the eip in the exception frame might
either be a trap or a fault.
regs->eip and vmcb->nextrip bound the trapping instruction, with eip
pointing at it, and nextrip pointing after it. These are the two
choices which get set in the exception frame, after hardware has decided
whether to send the trap, or fault because of a bad IDT setup.
Without NRIP support, the emulator must move regs->eip on to get the
trap frame eip pointing at the correct address as it is the only
available choice for an exception frame.
For ICEBP, there is no hardware support for injection at all (even with
NRIP available). The emulator must move regs->eip forward to get the
correct trap eip for the non-NRIP case, but with NRIP support,
vmcb->nextrip must be set, or the resulting trap frame contains an eip of 0.
I suppose this looks a little weird because of the intersection of
x86_swint_emulate_icebp and x86_swint_emulate_all, but it is not correct
to inject a software event with NRIP support for icebp, as that would
cause hardware to do a DPL check and fail to set the external bit in a
#NP error code.
Thanks.
My problem was that I didn't understand what the first 'if' in
inject_swint() was doing. I first read it as if it was trying to figure
out which instruction is being emulated.
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
