[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 2/4] VMX: Fix the mistake of exception execution



>>> On 30.05.12 at 04:35, Xudong Hao <xudong.hao@xxxxxxxxx> wrote:
> Fix the mistake for debug exception(#DB), overflow exception(#OF) and
> INT3(#BP), INTn instruction emulation.
> 
> Add inslen field in struct hvm_trap. According to instruction length,
> to distinguish INT3 is generated by opcode 'CC' or 'CD ib =3',
> so do INTO and #DB(debug exception).

As noted previously, using the instruction length here is not fully
correct. Depending on how significant the distinction between
software interrupt and software exception is, taking into account
something like the opcode sequence 66 CC may be necessary. If
the distinction is insignificant, then perhaps a code comment
should say so.

More comments inline...

> Note:
>  * For INTn (CD ib), it should use type 4 (software interrupt).
> 
>  * For INT3 (CC; NOT CD ib with ib=3) and INTO (CE; NOT CD ib with ib=4),
>    it should use type 6 (software exception).
> 
>  * For other exceptions (#DE, #DB, #BR, #UD, #NM, #TS, #NP, #SS, #GP, #PF, 
> #MF,
>    #AC, #MC, and #XM), it should use type 3 (hardware exception).
> 
>  * In the unlikely event that you are emulating the undocumented opcode F1
>    (informally called INT1 or ICEBP), it would use type 5 (privileged 
> software
>    exception).
> 
> Signed-off-by: Xudong Hao <xudong.hao@xxxxxxxxx>
> Signed-off-by: Eddie Dong <eddie.dong@xxxxxxxxx>
> Signed-off-by: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>
> ---
>  xen/arch/x86/hvm/vmx/vmx.c    |   43 
> ++++++++++++++++++++++++++++++++++++++++-
>  xen/include/asm-x86/hvm/hvm.h |    2 +
>  2 files changed, 44 insertions(+), 1 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index c96d18b..cf08a11 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -1381,6 +1381,19 @@ void vmx_inject_nmi(void)
>                             HVM_DELIVER_NO_ERROR_CODE);
>  }
>  
> +/*
> + * Generate the virtual event to guest.
> + * NOTE:
> + *    This is for processor execution generated exceptions,
> + * and handle #DB hardware exception and all software 
> + * exception/interrupt, which include:
> + *  - INT 3(CC), INTO (CE) instruction emulation, which should
> + *    use X86_EVENTTYPE_SW_EXCEPTION;
> + *  - INT nn (CD nn) instruction emulation, which should use
> + *    X86_EVENTTYPE_SW_INTERRUPT as interrupt type;
> + *  - opcode 0xf1 generated #DB should use privileged software
> + *    exception.
> + */
>  static void vmx_inject_trap(struct hvm_trap *trap)
>  {
>      unsigned long intr_info;
> @@ -1399,6 +1412,12 @@ static void vmx_inject_trap(struct hvm_trap *trap)
>      switch ( _trap.vector )
>      {
>      case TRAP_debug:
> +        _trap.type = X86_EVENTTYPE_HW_EXCEPTION;
> +        if ( _trap.inslen != 1 ) {

Opcode F1 certainly has length 1, so perhaps the condition is
inverted? Perhaps length 0 should be used here to distinguish
the hardware exception case from the F1-generated one.

> +            _trap.type = X86_EVENTTYPE_PRI_SW_EXCEPTION;  /* opcode 0xf1 */
> +            __vmwrite(VM_ENTRY_INSTRUCTION_LEN, _trap.inslen);
> +        }
> +
>          if ( guest_cpu_user_regs()->eflags & X86_EFLAGS_TF )
>          {
>              __restore_debug_registers(curr);
> @@ -1414,6 +1433,27 @@ static void vmx_inject_trap(struct hvm_trap *trap)
>              domain_pause_for_debugger();
>              return;
>          }
> +        _trap.type = X86_EVENTTYPE_SW_EXCEPTION;  /* CC */
> +        if ( _trap.inslen != 1 )
> +            _trap.type = X86_EVENTTYPE_SW_INTERRUPT;  /* CD ib with ib=3 */
> +        __vmwrite(VM_ENTRY_INSTRUCTION_LEN, _trap.inslen);
> +        break;
> +
> +    case TRAP_overflow:
> +        _trap.type = X86_EVENTTYPE_SW_EXCEPTION;  /* CE */
> +        if ( _trap.inslen != 1 )
> +            _trap.type = X86_EVENTTYPE_SW_INTERRUPT;  /* CD ib with ib=4 */
> +        __vmwrite(VM_ENTRY_INSTRUCTION_LEN, _trap.inslen);
> +        break;
> +
> +    default:
> +        if ( _trap.vector > TRAP_last_reserved ) /* int imm8 */
> +        {
> +            _trap.type = X86_EVENTTYPE_SW_INTERRUPT;
> +            __vmwrite(VM_ENTRY_INSTRUCTION_LEN, _trap.inslen);
> +        }
> +        break;
> +
>      }
>  
>      if ( unlikely(intr_info & INTR_INFO_VALID_MASK) &&
> @@ -2424,7 +2464,8 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>                      struct hvm_trap trap = {
>                          .vector = TRAP_int3,
>                          .type = X86_EVENTTYPE_SW_EXCEPTION,
> -                        .error_code = HVM_DELIVER_NO_ERROR_CODE
> +                        .error_code = HVM_DELIVER_NO_ERROR_CODE,
> +                        .inslen = __vmread(VM_EXIT_INSTRUCTION_LEN)
>                      };
>                      hvm_inject_trap(&trap);
>                      break;
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index 65f7e20..a3d8bf1 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -76,6 +76,7 @@ struct hvm_trap {
>      unsigned int  type;         /* X86_EVENTTYPE_* */
>      int           error_code;   /* HVM_DELIVER_NO_ERROR_CODE if n/a */
>      unsigned long cr2;          /* Only for TRAP_page_fault h/w exception 
> */
> +    int           inslen;       /* Instruction length */ 

May I suggest using "insnlen" instead?

Jan

>  };
>  
>  /*
> @@ -375,6 +376,7 @@ static inline int hvm_do_pmu_interrupt(struct 
> cpu_user_regs *regs)
>  #define X86_EVENTTYPE_NMI                   2    /* NMI                */
>  #define X86_EVENTTYPE_HW_EXCEPTION          3    /* hardware exception */
>  #define X86_EVENTTYPE_SW_INTERRUPT          4    /* software interrupt */
> +#define X86_EVENTTYPE_PRI_SW_EXCEPTION      5    /* privileged software 
> exception */
>  #define X86_EVENTTYPE_SW_EXCEPTION          6    /* software exception */
>  
>  int hvm_event_needs_reinjection(uint8_t type, uint8_t vector);
> -- 
> 1.5.5



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.