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@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 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 */
- }
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 */
break;
}
else if ( handled )
>>> On 14.05.12 at 12:41, "Hao, Xudong" <xudong.hao@intel.com> 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@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 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.> 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. Furthermore - is this really the only place needing adjustment after the removal of the corresponding writes above? Jan> break; > } > else if ( handled )
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: Monday, May 14, 2012 7:07 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 v3] Fix the mistake of exception execution > > >>> On 14.05.12 at 12:41, "Hao, Xudong" <xudong.hao@intel.com> 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@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 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 ) > >
>>> On 15.05.12 at 07:59, "Hao, Xudong" <xudong.hao@intel.com> wrote: >> From: Jan Beulich [mailto:JBeulich@suse.com] >> >>> On 14.05.12 at 12:41, "Hao, Xudong" <xudong.hao@intel.com> wrote: >> > 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.In that case it would have been nice to indicate that you don''t expect this to be applied just yet (i.e. by marking the patch RFC).>> > + __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));That''s okay as long as on all possible code paths arriving here VM_EXIT_INSTRUCTION_LEN is actually valid. I''m suspicious this might not be the case (especially in the case of injection originating from libxc).>> 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.Thanks, that''s good to be sure of. Jan
Aravindh Puthiyaparambil
2012-May-15 07:22 UTC
Re: [PATCH v3] Fix the mistake of exception execution
On Mon, May 14, 2012 at 11:48 PM, Jan Beulich <JBeulich@suse.com> wrote:> > >>> On 15.05.12 at 07:59, "Hao, Xudong" <xudong.hao@intel.com> wrote: > >> From: Jan Beulich [mailto:JBeulich@suse.com] > >> >>> On 14.05.12 at 12:41, "Hao, Xudong" <xudong.hao@intel.com> wrote: > >> > 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. > > In that case it would have been nice to indicate that you don''t expect > this to be applied just yet (i.e. by marking the patch RFC). > > >> > + __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)); > > That''s okay as long as on all possible code paths arriving here > VM_EXIT_INSTRUCTION_LEN is actually valid. I''m suspicious this might > not be the case (especially in the case of injection originating from > libxc).Your suspicion is warranted. IIRC this did not work for the libxc case injecting software interrupts. That is why I hard coded the instruction length. Maybe the instruction length can be made caller specific? Thanks, Aravindh
>>> On 15.05.12 at 09:22, Aravindh Puthiyaparambil <aravindh@virtuata.com> wrote: > On Mon, May 14, 2012 at 11:48 PM, Jan Beulich <JBeulich@suse.com> wrote: >> >> >>> On 15.05.12 at 07:59, "Hao, Xudong" <xudong.hao@intel.com> wrote: >> >> From: Jan Beulich [mailto:JBeulich@suse.com] >> >> >>> On 14.05.12 at 12:41, "Hao, Xudong" <xudong.hao@intel.com> wrote: >> >> > 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. >> >> In that case it would have been nice to indicate that you don''t expect >> this to be applied just yet (i.e. by marking the patch RFC). >> >> >> > + __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)); >> >> That''s okay as long as on all possible code paths arriving here >> VM_EXIT_INSTRUCTION_LEN is actually valid. I''m suspicious this might >> not be the case (especially in the case of injection originating from >> libxc). > > Your suspicion is warranted. IIRC this did not work for the libxc case > injecting software interrupts. That is why I hard coded the > instruction length. Maybe the instruction length can be made caller > specific?Yes, this is what I think is needed. It should probably be an input, with some special value (negative? zero?) indicating to use the __vmread() as above (so that instruction emulation callers don''t need to care). Jan
> -----Original Message----- > From: Aravindh Puthiyaparambil [mailto:aravindh@virtuata.com] > Sent: Tuesday, May 15, 2012 3:23 PM > To: Jan Beulich > Cc: Hao, Xudong; Keir Fraser(keir.xen@gmail.com); Dong, Eddie; Nakajima, Jun; > Zhang, Xiantao; xen-devel (xen-devel@lists.xen.org) > Subject: Re: [PATCH v3] Fix the mistake of exception execution > > On Mon, May 14, 2012 at 11:48 PM, Jan Beulich <JBeulich@suse.com> wrote: > > > > >>> On 15.05.12 at 07:59, "Hao, Xudong" <xudong.hao@intel.com> wrote: > > >> From: Jan Beulich [mailto:JBeulich@suse.com] > > >> >>> On 14.05.12 at 12:41, "Hao, Xudong" <xudong.hao@intel.com> > wrote: > > >> > 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. > > > > In that case it would have been nice to indicate that you don''t expect > > this to be applied just yet (i.e. by marking the patch RFC). > > > > >> > + __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)); > > > > That''s okay as long as on all possible code paths arriving here > > VM_EXIT_INSTRUCTION_LEN is actually valid. I''m suspicious this might > > not be the case (especially in the case of injection originating from > > libxc). > > Your suspicion is warranted. IIRC this did not work for the libxc case > injecting software interrupts. That is why I hard coded the > instruction length. Maybe the instruction length can be made caller > specific? >What''s traps did you inject? This patch has not handle the software interrupts, but hardware exceptions and #BP, #OF software exceptions.> Thanks, > Aravindh
Aravindh Puthiyaparambil
2012-May-15 08:19 UTC
Re: [PATCH v3] Fix the mistake of exception execution
On May 15, 2012 1:15 AM, "Hao, Xudong" <xudong.hao@intel.com> wrote:> > > -----Original Message----- > > From: Aravindh Puthiyaparambil [mailto:aravindh@virtuata.com] > > Sent: Tuesday, May 15, 2012 3:23 PM > > To: Jan Beulich > > Cc: Hao, Xudong; Keir Fraser(keir.xen@gmail.com); Dong, Eddie;Nakajima, Jun;> > Zhang, Xiantao; xen-devel (xen-devel@lists.xen.org) > > Subject: Re: [PATCH v3] Fix the mistake of exception execution > > > > On Mon, May 14, 2012 at 11:48 PM, Jan Beulich <JBeulich@suse.com> wrote: > > > > > > >>> On 15.05.12 at 07:59, "Hao, Xudong" <xudong.hao@intel.com> wrote: > > > >> From: Jan Beulich [mailto:JBeulich@suse.com] > > > >> >>> On 14.05.12 at 12:41, "Hao, Xudong" <xudong.hao@intel.com> > > wrote: > > > >> > 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 hwexception> > > > function, as function description above, int n (n > 32) is notdelivered by> > > > this function. > > > > I''ll write another patch of new function for int n handler. > > > > > > In that case it would have been nice to indicate that you don''t expect > > > this to be applied just yet (i.e. by marking the patch RFC). > > > > > > >> > + __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)); > > > > > > That''s okay as long as on all possible code paths arriving here > > > VM_EXIT_INSTRUCTION_LEN is actually valid. I''m suspicious this might > > > not be the case (especially in the case of injection originating from > > > libxc). > > > > Your suspicion is warranted. IIRC this did not work for the libxc case > > injecting software interrupts. That is why I hard coded the > > instruction length. Maybe the instruction length can be made caller > > specific? > > > > What''s traps did you inject? This patch has not handle the softwareinterrupts, but hardware exceptions and #BP, #OF software exceptions.>The function handles software interrupts though marked as software exception. Incorrect it might be but it works. Your patch removes that code. Thanks, Aravindh _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel