thr3ads.net - Xen devel - [PATCH v2] Fix the mistake for #DB and #OF exception [May 2012]

If this information is useful, please help other people find it:
Share via:

Hao, Xudong

2012-May-12 09:12 UTC

[PATCH v2] Fix the mistake for #DB and #OF exception

Fix the mistake for debug exception(#DB; generated by INT1), overflow
exception(#OF; generated by INTO) and int n instruction emulation.

#DB should use hardware exception(except #DB generated by opcode 0xf1), #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 cd4dd23a831d xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c	Fri May 11 18:59:07 2012 +0100
+++ b/xen/arch/x86/hvm/vmx/vmx.c	Mon May 13 01:01:24 2013 +0800
@@ -1350,6 +1350,14 @@ 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. INT3 and 
+ * INT0 use software exception, and INT n should use 
+ * software interrupt.
+ */
 void vmx_inject_hw_exception(int trap, int error_code)
 {
     unsigned long intr_info;
@@ -1365,7 +1373,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 +1394,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;

Dong, Eddie

2012-May-14 01:26 UTC

head link

Re: [PATCH v2] Fix the mistake for #DB and #OF exception

Xudong:
	VM_ENTRY_INSTRUCTION_LEN is hard to detect due to the prefix instruction. We
may rely on the caller to handle...
Thx, Eddie
> -----Original Message-----
> From: Hao, Xudong
> Sent: Saturday, May 12, 2012 5:13 PM
> To: Jan Beulich (JBeulich@suse.com); Keir Fraser (keir.xen@gmail.com)
> Cc: Aravindh Puthiyaparambil; Dong, Eddie; Zhang, Xiantao; Nakajima, Jun;
> xen-devel (xen-devel@lists.xen.org)
> Subject: [PATCH v2] Fix the mistake for #DB and #OF exception
> 
> Fix the mistake for debug exception(#DB; generated by INT1), overflow
> exception(#OF; generated by INTO) and int n instruction emulation.
> 
> #DB should use hardware exception(except #DB generated by opcode 0xf1),
> #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 cd4dd23a831d xen/arch/x86/hvm/vmx/vmx.c
> --- a/xen/arch/x86/hvm/vmx/vmx.c	Fri May 11 18:59:07 2012 +0100
> +++ b/xen/arch/x86/hvm/vmx/vmx.c	Mon May 13 01:01:24 2013 +0800
> @@ -1350,6 +1350,14 @@ 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. INT3 and
> + * INT0 use software exception, and INT n should use
> + * software interrupt.
> + */
>  void vmx_inject_hw_exception(int trap, int error_code)
>  {
>      unsigned long intr_info;
> @@ -1365,7 +1373,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 +1394,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;

Jan Beulich

2012-May-14 08:05 UTC

head link

Re: [PATCH v2] Fix the mistake for #DB and #OF exception

>>> On 12.05.12 at 11:12, "Hao, Xudong"
<xudong.hao@intel.com> wrote:
> Fix the mistake for debug exception(#DB; generated by INT1), overflow 
> exception(#OF; generated by INTO) and int n instruction emulation.
> 
> #DB should use hardware exception(except #DB generated by opcode 0xf1), #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 cd4dd23a831d xen/arch/x86/hvm/vmx/vmx.c
> --- a/xen/arch/x86/hvm/vmx/vmx.c	Fri May 11 18:59:07 2012 +0100
> +++ b/xen/arch/x86/hvm/vmx/vmx.c	Mon May 13 01:01:24 2013 +0800
> @@ -1350,6 +1350,14 @@ 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. INT3 and 
> + * INT0 use software exception, and INT n should use 
INTO ...
> + * software interrupt.
> + */
Neither comment nor description still say anything about what needs
to be fixed going forward (namely the need to properly handle INT nn
when nn < 0x20).
>  void vmx_inject_hw_exception(int trap, int error_code)
>  {
>      unsigned long intr_info;
> @@ -1365,7 +1373,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);
While the description correctly mentions the opcode 0xf1 case, the
code makes no attempt at dealing with it. At least a comment would
seem appropriate here, indicating the need for further adjustment.
> @@ -1387,10 +1394,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 */
So you''re adding one more of these incorrect length settings. This
is particularly harmful here, as iirc some gcc versions generate
2-byte INT 4 instructions in certain overflow checking functions.

As this needs to be taken care of here anyway, we should aim at
fixing it for the other code paths too (as I just saw Eddie also
suggests).

Jan
> +        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;

Hao, Xudong

2012-May-14 09:41 UTC

head link

Re: [PATCH v2] Fix the mistake for #DB and #OF exception

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Monday, May 14, 2012 4:05 PM
> To: Hao, Xudong
> Cc: Keir Fraser(keir.xen@gmail.com); Dong, Eddie; Nakajima, Jun; Zhang,
> Xiantao; xen-devel(xen-devel@lists.xen.org); Aravindh Puthiyaparambil
> Subject: Re: [PATCH v2] Fix the mistake for #DB and #OF exception
> 
> >>> On 12.05.12 at 11:12, "Hao, Xudong"
<xudong.hao@intel.com> wrote:
> > Fix the mistake for debug exception(#DB; generated by INT1), overflow
> > exception(#OF; generated by INTO) and int n instruction emulation.
> >
> > #DB should use hardware exception(except #DB generated by opcode
0xf1),
> #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 cd4dd23a831d xen/arch/x86/hvm/vmx/vmx.c
> > --- a/xen/arch/x86/hvm/vmx/vmx.c	Fri May 11 18:59:07 2012 +0100
> > +++ b/xen/arch/x86/hvm/vmx/vmx.c	Mon May 13 01:01:24 2013 +0800
> > @@ -1350,6 +1350,14 @@ 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. INT3 and
> > + * INT0 use software exception, and INT n should use
> 
> INTO ...
> 
> > + * software interrupt.
> > + */
> 
> Neither comment nor description still say anything about what needs
> to be fixed going forward (namely the need to properly handle INT nn
> when nn < 0x20).
> 
> >  void vmx_inject_hw_exception(int trap, int error_code)
> >  {
> >      unsigned long intr_info;
> > @@ -1365,7 +1373,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);
> 
> While the description correctly mentions the opcode 0xf1 case, the
> code makes no attempt at dealing with it. At least a comment would
> seem appropriate here, indicating the need for further adjustment.
> 
> > @@ -1387,10 +1394,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 */
> 
> So you''re adding one more of these incorrect length settings. This
> is particularly harmful here, as iirc some gcc versions generate
> 2-byte INT 4 instructions in certain overflow checking functions.
> 
> As this needs to be taken care of here anyway, we should aim at
> fixing it for the other code paths too (as I just saw Eddie also
> suggests).
> 
I will clean this patch only for fixing the mistake of int3, #DB and #OF just as
Eddie''s suggestion.
> Jan
> 
> > +        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 - May 2012 - [PATCH v2] Fix the mistake for #DB and #OF exception

[PATCH v2] Fix the mistake for #DB and #OF exception

Re: [PATCH v2] Fix the mistake for #DB and #OF exception

Re: [PATCH v2] Fix the mistake for #DB and #OF exception

Re: [PATCH v2] Fix the mistake for #DB and #OF exception