Re: [Xen-devel] [PATCH v3] Fix the mistake of exception execution

["Hao, Xudong" <xudong.hao@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Monday, May 14, 2012 7:07 PM
> To: Hao, Xudong
> Cc: Keir Fraser(keir.xen@xxxxxxxxx); Dong, Eddie; Nakajima, Jun; Zhang,
> Xiantao; xen-devel (xen-devel@xxxxxxxxxxxxx); Aravindh Puthiyaparambil
> Subject: Re: [PATCH v3] Fix the mistake of exception execution
> 
> >>> On 14.05.12 at 12:41, "Hao, Xudong" <xudong.hao@xxxxxxxxx> wrote:
> > Fix the mistake for debug exception(#DB), overflow exception(#OF; generated
> > by INTO) and int 3(#BP) instruction emulation.
> >
> > 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: 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    Wed May 15 02:31:34 2013 +0800
> > @@ -1350,6 +1350,19 @@ 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; opcode
> > + * 0xf1 generated #DB should use privileged software
> > + * exception, which is not deliverd here either.
> > + *    The caller of this function should set correct instruction
> > + * length.
> > + */
> >  void vmx_inject_hw_exception(int trap, int error_code)
> >  {
> >      unsigned long intr_info;
> > @@ -1365,7 +1378,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);
> > @@ -1383,16 +1395,14 @@ void vmx_inject_hw_exception(int trap, i
> >              return;
> >          }
> >
> > -        type = X86_EVENTTYPE_SW_EXCEPTION;
> > -        __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 1); /* int3 */
> > -        break;
> > -
> > +        type = X86_EVENTTYPE_SW_EXCEPTION; /* int3; CC */
> > +        break;
> > +
> > +    case TRAP_overflow:
> > +        type = X86_EVENTTYPE_SW_EXCEPTION;  /* into; CE */
> > +        break;
> > +
> >      default:
> > -        if ( trap > TRAP_last_reserved )
> > -        {
> > -            type = X86_EVENTTYPE_SW_EXCEPTION;
> > -            __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 2); /* int imm8 */
> > -        }
> 
> So this undoes Aravindh's earlier change, without replacement. I
> don't think that's acceptable.
> 

This is the first patch that just correct some instruction in hw exception 
function, as function description above, int n (n > 32) is not delivered by 
this function. 
I'll write another patch of new function for int n handler.

> >          break;
> >      }
> >
> > @@ -2447,6 +2457,11 @@ void vmx_vmexit_handler(struct cpu_user_
> >                  if ( handled < 0 )
> >                  {
> >                      vmx_inject_exception(TRAP_int3,
> HVM_DELIVER_NO_ERROR_CODE, 0);
> > +                    /*
> > +                     * According to the vmx_inject_hw_exception()
> description,
> > +                     * it must set correct instruction length by caller
> itself.
> > +                     */
> > +                    __vmwrite(VM_ENTRY_INSTRUCTION_LEN, 1); /*
> int3, CC */
> 
> Still using a hard-coded 1 here, the more that afaict you can't
> distinguish CC and CD 03 here.
> 

Just copied it from original code, how about this replacement:

+     __vmwrite(VM_ENTRY_INSTRUCTION_LEN, __vmread(VM_EXIT_INSTRUCTION_LEN));

> Furthermore - is this really the only place needing adjustment after
> the removal of the corresponding writes above?
> 

Yes, I searched whole code, only this place need to do adjustment after 
function changes.

> Jan
> 
> >                      break;
> >                  }
> >                  else if ( handled )
> 
> 


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