From mboxrd@z Thu Jan 1 00:00:00 1970 From: Aravind Gopalakrishnan Subject: Re: [PATCH 4/6] x86/emulate: Support for emulating software event injection Date: Tue, 23 Sep 2014 17:24:43 -0500 Message-ID: <5421F32B.2070903@amd.com> References: <1411484611-31027-1-git-send-email-andrew.cooper3@citrix.com> <1411484611-31027-5-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1411484611-31027-5-git-send-email-andrew.cooper3@citrix.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: Andrew Cooper , Xen-devel Cc: Boris Ostrovsky , Suravee Suthikulpanit , Jan Beulich List-Id: xen-devel@lists.xenproject.org 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 > 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/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; > + } > >