| Message ID | 96a1376e-9464-f797-30fd-f6923efbf6c7@list.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN,v1,1/1] x86/vm_event: add fast single step | expand |
On 17/12/2019 14:40, Sergey Kovalev wrote: > On break point event eight context switches occures. > > With fast single step it is possible to shorten path for two context > switches > and gain 35% spead-up. > > Was tested on Debian branch of Xen 4.12. See at: > https://github.com/skvl/xen/tree/debian/knorrie/4.12/fast-singlestep > > Rebased on master: > https://github.com/skvl/xen/tree/fast-singlestep > > Signed-off-by: Sergey Kovalev <valor@list.ru> 35% looks like a good number, but what is "fast single step"? All this appears to be is plumbing for to cause an altp2m switch on single step. ~Andrew
On 17.12.2019 17:48, Andrew Cooper wrote: > On 17/12/2019 14:40, Sergey Kovalev wrote: >> On break point event eight context switches occures. >> >> With fast single step it is possible to shorten path for two context >> switches >> and gain 35% spead-up. >> >> Was tested on Debian branch of Xen 4.12. See at: >> https://github.com/skvl/xen/tree/debian/knorrie/4.12/fast-singlestep >> >> Rebased on master: >> https://github.com/skvl/xen/tree/fast-singlestep >> >> Signed-off-by: Sergey Kovalev <valor@list.ru> > > 35% looks like a good number, but what is "fast single step"? All this > appears to be is plumbing for to cause an altp2m switch on single step. > > ~Andrew > You are right. I should quoted "fast single step". Original INT#3 path is like this (in PlangUML): @startuml VM->Xen : EXIT_REASON_EXCEPTION_NMI Xen->LibVMI: request(VM_EVENT_REASON_SOFTWARE_BREAKPOINT) LibVMI->Xen: response(singlestep | altp2m) Xen->VM: VM->Xen: EXIT_REASON_MONITOR_TRAP_FLAG Xen->LibVMI: request(VM_EVENT_REASON_SINGLESTEP) LibVMI->Xen: response(altp2m) Xen->VM: @enduml With fast single step it looks like this: @startuml VM->Xen : EXIT_REASON_EXCEPTION_NMI Xen->LibVMI: request(VM_EVENT_REASON_SOFTWARE_BREAKPOINT) LibVMI->Xen: response(fast singlestep | altp2m) Xen->VM: VM->Xen: EXIT_REASON_MONITOR_TRAP_FLAG Xen->Xen: fast singlestep Xen->VM: @enduml So we just store altp2m index and switch to it on MTF.
On Tue, Dec 17, 2019 at 7:48 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 17/12/2019 14:40, Sergey Kovalev wrote: > > On break point event eight context switches occures. > > > > With fast single step it is possible to shorten path for two context > > switches > > and gain 35% spead-up. > > > > Was tested on Debian branch of Xen 4.12. See at: > > https://github.com/skvl/xen/tree/debian/knorrie/4.12/fast-singlestep > > > > Rebased on master: > > https://github.com/skvl/xen/tree/fast-singlestep > > > > Signed-off-by: Sergey Kovalev <valor@list.ru> > > 35% looks like a good number, but what is "fast single step"? All this > appears to be is plumbing for to cause an altp2m switch on single step. Yes, a better explanation would be much needed here and I'm not 100% sure it correctly implements what I think it tries to. This is my interpretation of what the idea is: when using DRAKVUF (or another system using altp2m with shadow pages similar to what I describe in https://xenproject.org/2016/04/13/stealthy-monitoring-with-xen-altp2m), after a breakpoint is hit the system switches to the default unrestricted altp2m view with singlestep enabled. When the singlestep traps to Xen another vm_event is sent to the monitor agent, which then normally disables singlestepping and switches the altp2m view back to the restricted view. This patch looks like its short-circuiting that last part so that it doesn't need to send the vm_event out for the singlestep event and should switch back to the restricted view in Xen automatically. It's a nice optimization. But what seems to be missing is the altp2m switch itself. Tamas
On Tue, Dec 17, 2019 at 8:08 AM Tamas K Lengyel <tamas@tklengyel.com> wrote: > > On Tue, Dec 17, 2019 at 7:48 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > > > On 17/12/2019 14:40, Sergey Kovalev wrote: > > > On break point event eight context switches occures. > > > > > > With fast single step it is possible to shorten path for two context > > > switches > > > and gain 35% spead-up. > > > > > > Was tested on Debian branch of Xen 4.12. See at: > > > https://github.com/skvl/xen/tree/debian/knorrie/4.12/fast-singlestep > > > > > > Rebased on master: > > > https://github.com/skvl/xen/tree/fast-singlestep > > > > > > Signed-off-by: Sergey Kovalev <valor@list.ru> > > > > 35% looks like a good number, but what is "fast single step"? All this > > appears to be is plumbing for to cause an altp2m switch on single step. > > Yes, a better explanation would be much needed here and I'm not 100% > sure it correctly implements what I think it tries to. > > This is my interpretation of what the idea is: when using DRAKVUF (or > another system using altp2m with shadow pages similar to what I > describe in https://xenproject.org/2016/04/13/stealthy-monitoring-with-xen-altp2m), > after a breakpoint is hit the system switches to the default > unrestricted altp2m view with singlestep enabled. When the singlestep > traps to Xen another vm_event is sent to the monitor agent, which then > normally disables singlestepping and switches the altp2m view back to > the restricted view. This patch looks like its short-circuiting that > last part so that it doesn't need to send the vm_event out for the > singlestep event and should switch back to the restricted view in Xen > automatically. It's a nice optimization. But what seems to be missing > is the altp2m switch itself. Never mind, p2m_altp2m_check does the altp2m switch as well, so this patch implements what I described above. Please update the patch message to be more descriptive (you can copy my description from above). Thanks! Tamas
On 17/12/2019 15:10, Tamas K Lengyel wrote: > On Tue, Dec 17, 2019 at 8:08 AM Tamas K Lengyel <tamas@tklengyel.com> wrote: >> On Tue, Dec 17, 2019 at 7:48 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote: >>> On 17/12/2019 14:40, Sergey Kovalev wrote: >>>> On break point event eight context switches occures. >>>> >>>> With fast single step it is possible to shorten path for two context >>>> switches >>>> and gain 35% spead-up. >>>> >>>> Was tested on Debian branch of Xen 4.12. See at: >>>> https://github.com/skvl/xen/tree/debian/knorrie/4.12/fast-singlestep >>>> >>>> Rebased on master: >>>> https://github.com/skvl/xen/tree/fast-singlestep >>>> >>>> Signed-off-by: Sergey Kovalev <valor@list.ru> >>> 35% looks like a good number, but what is "fast single step"? All this >>> appears to be is plumbing for to cause an altp2m switch on single step. >> Yes, a better explanation would be much needed here and I'm not 100% >> sure it correctly implements what I think it tries to. >> >> This is my interpretation of what the idea is: when using DRAKVUF (or >> another system using altp2m with shadow pages similar to what I >> describe in https://xenproject.org/2016/04/13/stealthy-monitoring-with-xen-altp2m), >> after a breakpoint is hit the system switches to the default >> unrestricted altp2m view with singlestep enabled. When the singlestep >> traps to Xen another vm_event is sent to the monitor agent, which then >> normally disables singlestepping and switches the altp2m view back to >> the restricted view. This patch looks like its short-circuiting that >> last part so that it doesn't need to send the vm_event out for the >> singlestep event and should switch back to the restricted view in Xen >> automatically. It's a nice optimization. But what seems to be missing >> is the altp2m switch itself. > Never mind, p2m_altp2m_check does the altp2m switch as well, so this > patch implements what I described above. Please update the patch > message to be more descriptive (you can copy my description from > above). Also please read CODING_STYLE in the root of the xen repository. The important ones you need to fix are spaces in "if ( ... )" statements, and binary operators on the end of the first line rather than the beginning of the continuation. ~Andrew
Andrew, Tamas thank you very much. I will improve the patch. December 17, 2019 3:13:42 PM UTC, Andrew Cooper <andrew.cooper3@citrix.com> пишет: >On 17/12/2019 15:10, Tamas K Lengyel wrote: >> On Tue, Dec 17, 2019 at 8:08 AM Tamas K Lengyel <tamas@tklengyel.com> >wrote: >>> On Tue, Dec 17, 2019 at 7:48 AM Andrew Cooper ><andrew.cooper3@citrix.com> wrote: >>>> On 17/12/2019 14:40, Sergey Kovalev wrote: >>>>> On break point event eight context switches occures. >>>>> >>>>> With fast single step it is possible to shorten path for two >context >>>>> switches >>>>> and gain 35% spead-up. >>>>> >>>>> Was tested on Debian branch of Xen 4.12. See at: >>>>> >https://github.com/skvl/xen/tree/debian/knorrie/4.12/fast-singlestep >>>>> >>>>> Rebased on master: >>>>> https://github.com/skvl/xen/tree/fast-singlestep >>>>> >>>>> Signed-off-by: Sergey Kovalev <valor@list.ru> >>>> 35% looks like a good number, but what is "fast single step"? All >this >>>> appears to be is plumbing for to cause an altp2m switch on single >step. >>> Yes, a better explanation would be much needed here and I'm not 100% >>> sure it correctly implements what I think it tries to. >>> >>> This is my interpretation of what the idea is: when using DRAKVUF >(or >>> another system using altp2m with shadow pages similar to what I >>> describe in >https://xenproject.org/2016/04/13/stealthy-monitoring-with-xen-altp2m), >>> after a breakpoint is hit the system switches to the default >>> unrestricted altp2m view with singlestep enabled. When the >singlestep >>> traps to Xen another vm_event is sent to the monitor agent, which >then >>> normally disables singlestepping and switches the altp2m view back >to >>> the restricted view. This patch looks like its short-circuiting that >>> last part so that it doesn't need to send the vm_event out for the >>> singlestep event and should switch back to the restricted view in >Xen >>> automatically. It's a nice optimization. But what seems to be >missing >>> is the altp2m switch itself. >> Never mind, p2m_altp2m_check does the altp2m switch as well, so this >> patch implements what I described above. Please update the patch >> message to be more descriptive (you can copy my description from >> above). > >Also please read CODING_STYLE in the root of the xen repository. The >important ones you need to fix are spaces in "if ( ... )" statements, >and binary operators on the end of the first line rather than the >beginning of the continuation. > >~Andrew
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 47573f71b8..4999569503 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -5126,6 +5126,18 @@ void hvm_toggle_singlestep(struct vcpu *v) v->arch.hvm.single_step = !v->arch.hvm.single_step; } +void hvm_fast_singlestep(struct vcpu *v, uint16_t p2midx) +{ + ASSERT(atomic_read(&v->pause_count)); + + if ( !hvm_is_singlestep_supported() ) + return; + + v->arch.hvm.single_step = true; + v->arch.hvm.fast_single_step.enabled = true; + v->arch.hvm.fast_single_step.p2midx = p2midx; +} + /* * Segment caches in VMCB/VMCS are inconsistent about which bits are checked, * important, and preserved across vmentry/exit. Cook the values to make them diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c index 1f23fe25e8..8c05e33922 100644 --- a/xen/arch/x86/hvm/monitor.c +++ b/xen/arch/x86/hvm/monitor.c @@ -28,6 +28,7 @@ #include <asm/hvm/monitor.h> #include <asm/altp2m.h> #include <asm/monitor.h> +#include <asm/p2m.h> #include <asm/paging.h> #include <asm/vm_event.h> #include <public/vm_event.h> @@ -159,6 +160,14 @@ int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type, case HVM_MONITOR_SINGLESTEP_BREAKPOINT: if ( !ad->monitor.singlestep_enabled ) return 0; + if (curr->arch.hvm.fast_single_step.enabled) + { + p2m_altp2m_check(curr, curr->arch.hvm.fast_single_step.p2midx); + curr->arch.hvm.single_step = false; + curr->arch.hvm.fast_single_step.enabled = false; + curr->arch.hvm.fast_single_step.p2midx = 0; + return 0; + } req.reason = VM_EVENT_REASON_SINGLESTEP; req.u.singlestep.gfn = gfn_of_rip(rip); sync = true; diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c index 52c2a71fa0..385116b5f2 100644 --- a/xen/arch/x86/vm_event.c +++ b/xen/arch/x86/vm_event.c @@ -61,7 +61,8 @@ void vm_event_cleanup_domain(struct domain *d) void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v, vm_event_response_t *rsp) { - if ( !(rsp->flags & VM_EVENT_FLAG_TOGGLE_SINGLESTEP) ) + if ( !(rsp->flags & VM_EVENT_FLAG_TOGGLE_SINGLESTEP + || rsp->flags & VM_EVENT_FLAG_FAST_SINGLESTEP) ) return; if ( !is_hvm_domain(d) ) @@ -69,7 +70,10 @@ void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v, ASSERT(atomic_read(&v->vm_event_pause_count)); - hvm_toggle_singlestep(v); + if (rsp->flags & VM_EVENT_FLAG_TOGGLE_SINGLESTEP) + hvm_toggle_singlestep(v); + else + hvm_fast_singlestep(v, rsp->u.fast_singlestep.p2midx); } void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp) diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h index 1d7b66f927..09793c12e9 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -323,6 +323,7 @@ int hvm_debug_op(struct vcpu *v, int32_t op); /* Caller should pause vcpu before calling this function */ void hvm_toggle_singlestep(struct vcpu *v); +void hvm_fast_singlestep(struct vcpu *v, uint16_t p2midx); int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla, struct npfec npfec); diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h index 38f5c2bb9b..8b84941111 100644 --- a/xen/include/asm-x86/hvm/vcpu.h +++ b/xen/include/asm-x86/hvm/vcpu.h @@ -172,6 +172,10 @@ struct hvm_vcpu { bool flag_dr_dirty; bool debug_state_latch; bool single_step; + struct { + bool enabled; + uint16_t p2midx; + } fast_single_step; struct hvm_vcpu_asid n1asid; diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h index aa54c86325..cb577a7ba9 100644 --- a/xen/include/public/vm_event.h +++ b/xen/include/public/vm_event.h @@ -110,6 +110,11 @@ * interrupt pending after resuming the VCPU. */ #define VM_EVENT_FLAG_GET_NEXT_INTERRUPT (1 << 10) +/* + * Execute fast singlestepping on vm_event response. + * Requires the vCPU to be paused already (synchronous events only). + */ +#define VM_EVENT_FLAG_FAST_SINGLESTEP (1 << 11) /* * Reasons for the vm event request @@ -276,6 +281,10 @@ struct vm_event_singlestep { uint64_t gfn; }; +struct vm_event_fast_singlestep { + uint16_t p2midx; +}; + struct vm_event_debug { uint64_t gfn; uint32_t insn_length; @@ -363,6 +372,7 @@ typedef struct vm_event_st { struct vm_event_mov_to_msr mov_to_msr; struct vm_event_desc_access desc_access; struct vm_event_singlestep singlestep; + struct vm_event_fast_singlestep fast_singlestep; struct vm_event_debug software_breakpoint; struct vm_event_debug debug_exception; struct vm_event_cpuid cpuid;
On break point event eight context switches occures. With fast single step it is possible to shorten path for two context switches and gain 35% spead-up. Was tested on Debian branch of Xen 4.12. See at: https://github.com/skvl/xen/tree/debian/knorrie/4.12/fast-singlestep Rebased on master: https://github.com/skvl/xen/tree/fast-singlestep Signed-off-by: Sergey Kovalev <valor@list.ru> --- xen/arch/x86/hvm/hvm.c | 12 ++++++++++++ xen/arch/x86/hvm/monitor.c | 9 +++++++++ xen/arch/x86/vm_event.c | 8 ++++++-- xen/include/asm-x86/hvm/hvm.h | 1 + xen/include/asm-x86/hvm/vcpu.h | 4 ++++ xen/include/public/vm_event.h | 10 ++++++++++ 6 files changed, 42 insertions(+), 2 deletions(-)