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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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).

>> 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


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