| Message ID | 1455119433-2308-1-git-send-email-czuzu@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 10.02.16 at 16:50, <czuzu@bitdefender.com> wrote: > @@ -151,61 +154,52 @@ void hvm_event_guest_request(void) > } > } > > -int hvm_event_int3(unsigned long rip) > +static inline > +uint64_t gfn_of_rip(unsigned long rip) This should be a single line and the return value should be unsigned long. > { > - int rc = 0; > struct vcpu *curr = current; > + struct segment_register sreg; > + uint32_t pfec = PFEC_page_present | PFEC_insn_fetch; > > - if ( curr->domain->arch.monitor.software_breakpoint_enabled ) > - { > - struct segment_register sreg; > - uint32_t pfec = PFEC_page_present | PFEC_insn_fetch; > - vm_event_request_t req = { > - .reason = VM_EVENT_REASON_SOFTWARE_BREAKPOINT, > - .vcpu_id = curr->vcpu_id, > - }; > - > - hvm_get_segment_register(curr, x86_seg_ss, &sreg); > - if ( sreg.attr.fields.dpl == 3 ) > - pfec |= PFEC_user_mode; > + hvm_get_segment_register(curr, x86_seg_ss, &sreg); > + if ( sreg.attr.fields.dpl == 3 ) > + pfec |= PFEC_user_mode; > > - hvm_get_segment_register(curr, x86_seg_cs, &sreg); > - req.u.software_breakpoint.gfn = paging_gva_to_gfn(curr, > - sreg.base + rip, > - &pfec); > - > - rc = hvm_event_traps(1, &req); > - } > + hvm_get_segment_register(curr, x86_seg_cs, &sreg); > > - return rc; > + return (uint64_t) paging_gva_to_gfn(curr, sreg.base + rip, &pfec); > } Pointless cast. > --- a/xen/include/asm-x86/hvm/event.h > +++ b/xen/include/asm-x86/hvm/event.h > @@ -17,6 +17,12 @@ > #ifndef __ASM_X86_HVM_EVENT_H__ > #define __ASM_X86_HVM_EVENT_H__ > > +enum hvm_event_breakpoint_type > +{ > + HVM_EVENT_SOFTWARE_BREAKPOINT, > + HVM_EVENT_SINGLESTEP_BREAKPOINT, > +}; I don't see what good it does to put existing constants into an enum. > @@ -27,9 +33,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_breakpoint(unsigned long rip, > + enum hvm_event_breakpoint_type type); I guess the comment was here for a reason, and this reason doesn't go away with this code folding. But I'll leave it to the VM event code maintainers to judge. Jan
On 10/02/16 16:18, Jan Beulich wrote: > >> --- a/xen/include/asm-x86/hvm/event.h >> +++ b/xen/include/asm-x86/hvm/event.h >> @@ -17,6 +17,12 @@ >> #ifndef __ASM_X86_HVM_EVENT_H__ >> #define __ASM_X86_HVM_EVENT_H__ >> >> +enum hvm_event_breakpoint_type >> +{ >> + HVM_EVENT_SOFTWARE_BREAKPOINT, >> + HVM_EVENT_SINGLESTEP_BREAKPOINT, >> +}; > I don't see what good it does to put existing constants into an > enum. An enum was requested during review of v1, because "bool software_breakpoint" isn't a good function interface. I don't mind if #defines were used instead, but the parameter should have a semantic name used in calling code. ~Andrew
On 2/10/2016 6:18 PM, Jan Beulich wrote: >>>> On 10.02.16 at 16:50, <czuzu@bitdefender.com> wrote: >> @@ -151,61 +154,52 @@ void hvm_event_guest_request(void) >> } >> } >> >> -int hvm_event_int3(unsigned long rip) >> +static inline >> +uint64_t gfn_of_rip(unsigned long rip) > This should be a single line and the return value should be > unsigned long. Noted. >> + return (uint64_t) paging_gva_to_gfn(curr, sreg.base + rip, &pfec); >> } > Pointless cast. Noted. > >> --- a/xen/include/asm-x86/hvm/event.h >> +++ b/xen/include/asm-x86/hvm/event.h >> @@ -17,6 +17,12 @@ >> #ifndef __ASM_X86_HVM_EVENT_H__ >> #define __ASM_X86_HVM_EVENT_H__ >> >> +enum hvm_event_breakpoint_type >> +{ >> + HVM_EVENT_SOFTWARE_BREAKPOINT, >> + HVM_EVENT_SINGLESTEP_BREAKPOINT, >> +}; > I don't see what good it does to put existing constants into an > enum. As Andrew pointed out, an enum was requested in v1 instead of the single_step param. One could use the already existing VM_EVENT_REASON_* constants, but conceptually this function only involves a subset of those (i.e. *breakpoint vm-events*). > >> @@ -27,9 +33,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_breakpoint(unsigned long rip, >> + enum hvm_event_breakpoint_type type); > I guess the comment was here for a reason, and this reason doesn't > go away with this code folding. But I'll leave it to the VM event code > maintainers to judge. > > Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > That comment seemed & still seems wrong to me, I don't see any code paths out of which that function would return -1. Thank you, Corneliu.
>>> On 10.02.16 at 17:37, <andrew.cooper3@citrix.com> wrote: > On 10/02/16 16:18, Jan Beulich wrote: >> >>> --- a/xen/include/asm-x86/hvm/event.h >>> +++ b/xen/include/asm-x86/hvm/event.h >>> @@ -17,6 +17,12 @@ >>> #ifndef __ASM_X86_HVM_EVENT_H__ >>> #define __ASM_X86_HVM_EVENT_H__ >>> >>> +enum hvm_event_breakpoint_type >>> +{ >>> + HVM_EVENT_SOFTWARE_BREAKPOINT, >>> + HVM_EVENT_SINGLESTEP_BREAKPOINT, >>> +}; >> I don't see what good it does to put existing constants into an >> enum. > > An enum was requested during review of v1, because "bool > software_breakpoint" isn't a good function interface. > > I don't mind if #defines were used instead, but the parameter should > have a semantic name used in calling code. To calling code there's no difference whether the parameter is of an enum type or just plain or unsigned int. Jan
>>> On 10.02.16 at 18:04, <czuzu@bitdefender.com> wrote: > On 2/10/2016 6:18 PM, Jan Beulich wrote: >>>>> On 10.02.16 at 16:50, <czuzu@bitdefender.com> wrote: >>> --- a/xen/include/asm-x86/hvm/event.h >>> +++ b/xen/include/asm-x86/hvm/event.h >>> @@ -17,6 +17,12 @@ >>> #ifndef __ASM_X86_HVM_EVENT_H__ >>> #define __ASM_X86_HVM_EVENT_H__ >>> >>> +enum hvm_event_breakpoint_type >>> +{ >>> + HVM_EVENT_SOFTWARE_BREAKPOINT, >>> + HVM_EVENT_SINGLESTEP_BREAKPOINT, >>> +}; >> I don't see what good it does to put existing constants into an >> enum. > As Andrew pointed out, an enum was requested in v1 instead of the > single_step param. > One could use the already existing VM_EVENT_REASON_* constants, but > conceptually this > function only involves a subset of those (i.e. *breakpoint vm-events*). Re-using existing constants would seem fine to me. I only now realize that I've made a mistake while looking at the above - the capitals made it implicitly "obvious" to me that they're on the right side of an assignment. Please use capitals only for #define-d constants, not enumerated ones. Jan
On 02/10/2016 07:04 PM, Corneliu ZUZU wrote: >>> @@ -27,9 +33,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_breakpoint(unsigned long rip, >>> + enum hvm_event_breakpoint_type type); >> I guess the comment was here for a reason, and this reason doesn't >> go away with this code folding. But I'll leave it to the VM event code >> maintainers to judge. >> >> Jan > > That comment seemed & still seems wrong to me, I don't see any code > paths out of which that function would return -1. That seems to be true. Those functions return whatever hvm_event_traps() returns, which is 0 on success, 1 (maybe the minus is a typo?) if there's no ring, or whatever value vm_event_claim_slot() returns. Vm_event_claim_slot()'s documentation says that it can only return 0 (on success), -ENOSYS or -EBUSY, none of which translate to -1 (and the code seems to agree with that claim). Maybe I'm missing some macro wizardry here, but I don't think so - it looks like the comment is stale. Tamas, maybe you remember more, should those functions return -1 if no listener is present? Thanks, Razvan
On Wed, Feb 10, 2016 at 10:28 AM, Razvan Cojocaru <rcojocaru@bitdefender.com > wrote: > On 02/10/2016 07:04 PM, Corneliu ZUZU wrote: > >>> @@ -27,9 +33,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_breakpoint(unsigned long rip, > >>> + enum hvm_event_breakpoint_type type); > >> I guess the comment was here for a reason, and this reason doesn't > >> go away with this code folding. But I'll leave it to the VM event code > >> maintainers to judge. > >> > >> Jan > > > > That comment seemed & still seems wrong to me, I don't see any code > > paths out of which that function would return -1. > > That seems to be true. Those functions return whatever hvm_event_traps() > returns, which is 0 on success, 1 (maybe the minus is a typo?) if > there's no ring, or whatever value vm_event_claim_slot() returns. > Vm_event_claim_slot()'s documentation says that it can only return 0 (on > success), -ENOSYS or -EBUSY, none of which translate to -1 (and the code > seems to agree with that claim). > > Maybe I'm missing some macro wizardry here, but I don't think so - it > looks like the comment is stale. Tamas, maybe you remember more, should > those functions return -1 if no listener is present? > It could very well be that it's just a comment that was forgotten and is out-of-date. I don't see any issue removing it if it's actually misleading (as it seems to be). Tamas
On 2/10/2016 7:11 PM, Jan Beulich wrote: >>>> On 10.02.16 at 18:04, <czuzu@bitdefender.com> wrote: >> On 2/10/2016 6:18 PM, Jan Beulich wrote: >>>>>> On 10.02.16 at 16:50, <czuzu@bitdefender.com> wrote: >>>> --- a/xen/include/asm-x86/hvm/event.h >>>> +++ b/xen/include/asm-x86/hvm/event.h >>>> @@ -17,6 +17,12 @@ >>>> #ifndef __ASM_X86_HVM_EVENT_H__ >>>> #define __ASM_X86_HVM_EVENT_H__ >>>> >>>> +enum hvm_event_breakpoint_type >>>> +{ >>>> + HVM_EVENT_SOFTWARE_BREAKPOINT, >>>> + HVM_EVENT_SINGLESTEP_BREAKPOINT, >>>> +}; >>> I don't see what good it does to put existing constants into an >>> enum. >> As Andrew pointed out, an enum was requested in v1 instead of the >> single_step param. >> One could use the already existing VM_EVENT_REASON_* constants, but >> conceptually this >> function only involves a subset of those (i.e. *breakpoint vm-events*). > Re-using existing constants would seem fine to me. Yes but then IMHO conceptually that would be wrong, since it would be like saying "that param can be any type of vm-event", even though it can actually be only from a subset of vm-event types and it will never be *any* type of vm-event. > I only now realize that I've made a mistake while looking at the > above - the capitals made it implicitly "obvious" to me that they're > on the right side of an assignment. Please use capitals only for > #define-d constants, not enumerated ones. > > Jan Most examples of existing enums I've looked at were capital-case, I thought they're supposed to be like that. Could you please provide an example on how enum values should look? (there isn't any in CODING_STYLE) Let me know if you still want me to change this and how. Corneliu.
On 10/02/2016 17:11, Jan Beulich wrote: >>>> On 10.02.16 at 18:04, <czuzu@bitdefender.com> wrote: >> On 2/10/2016 6:18 PM, Jan Beulich wrote: >>>>>> On 10.02.16 at 16:50, <czuzu@bitdefender.com> wrote: >>>> --- a/xen/include/asm-x86/hvm/event.h >>>> +++ b/xen/include/asm-x86/hvm/event.h >>>> @@ -17,6 +17,12 @@ >>>> #ifndef __ASM_X86_HVM_EVENT_H__ >>>> #define __ASM_X86_HVM_EVENT_H__ >>>> >>>> +enum hvm_event_breakpoint_type >>>> +{ >>>> + HVM_EVENT_SOFTWARE_BREAKPOINT, >>>> + HVM_EVENT_SINGLESTEP_BREAKPOINT, >>>> +}; >>> I don't see what good it does to put existing constants into an >>> enum. >> As Andrew pointed out, an enum was requested in v1 instead of the >> single_step param. >> One could use the already existing VM_EVENT_REASON_* constants, but >> conceptually this >> function only involves a subset of those (i.e. *breakpoint vm-events*). > Re-using existing constants would seem fine to me. > > I only now realize that I've made a mistake while looking at the > above - the capitals made it implicitly "obvious" to me that they're > on the right side of an assignment. Please use capitals only for > #define-d constants, not enumerated ones. Substantially more enums in the Xen codebase use caps than lowercase. Given no specific direction in CODING_STYLE, this is an unreasonable request. ~Andrew
On 2/10/2016 7:11 PM, Jan Beulich wrote: >>>> On 10.02.16 at 18:04, <czuzu@bitdefender.com> wrote: >> On 2/10/2016 6:18 PM, Jan Beulich wrote: >>>>>> On 10.02.16 at 16:50, <czuzu@bitdefender.com> wrote: >>>> --- a/xen/include/asm-x86/hvm/event.h >>>> +++ b/xen/include/asm-x86/hvm/event.h >>>> @@ -17,6 +17,12 @@ >>>> #ifndef __ASM_X86_HVM_EVENT_H__ >>>> #define __ASM_X86_HVM_EVENT_H__ >>>> >>>> +enum hvm_event_breakpoint_type >>>> +{ >>>> + HVM_EVENT_SOFTWARE_BREAKPOINT, >>>> + HVM_EVENT_SINGLESTEP_BREAKPOINT, >>>> +}; >>> I don't see what good it does to put existing constants into an >>> enum. >> As Andrew pointed out, an enum was requested in v1 instead of the >> single_step param. >> One could use the already existing VM_EVENT_REASON_* constants, but >> conceptually this >> function only involves a subset of those (i.e. *breakpoint vm-events*). > Re-using existing constants would seem fine to me. > > I only now realize that I've made a mistake while looking at the > above - the capitals made it implicitly "obvious" to me that they're > on the right side of an assignment. Please use capitals only for > #define-d constants, not enumerated ones. > > Jan > > Also, why this bias towards #define vs enums? I usually always prefer the latter, they make the code more readable, e.g. it's clearer what can or cannot be passed to a corresponding function parameter. And from an IDE user's point-of-view, I don't understand why I have to read a comment and search for a list of identifiers rather than just clicking on a type name and getting there in a flash. They also make debugging easier, etc.. :). Corneliu.
>>> On 10.02.16 at 21:56, <andrew.cooper3@citrix.com> wrote: > On 10/02/2016 17:11, Jan Beulich wrote: >>>>> On 10.02.16 at 18:04, <czuzu@bitdefender.com> wrote: >>> On 2/10/2016 6:18 PM, Jan Beulich wrote: >>>>>>> On 10.02.16 at 16:50, <czuzu@bitdefender.com> wrote: >>>>> --- a/xen/include/asm-x86/hvm/event.h >>>>> +++ b/xen/include/asm-x86/hvm/event.h >>>>> @@ -17,6 +17,12 @@ >>>>> #ifndef __ASM_X86_HVM_EVENT_H__ >>>>> #define __ASM_X86_HVM_EVENT_H__ >>>>> >>>>> +enum hvm_event_breakpoint_type >>>>> +{ >>>>> + HVM_EVENT_SOFTWARE_BREAKPOINT, >>>>> + HVM_EVENT_SINGLESTEP_BREAKPOINT, >>>>> +}; >>>> I don't see what good it does to put existing constants into an >>>> enum. >>> As Andrew pointed out, an enum was requested in v1 instead of the >>> single_step param. >>> One could use the already existing VM_EVENT_REASON_* constants, but >>> conceptually this >>> function only involves a subset of those (i.e. *breakpoint vm-events*). >> Re-using existing constants would seem fine to me. >> >> I only now realize that I've made a mistake while looking at the >> above - the capitals made it implicitly "obvious" to me that they're >> on the right side of an assignment. Please use capitals only for >> #define-d constants, not enumerated ones. > > Substantially more enums in the Xen codebase use caps than lowercase. Well, sadly this indeed seems to be the case. > Given no specific direction in CODING_STYLE, this is an unreasonable > request. Hence I withdraw the request, despite continuing to be convinced that all-caps enumerators are bad practice. Jan
diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c index a3d4892..e3444db 100644 --- a/xen/arch/x86/hvm/event.c +++ b/xen/arch/x86/hvm/event.c @@ -1,9 +1,12 @@ /* -* event.c: Common hardware virtual machine event abstractions. +* arch/x86/hvm/event.c +* +* Arch-specific hardware virtual machine event abstractions. * * Copyright (c) 2004, Intel Corporation. * Copyright (c) 2005, International Business Machines Corporation. * Copyright (c) 2008, Citrix Systems, Inc. +* Copyright (c) 2016, Bitdefender S.R.L. * * This program is free software; you can redistribute it and/or modify it * under the terms and conditions of the GNU General Public License, @@ -151,61 +154,52 @@ void hvm_event_guest_request(void) } } -int hvm_event_int3(unsigned long rip) +static inline +uint64_t gfn_of_rip(unsigned long rip) { - int rc = 0; struct vcpu *curr = current; + struct segment_register sreg; + uint32_t pfec = PFEC_page_present | PFEC_insn_fetch; - if ( curr->domain->arch.monitor.software_breakpoint_enabled ) - { - struct segment_register sreg; - uint32_t pfec = PFEC_page_present | PFEC_insn_fetch; - vm_event_request_t req = { - .reason = VM_EVENT_REASON_SOFTWARE_BREAKPOINT, - .vcpu_id = curr->vcpu_id, - }; - - hvm_get_segment_register(curr, x86_seg_ss, &sreg); - if ( sreg.attr.fields.dpl == 3 ) - pfec |= PFEC_user_mode; + hvm_get_segment_register(curr, x86_seg_ss, &sreg); + if ( sreg.attr.fields.dpl == 3 ) + pfec |= PFEC_user_mode; - hvm_get_segment_register(curr, x86_seg_cs, &sreg); - req.u.software_breakpoint.gfn = paging_gva_to_gfn(curr, - sreg.base + rip, - &pfec); - - rc = hvm_event_traps(1, &req); - } + hvm_get_segment_register(curr, x86_seg_cs, &sreg); - return rc; + return (uint64_t) paging_gva_to_gfn(curr, sreg.base + rip, &pfec); } -int hvm_event_single_step(unsigned long rip) +int hvm_event_breakpoint(unsigned long rip, + enum hvm_event_breakpoint_type type) { - int rc = 0; struct vcpu *curr = current; + struct arch_domain *ad = &curr->domain->arch; + vm_event_request_t req; - if ( curr->domain->arch.monitor.singlestep_enabled ) + switch ( type ) { - struct segment_register sreg; - uint32_t pfec = PFEC_page_present | PFEC_insn_fetch; - vm_event_request_t req = { - .reason = VM_EVENT_REASON_SINGLESTEP, - .vcpu_id = curr->vcpu_id, - }; - - hvm_get_segment_register(curr, x86_seg_ss, &sreg); - if ( sreg.attr.fields.dpl == 3 ) - pfec |= PFEC_user_mode; + case HVM_EVENT_SOFTWARE_BREAKPOINT: + if ( !ad->monitor.software_breakpoint_enabled ) + return 0; + req.reason = VM_EVENT_REASON_SOFTWARE_BREAKPOINT; + req.u.software_breakpoint.gfn = gfn_of_rip(rip); + break; - hvm_get_segment_register(curr, x86_seg_cs, &sreg); - req.u.singlestep.gfn = paging_gva_to_gfn(curr, sreg.base + rip, - &pfec); + case HVM_EVENT_SINGLESTEP_BREAKPOINT: + if ( !ad->monitor.singlestep_enabled ) + return 0; + req.reason = VM_EVENT_REASON_SINGLESTEP; + req.u.singlestep.gfn = gfn_of_rip(rip); + break; - rc = hvm_event_traps(1, &req); + default: + return -EOPNOTSUPP; } - return rc; + req.vcpu_id = curr->vcpu_id; + + return hvm_event_traps(1, &req); } /* diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index b9f340c..cf0e642 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -3192,8 +3192,10 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) break; } else { - int handled = hvm_event_int3(regs->eip); - + int handled = + hvm_event_breakpoint(regs->eip, + HVM_EVENT_SOFTWARE_BREAKPOINT); + if ( handled < 0 ) { struct hvm_trap trap = { @@ -3517,10 +3519,11 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) case EXIT_REASON_MONITOR_TRAP_FLAG: v->arch.hvm_vmx.exec_control &= ~CPU_BASED_MONITOR_TRAP_FLAG; vmx_update_cpu_exec_control(v); - if ( v->arch.hvm_vcpu.single_step ) { - hvm_event_single_step(regs->eip); - if ( v->domain->debugger_attached ) - domain_pause_for_debugger(); + if ( v->arch.hvm_vcpu.single_step ) + { + hvm_event_breakpoint(regs->eip, HVM_EVENT_SINGLESTEP_BREAKPOINT); + if ( v->domain->debugger_attached ) + domain_pause_for_debugger(); } break; diff --git a/xen/include/asm-x86/hvm/event.h b/xen/include/asm-x86/hvm/event.h index 11eb1fe..7a83b45 100644 --- a/xen/include/asm-x86/hvm/event.h +++ b/xen/include/asm-x86/hvm/event.h @@ -17,6 +17,12 @@ #ifndef __ASM_X86_HVM_EVENT_H__ #define __ASM_X86_HVM_EVENT_H__ +enum hvm_event_breakpoint_type +{ + HVM_EVENT_SOFTWARE_BREAKPOINT, + HVM_EVENT_SINGLESTEP_BREAKPOINT, +}; + /* * Called for current VCPU on crX/MSR changes by guest. * The event might not fire if the client has subscribed to it in onchangeonly @@ -27,9 +33,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_breakpoint(unsigned long rip, + enum hvm_event_breakpoint_type type); void hvm_event_guest_request(void); #endif /* __ASM_X86_HVM_EVENT_H__ */
This patch merges almost identical functions hvm_event_int3 and hvm_event_single_step into a single function called hvm_event_breakpoint. Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com> --- xen/arch/x86/hvm/event.c | 76 +++++++++++++++++++---------------------- xen/arch/x86/hvm/vmx/vmx.c | 15 ++++---- xen/include/asm-x86/hvm/event.h | 11 ++++-- 3 files changed, 52 insertions(+), 50 deletions(-)