Re: [PATCH 2/7] x86: hvm events: merge 2 functions into 1

[Corneliu ZUZU <czuzu@bitdefender.com>]

On 2/8/2016 7:15 PM, Andrew Cooper wrote:
> On 08/02/16 16:57, Corneliu ZUZU wrote:
>> diff --git a/xen/include/asm-x86/hvm/event.h b/xen/include/asm-x86/hvm/event.h
>> index 11eb1fe..7c2252b 100644
>> --- a/xen/include/asm-x86/hvm/event.h
>> +++ b/xen/include/asm-x86/hvm/event.h
>> @@ -27,9 +27,8 @@ bool_t hvm_event_cr(unsigned int index, unsigned long value,
>>   #define hvm_event_crX(what, new, old) \
>>       hvm_event_cr(VM_EVENT_X86_##what, new, old)
>>   void hvm_event_msr(unsigned int msr, uint64_t value);
>> -/* Called for current VCPU: returns -1 if no listener */
>> -int hvm_event_int3(unsigned long rip);
>> -int hvm_event_single_step(unsigned long rip);
>> +int hvm_event_software_breakpoint(unsigned long rip,
>> +                                  bool_t single_step);
> Are we liable to ever gain any other type of software breakpoint?  Might
> it be more sense to pass an enum rather than a bool here?
>
> Otherwise, the changes look sensible.
>
> ~Andrew
>

Well, IMHO, no. Besides breakpointing/trapping after each instruction 
(i.e. VM_EVENT_REASON_SINGLESTEP)
and on arbitrary instructions (VM_EVENT_REASON_SOFTWARE_BREAKPOINT) I 
don't see what other
types of breakpointing one can define (at least from the hypervisor's 
point of view), and if in the future
there will be other types, that could also be changed into an enum then.
But making that param an enum now would also be fine by me.
Since I noticed Tamas also prefers this option, I will make that change.

Corneliu.