* [PATCH v3] Fix the mistake of exception execution
@ 2012-05-14 10:41 Hao, Xudong
2012-05-14 11:06 ` Jan Beulich
0 siblings, 1 reply; 8+ messages in thread
From: Hao, Xudong @ 2012-05-14 10:41 UTC (permalink / raw)
To: Jan Beulich (JBeulich@suse.com), Keir Fraser (keir.xen@gmail.com)
Cc: Aravindh Puthiyaparambil, Dong, Eddie, Zhang, Xiantao,
Nakajima, Jun, xen-devel (xen-devel@lists.xen.org)
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 )
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] Fix the mistake of exception execution
2012-05-14 10:41 [PATCH v3] Fix the mistake of exception execution Hao, Xudong
@ 2012-05-14 11:06 ` Jan Beulich
2012-05-15 5:59 ` Hao, Xudong
0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2012-05-14 11:06 UTC (permalink / raw)
To: Xudong Hao
Cc: Aravindh Puthiyaparambil, Eddie Dong,
xen-devel (xen-devel@lists.xen.org),
Keir Fraser(keir.xen@gmail.com), Jun Nakajima, Xiantao Zhang
>>> 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 )
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] Fix the mistake of exception execution
2012-05-14 11:06 ` Jan Beulich
@ 2012-05-15 5:59 ` Hao, Xudong
2012-05-15 6:48 ` Jan Beulich
0 siblings, 1 reply; 8+ messages in thread
From: Hao, Xudong @ 2012-05-15 5:59 UTC (permalink / raw)
To: Jan Beulich
Cc: Aravindh Puthiyaparambil, Dong, Eddie,
xen-devel (xen-devel@lists.xen.org),
Keir Fraser(keir.xen@gmail.com), Nakajima, Jun, Zhang, Xiantao
> -----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 )
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] Fix the mistake of exception execution
2012-05-15 5:59 ` Hao, Xudong
@ 2012-05-15 6:48 ` Jan Beulich
2012-05-15 7:22 ` Aravindh Puthiyaparambil
0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2012-05-15 6:48 UTC (permalink / raw)
To: Xudong Hao
Cc: Aravindh Puthiyaparambil, Eddie Dong,
xen-devel (xen-devel@lists.xen.org),
Keir Fraser(keir.xen@gmail.com), Jun Nakajima, Xiantao Zhang
>>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] Fix the mistake of exception execution
2012-05-15 6:48 ` Jan Beulich
@ 2012-05-15 7:22 ` Aravindh Puthiyaparambil
2012-05-15 7:32 ` Jan Beulich
2012-05-15 8:14 ` Hao, Xudong
0 siblings, 2 replies; 8+ messages in thread
From: Aravindh Puthiyaparambil @ 2012-05-15 7:22 UTC (permalink / raw)
To: Jan Beulich
Cc: Xudong Hao, Eddie Dong, xen-devel (xen-devel@lists.xen.org),
Keir Fraser(keir.xen@gmail.com), Jun Nakajima, Xiantao Zhang
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] Fix the mistake of exception execution
2012-05-15 7:22 ` Aravindh Puthiyaparambil
@ 2012-05-15 7:32 ` Jan Beulich
2012-05-15 8:14 ` Hao, Xudong
1 sibling, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2012-05-15 7:32 UTC (permalink / raw)
To: Aravindh Puthiyaparambil
Cc: Eddie Dong, Xudong Hao, xen-devel (xen-devel@lists.xen.org),
Keir Fraser(keir.xen@gmail.com), Jun Nakajima, Xiantao Zhang
>>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] Fix the mistake of exception execution
2012-05-15 7:22 ` Aravindh Puthiyaparambil
2012-05-15 7:32 ` Jan Beulich
@ 2012-05-15 8:14 ` Hao, Xudong
2012-05-15 8:19 ` Aravindh Puthiyaparambil
1 sibling, 1 reply; 8+ messages in thread
From: Hao, Xudong @ 2012-05-15 8:14 UTC (permalink / raw)
To: Aravindh Puthiyaparambil
Cc: Nakajima, Jun, Dong, Eddie, xen-devel (xen-devel@lists.xen.org),
Keir Fraser(keir.xen@gmail.com), Jan Beulich, Zhang, Xiantao
> -----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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] Fix the mistake of exception execution
2012-05-15 8:14 ` Hao, Xudong
@ 2012-05-15 8:19 ` Aravindh Puthiyaparambil
0 siblings, 0 replies; 8+ messages in thread
From: Aravindh Puthiyaparambil @ 2012-05-15 8:19 UTC (permalink / raw)
To: Hao, Xudong
Cc: Nakajima, Jun, Dong, Eddie, xen-devel (xen-devel@lists.xen.org),
Keir Fraser(keir.xen@gmail.com), Jan Beulich, Zhang, Xiantao
[-- Attachment #1.1: Type: text/plain, Size: 2731 bytes --]
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 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.
>
The function handles software interrupts though marked as software
exception. Incorrect it might be but it works. Your patch removes that code.
Thanks,
Aravindh
[-- Attachment #1.2: Type: text/html, Size: 4125 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-05-15 8:19 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-14 10:41 [PATCH v3] Fix the mistake of exception execution Hao, Xudong
2012-05-14 11:06 ` Jan Beulich
2012-05-15 5:59 ` Hao, Xudong
2012-05-15 6:48 ` Jan Beulich
2012-05-15 7:22 ` Aravindh Puthiyaparambil
2012-05-15 7:32 ` Jan Beulich
2012-05-15 8:14 ` Hao, Xudong
2012-05-15 8:19 ` Aravindh Puthiyaparambil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).