Hao, Xudong
2012-May-12 06:38 UTC
[PATCH] Fix the mistake for #BP and #OF exception handler
Fix the mistake for breakpoint exception(#BP; generated by INT3), overflow exception(#OF; generated by INTO) and int n instruction emulation. #BP and #OF should use software exception, which int n instruction should use software interrupt. Signed-off-by: Eddie Dong<eddie.dong@intel.com> Signed-off-by: Xudong Hao <xudong.hao@intel.com> diff -r 98fe3b2a572d xen/arch/x86/hvm/vmx/vmx.c --- a/xen/arch/x86/hvm/vmx/vmx.c Tue May 01 14:20:37 2012 +0100 +++ b/xen/arch/x86/hvm/vmx/vmx.c Sun May 12 03:14:04 2013 +0800 @@ -1350,6 +1350,15 @@ static void __vmx_inject_exception(int t curr->arch.hvm_vmx.vmx_emulate = 1; } +/* + * Generate the virtual event to guest. + * NOTE: + * This is for processor execution generated exceptions, + * and INT 3(CC), INTO (CE) instruction emulation. It is + * not intended for the delivery of event due to emulation + * of INT nn (CD nn) instruction, which should use + * X86_EVENTTYPE_SW_INTERRUPT as interrupt type. + */ void vmx_inject_hw_exception(int trap, int error_code) { unsigned long intr_info; @@ -1365,7 +1374,6 @@ void vmx_inject_hw_exception(int trap, i switch ( trap ) { case TRAP_debug: - type = X86_EVENTTYPE_SW_EXCEPTION; if ( guest_cpu_user_regs()->eflags & X86_EFLAGS_TF ) { __restore_debug_registers(curr); @@ -1387,10 +1395,15 @@ void vmx_inject_hw_exception(int trap, i __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 1); /* int3 */ break; + case TRAP_overflow: + type = X86_EVENTTYPE_SW_EXCEPTION; + __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 1); /* into */ + break; + default: if ( trap > TRAP_last_reserved ) { - type = X86_EVENTTYPE_SW_EXCEPTION; + type = X86_EVENTTYPE_SW_INTERRUPT; __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 2); /* int imm8 */ } break;
Keir Fraser
2012-May-12 06:49 UTC
Re: [PATCH] Fix the mistake for #BP and #OF exception handler
On 12/05/2012 07:38, "Hao, Xudong" <xudong.hao@intel.com> wrote:> Fix the mistake for breakpoint exception(#BP; generated by INT3), overflow > exception(#OF; generated by INTO) and int n instruction emulation. > > #BP and #OF should use software exception, which int n instruction should use > software interrupt.This patch doesn''t fix #BP, which is already using SW_EXCEPTION. It does however fix #DB, which was using SW_EXCEPTION and you fix to use HW_EXCEPTION. Also your new comment at the top of the function is a bit wishful. It may not be suitable to use this function for INT nn, but it *is* making a half-arsed attempt at handling it anyway, and it is being called for that purpose. Perhaps the comment should admit this, but indicate that this needs to go away during 4.3 development? Apart from these gripes, your code changes look correct. If this tests okay for others, we can check this in when you provide a fixed changeset description and code comment. -- Keir> Signed-off-by: Eddie Dong<eddie.dong@intel.com> > Signed-off-by: Xudong Hao <xudong.hao@intel.com> > > diff -r 98fe3b2a572d xen/arch/x86/hvm/vmx/vmx.c > --- a/xen/arch/x86/hvm/vmx/vmx.c Tue May 01 14:20:37 2012 +0100 > +++ b/xen/arch/x86/hvm/vmx/vmx.c Sun May 12 03:14:04 2013 +0800 > @@ -1350,6 +1350,15 @@ static void __vmx_inject_exception(int t > curr->arch.hvm_vmx.vmx_emulate = 1; > } > > +/* > + * Generate the virtual event to guest. > + * NOTE: > + * This is for processor execution generated exceptions, > + * and INT 3(CC), INTO (CE) instruction emulation. It is > + * not intended for the delivery of event due to emulation > + * of INT nn (CD nn) instruction, which should use > + * X86_EVENTTYPE_SW_INTERRUPT as interrupt type. > + */ > void vmx_inject_hw_exception(int trap, int error_code) > { > unsigned long intr_info; > @@ -1365,7 +1374,6 @@ void vmx_inject_hw_exception(int trap, i > switch ( trap ) > { > case TRAP_debug: > - type = X86_EVENTTYPE_SW_EXCEPTION; > if ( guest_cpu_user_regs()->eflags & X86_EFLAGS_TF ) > { > __restore_debug_registers(curr); > @@ -1387,10 +1395,15 @@ void vmx_inject_hw_exception(int trap, i > __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 1); /* int3 */ > break; > > + case TRAP_overflow: > + type = X86_EVENTTYPE_SW_EXCEPTION; > + __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 1); /* into */ > + break; > + > default: > if ( trap > TRAP_last_reserved ) > { > - type = X86_EVENTTYPE_SW_EXCEPTION; > + type = X86_EVENTTYPE_SW_INTERRUPT; > __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 2); /* int imm8 */ > } > break; > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Hao, Xudong
2012-May-12 07:17 UTC
Re: [PATCH] Fix the mistake for #BP and #OF exception handler
> -----Original Message----- > From: Keir Fraser [mailto:keir.xen@gmail.com] > Sent: Saturday, May 12, 2012 2:50 PM > To: Hao, Xudong; Jan Beulich (JBeulich@suse.com) > Cc: Aravindh Puthiyaparambil; Dong, Eddie; Zhang, Xiantao; Nakajima, Jun; > xen-devel (xen-devel@lists.xen.org) > Subject: Re: [Xen-devel] [PATCH] Fix the mistake for #BP and #OF exception > handler > > On 12/05/2012 07:38, "Hao, Xudong" <xudong.hao@intel.com> wrote: > > > Fix the mistake for breakpoint exception(#BP; generated by INT3), overflow > > exception(#OF; generated by INTO) and int n instruction emulation. > > > > #BP and #OF should use software exception, which int n instruction should > use > > software interrupt. > > This patch doesn''t fix #BP, which is already using SW_EXCEPTION. It does > however fix #DB, which was using SW_EXCEPTION and you fix to use > HW_EXCEPTION. >Yes, it fixed #DB, #OF and int n , not for #BP.> Also your new comment at the top of the function is a bit wishful. It may > not be suitable to use this function for INT nn, but it *is* making a > half-arsed attempt at handling it anyway, and it is being called for that > purpose. Perhaps the comment should admit this, but indicate that this needs > to go away during 4.3 development? >No, the new comment on the top of function is a little suitable, I''ll correct it.> Apart from these gripes, your code changes look correct. If this tests okay > for others, we can check this in when you provide a fixed changeset > description and code comment. >I''ll supply next version to modify the description and comment.> -- Keir > > > Signed-off-by: Eddie Dong<eddie.dong@intel.com> > > Signed-off-by: Xudong Hao <xudong.hao@intel.com> > > > > diff -r 98fe3b2a572d xen/arch/x86/hvm/vmx/vmx.c > > --- a/xen/arch/x86/hvm/vmx/vmx.c Tue May 01 14:20:37 2012 +0100 > > +++ b/xen/arch/x86/hvm/vmx/vmx.c Sun May 12 03:14:04 2013 +0800 > > @@ -1350,6 +1350,15 @@ static void __vmx_inject_exception(int t > > curr->arch.hvm_vmx.vmx_emulate = 1; > > } > > > > +/* > > + * Generate the virtual event to guest. > > + * NOTE: > > + * This is for processor execution generated exceptions, > > + * and INT 3(CC), INTO (CE) instruction emulation. It is > > + * not intended for the delivery of event due to emulation > > + * of INT nn (CD nn) instruction, which should use > > + * X86_EVENTTYPE_SW_INTERRUPT as interrupt type. > > + */ > > void vmx_inject_hw_exception(int trap, int error_code) > > { > > unsigned long intr_info; > > @@ -1365,7 +1374,6 @@ void vmx_inject_hw_exception(int trap, i > > switch ( trap ) > > { > > case TRAP_debug: > > - type = X86_EVENTTYPE_SW_EXCEPTION; > > if ( guest_cpu_user_regs()->eflags & X86_EFLAGS_TF ) > > { > > __restore_debug_registers(curr); > > @@ -1387,10 +1395,15 @@ void vmx_inject_hw_exception(int trap, i > > __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 1); /* int3 */ > > break; > > > > + case TRAP_overflow: > > + type = X86_EVENTTYPE_SW_EXCEPTION; > > + __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 1); /* into */ > > + break; > > + > > default: > > if ( trap > TRAP_last_reserved ) > > { > > - type = X86_EVENTTYPE_SW_EXCEPTION; > > + type = X86_EVENTTYPE_SW_INTERRUPT; > > __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 2); /* int imm8 > */ > > } > > break; > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel >