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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.05.12 at 09:22, Aravindh Puthiyaparambil <aravindh@xxxxxxxxxxxx> 
>>> wrote:
> On Mon, May 14, 2012 at 11:48 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>
>> >>> On 15.05.12 at 07:59, "Hao, Xudong" <xudong.hao@xxxxxxxxx> wrote:
>> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> >> >>> On 14.05.12 at 12:41, "Hao, Xudong" <xudong.hao@xxxxxxxxx> 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


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