From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 4/6] x86/emulate: Support for emulating software event injection Date: Wed, 24 Sep 2014 14:04:29 +0100 Message-ID: <5422C15D.7030003@citrix.com> References: <1411484611-31027-1-git-send-email-andrew.cooper3@citrix.com> <1411484611-31027-5-git-send-email-andrew.cooper3@citrix.com> <5422C0AE.3090404@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5422C0AE.3090404@oracle.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: Boris Ostrovsky , Xen-devel Cc: Aravind Gopalakrishnan , Suravee Suthikulpanit , Jan Beulich List-Id: xen-devel@lists.xenproject.org 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 >> Signed-off-by: Andrew Cooper >> Signed-off-by: Jan Beulich >> CC: Boris Ostrovsky >> CC: Suravee Suthikulpanit >> CC: Aravind Gopalakrishnan >> --- >> 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 >> #include >> #include >> +#include >> 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. ~Andrew