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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.05.12 at 11:12, "Hao, Xudong" <xudong.hao@xxxxxxxxx> 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@xxxxxxxxx>
> Signed-off-by: Xudong Hao <xudong.hao@xxxxxxxxx>
> 
> 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
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel