From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH 3/6] x86/hvm: Don't discard the SW/HW event distinction from the emulator Date: Fri, 26 Sep 2014 16:12:57 -0400 Message-ID: <5425C8C9.6080709@oracle.com> References: <1411484611-31027-1-git-send-email-andrew.cooper3@citrix.com> <1411484611-31027-4-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-4-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: Eddie Dong , Kevin Tian , Aravind Gopalakrishnan , Jun Nakajima , Suravee Suthikulpanit List-Id: xen-devel@lists.xenproject.org On 09/23/2014 11:03 AM, Andrew Cooper wrote: > Injecting emulator software events as hardware exceptions results in a bypass > of DPL checks. As the emulator doesn't perform DPL checks itself, guest > userspace is capable of bypassing DPL checks and injecting arbitrary events. > > Propagating software event information from the emulator allows VMX to now > properly inject software events, including DPL and presence checks, as well > correct fault/trap frames. > > Reported-by: Andrei LUTAS > Signed-off-by: Andrew Cooper > Tested-by: Andrei LUTAS > Reviewed-by: Jan Beulich > CC: Boris Ostrovsky > CC: Suravee Suthikulpanit > CC: Aravind Gopalakrishnan > CC: Jun Nakajima > CC: Eddie Dong > CC: Kevin Tian Reviewed-by: Boris Ostrovsky > --- > xen/arch/x86/hvm/emulate.c | 41 ++++++++++++++++++++++++++++--------- > xen/arch/x86/hvm/io.c | 2 +- > xen/arch/x86/hvm/svm/svm.c | 2 +- > xen/arch/x86/hvm/vmx/realmode.c | 14 ++++++------- > xen/arch/x86/hvm/vmx/vmx.c | 2 +- > xen/include/asm-x86/hvm/emulate.h | 5 ++--- > 6 files changed, 43 insertions(+), 23 deletions(-) > > diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c > index 5d5d765..7ee146b 100644 > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -441,9 +441,10 @@ static int hvmemul_virtual_to_linear( > > /* This is a singleton operation: fail it with an exception. */ > hvmemul_ctxt->exn_pending = 1; > - hvmemul_ctxt->exn_vector = TRAP_gp_fault; > - hvmemul_ctxt->exn_error_code = 0; > - hvmemul_ctxt->exn_insn_len = 0; > + hvmemul_ctxt->trap.vector = TRAP_gp_fault; > + hvmemul_ctxt->trap.type = X86_EVENTTYPE_HW_EXCEPTION; > + hvmemul_ctxt->trap.error_code = 0; > + hvmemul_ctxt->trap.insn_len = 0; > return X86EMUL_EXCEPTION; > } > > @@ -1111,9 +1112,10 @@ static int hvmemul_inject_hw_exception( > container_of(ctxt, struct hvm_emulate_ctxt, ctxt); > > hvmemul_ctxt->exn_pending = 1; > - hvmemul_ctxt->exn_vector = vector; > - hvmemul_ctxt->exn_error_code = error_code; > - hvmemul_ctxt->exn_insn_len = 0; > + hvmemul_ctxt->trap.vector = vector; > + hvmemul_ctxt->trap.type = X86_EVENTTYPE_HW_EXCEPTION; > + hvmemul_ctxt->trap.error_code = error_code; > + hvmemul_ctxt->trap.insn_len = 0; > > return X86EMUL_OKAY; > } > @@ -1127,10 +1129,29 @@ static int hvmemul_inject_sw_interrupt( > struct hvm_emulate_ctxt *hvmemul_ctxt = > container_of(ctxt, struct hvm_emulate_ctxt, ctxt); > > + switch ( type ) > + { > + case x86_swint_icebp: > + hvmemul_ctxt->trap.type = X86_EVENTTYPE_PRI_SW_EXCEPTION; > + break; > + > + case x86_swint_int3: > + case x86_swint_into: > + hvmemul_ctxt->trap.type = X86_EVENTTYPE_SW_EXCEPTION; > + break; > + > + case x86_swint_int: > + hvmemul_ctxt->trap.type = X86_EVENTTYPE_SW_INTERRUPT; > + break; > + > + default: > + return X86EMUL_UNHANDLEABLE; > + } > + > hvmemul_ctxt->exn_pending = 1; > - hvmemul_ctxt->exn_vector = vector; > - hvmemul_ctxt->exn_error_code = -1; > - hvmemul_ctxt->exn_insn_len = insn_len; > + hvmemul_ctxt->trap.vector = vector; > + hvmemul_ctxt->trap.error_code = HVM_DELIVER_NO_ERROR_CODE; > + hvmemul_ctxt->trap.insn_len = insn_len; > > return X86EMUL_OKAY; > } > @@ -1404,7 +1425,7 @@ void hvm_mem_event_emulate_one(bool_t nowrite, unsigned int trapnr, > break; > case X86EMUL_EXCEPTION: > if ( ctx.exn_pending ) > - hvm_inject_hw_exception(ctx.exn_vector, ctx.exn_error_code); > + hvm_inject_trap(&ctx.trap); > break; > } > > diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c > index 9f565d6..e5d5e79 100644 > --- a/xen/arch/x86/hvm/io.c > +++ b/xen/arch/x86/hvm/io.c > @@ -113,7 +113,7 @@ int handle_mmio(void) > return 0; > case X86EMUL_EXCEPTION: > if ( ctxt.exn_pending ) > - hvm_inject_hw_exception(ctxt.exn_vector, ctxt.exn_error_code); > + hvm_inject_trap(&ctxt.trap); > break; > default: > break; > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 5d404ce..de982fd 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -2080,7 +2080,7 @@ static void svm_vmexit_ud_intercept(struct cpu_user_regs *regs) > break; > case X86EMUL_EXCEPTION: > if ( ctxt.exn_pending ) > - hvm_inject_hw_exception(ctxt.exn_vector, ctxt.exn_error_code); > + hvm_inject_trap(&ctxt.trap); > /* fall through */ > default: > hvm_emulate_writeback(&ctxt); > diff --git a/xen/arch/x86/hvm/vmx/realmode.c b/xen/arch/x86/hvm/vmx/realmode.c > index 45066b2..9a6de6c 100644 > --- a/xen/arch/x86/hvm/vmx/realmode.c > +++ b/xen/arch/x86/hvm/vmx/realmode.c > @@ -129,27 +129,27 @@ static void realmode_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt) > gdprintk(XENLOG_ERR, "Exception pending but no info.\n"); > goto fail; > } > - hvmemul_ctxt->exn_vector = (uint8_t)intr_info; > - hvmemul_ctxt->exn_insn_len = 0; > + hvmemul_ctxt->trap.vector = (uint8_t)intr_info; > + hvmemul_ctxt->trap.insn_len = 0; > } > > if ( unlikely(curr->domain->debugger_attached) && > - ((hvmemul_ctxt->exn_vector == TRAP_debug) || > - (hvmemul_ctxt->exn_vector == TRAP_int3)) ) > + ((hvmemul_ctxt->trap.vector == TRAP_debug) || > + (hvmemul_ctxt->trap.vector == TRAP_int3)) ) > { > domain_pause_for_debugger(); > } > else if ( curr->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE ) > { > gdprintk(XENLOG_ERR, "Exception %02x in protected mode.\n", > - hvmemul_ctxt->exn_vector); > + hvmemul_ctxt->trap.vector); > goto fail; > } > else > { > realmode_deliver_exception( > - hvmemul_ctxt->exn_vector, > - hvmemul_ctxt->exn_insn_len, > + hvmemul_ctxt->trap.vector, > + hvmemul_ctxt->trap.insn_len, > hvmemul_ctxt); > } > } > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 84119ed..addaa81 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -2510,7 +2510,7 @@ static void vmx_vmexit_ud_intercept(struct cpu_user_regs *regs) > break; > case X86EMUL_EXCEPTION: > if ( ctxt.exn_pending ) > - hvm_inject_hw_exception(ctxt.exn_vector, ctxt.exn_error_code); > + hvm_inject_trap(&ctxt.trap); > /* fall through */ > default: > hvm_emulate_writeback(&ctxt); > diff --git a/xen/include/asm-x86/hvm/emulate.h b/xen/include/asm-x86/hvm/emulate.h > index efff97e..6cdc57b 100644 > --- a/xen/include/asm-x86/hvm/emulate.h > +++ b/xen/include/asm-x86/hvm/emulate.h > @@ -13,6 +13,7 @@ > #define __ASM_X86_HVM_EMULATE_H__ > > #include > +#include > #include > > struct hvm_emulate_ctxt { > @@ -28,9 +29,7 @@ struct hvm_emulate_ctxt { > unsigned long seg_reg_dirty; > > bool_t exn_pending; > - uint8_t exn_vector; > - uint8_t exn_insn_len; > - int32_t exn_error_code; > + struct hvm_trap trap; > > uint32_t intr_shadow; > };