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

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Hao, Xudong
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<[hidden email]>
Signed-off-by: Xudong Hao <[hidden email]>

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;

_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

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

Dong, Eddie
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 ([hidden email]); Keir Fraser ([hidden email])
> Cc: Aravindh Puthiyaparambil; Dong, Eddie; Zhang, Xiantao; Nakajima, Jun;
> xen-devel ([hidden email])
> 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<[hidden email]>
> Signed-off-by: Xudong Hao <[hidden email]>
>
> 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;

_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

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

Jan Beulich-2
In reply to this post by Hao, Xudong
>>> On 12.05.12 at 11:12, "Hao, Xudong" <[hidden email]> 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<[hidden email]>
> Signed-off-by: Xudong Hao <[hidden email]>
>
> 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;




_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

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

Hao, Xudong
> -----Original Message-----
> From: Jan Beulich [mailto:[hidden email]]
> Sent: Monday, May 14, 2012 4:05 PM
> To: Hao, Xudong
> Cc: Keir Fraser([hidden email]); Dong, Eddie; Nakajima, Jun; Zhang,
> Xiantao; xen-devel([hidden email]); Aravindh Puthiyaparambil
> Subject: Re: [PATCH v2] Fix the mistake for #DB and #OF exception
>
> >>> On 12.05.12 at 11:12, "Hao, Xudong" <[hidden email]> 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<[hidden email]>
> > Signed-off-by: Xudong Hao <[hidden email]>
> >
> > 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 mailing list
[hidden email]
http://lists.xen.org/xen-devel