[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC V2 2/6] xen: Optimize introspection access to guest state
On 11/07/14 16:43, Razvan Cojocaru wrote: > Speed optimization for introspection purposes: a handful of registers > are sent along with each mem_event. This requires enlargement of the > mem_event_request / mem_event_response stuctures, and additional code > to fill in relevant values. Since the EPT event processing code needs > more data than CR3 or MSR event processors, hvm_mem_event_fill_regs() > fills in less data than p2m_mem_event_fill_regs(), in order to avoid > overhead. Struct hvm_hw_cpu has been considered instead of the custom > struct mem_event_regs_st, but its size would cause quick filling up > of the mem_event ring buffer. > > Changes since V1: > - Replaced guest_x86_mode with cs_arbytes in the mem_event_regs_st > structure. > - Removed superfluous preprocessor check for __x86_64__ in > p2m_mem_event_fill_regs(). > > Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx> > --- > xen/arch/x86/hvm/hvm.c | 33 ++++++++++++++++++++++ > xen/arch/x86/mm/p2m.c | 60 > ++++++++++++++++++++++++++++++++++++++++ > xen/include/public/mem_event.h | 38 +++++++++++++++++++++++++ > 3 files changed, 131 insertions(+) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index ef2411c..89a0382 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -6085,6 +6085,38 @@ int hvm_debug_op(struct vcpu *v, int32_t op) > return rc; > } > > +static inline void hvm_mem_event_fill_regs(mem_event_request_t *req) For C functions like this, the compiler is far better at guessing the inlineabilty of the function than the programmer. You don't need to call it out explicitly. > +{ > + struct cpu_user_regs *regs = guest_cpu_user_regs(); > + struct vcpu *v = current; > + > + req->regs.rax = regs->eax; > + req->regs.rcx = regs->ecx; > + req->regs.rdx = regs->edx; > + req->regs.rbx = regs->ebx; > + req->regs.rsp = regs->esp; > + req->regs.rbp = regs->ebp; > + req->regs.rsi = regs->esi; > + req->regs.rdi = regs->edi; > + > + req->regs.r8 = regs->r8; > + req->regs.r9 = regs->r9; > + req->regs.r10 = regs->r10; > + req->regs.r11 = regs->r11; > + req->regs.r12 = regs->r12; > + req->regs.r13 = regs->r13; > + req->regs.r14 = regs->r14; > + req->regs.r15 = regs->r15; > + > + req->regs.rflags = regs->eflags; > + req->regs.rip = regs->eip; > + > + req->regs.msr_efer = v->arch.hvm_vcpu.guest_efer; > + req->regs.cr0 = v->arch.hvm_vcpu.guest_cr[0]; > + req->regs.cr3 = v->arch.hvm_vcpu.guest_cr[3]; > + req->regs.cr4 = v->arch.hvm_vcpu.guest_cr[4]; > +} > + > static int hvm_memory_event_traps(long p, uint32_t reason, > unsigned long value, unsigned long old, > bool_t gla_valid, unsigned long gla) > @@ -6129,6 +6161,7 @@ static int hvm_memory_event_traps(long p, uint32_t > reason, > req.gla = old; > } > > + hvm_mem_event_fill_regs(&req); > mem_event_put_request(d, &d->mem_event->access, &req); > > return 1; > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c > index 642ec28..13fdf78 100644 > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -1314,6 +1314,63 @@ void p2m_mem_paging_resume(struct domain *d) > } > } > > +static inline void p2m_mem_event_fill_regs(mem_event_request_t *req) > +{ > + struct cpu_user_regs *regs = guest_cpu_user_regs(); > + struct segment_register seg; > + struct hvm_hw_cpu ctxt; > + struct vcpu *v = current; > + > + memset(&ctxt, 0, sizeof(struct hvm_hw_cpu)); You don't need to zero ctxt if you are about to save into it. > + > + /* Architecture-specific vmcs/vmcb bits */ > + hvm_funcs.save_cpu_ctxt(v, &ctxt); > + > + req->regs.rax = regs->eax; > + req->regs.rcx = regs->ecx; > + req->regs.rdx = regs->edx; > + req->regs.rbx = regs->ebx; > + req->regs.rsp = regs->esp; > + req->regs.rbp = regs->ebp; > + req->regs.rsi = regs->esi; > + req->regs.rdi = regs->edi; > + > + req->regs.r8 = regs->r8; > + req->regs.r9 = regs->r9; > + req->regs.r10 = regs->r10; > + req->regs.r11 = regs->r11; > + req->regs.r12 = regs->r12; > + req->regs.r13 = regs->r13; > + req->regs.r14 = regs->r14; > + req->regs.r15 = regs->r15; > + > + req->regs.rflags = regs->eflags; > + req->regs.rip = regs->eip; > + > + req->regs.dr7 = v->arch.debugreg[7]; > + req->regs.cr0 = ctxt.cr0; > + req->regs.cr2 = ctxt.cr2; > + req->regs.cr3 = ctxt.cr3; > + req->regs.cr4 = ctxt.cr4; > + > + req->regs.sysenter_cs = ctxt.sysenter_cs; > + req->regs.sysenter_esp = ctxt.sysenter_esp; > + req->regs.sysenter_eip = ctxt.sysenter_eip; > + > + req->regs.msr_efer = ctxt.msr_efer; > + req->regs.msr_star = ctxt.msr_star; > + req->regs.msr_lstar = ctxt.msr_lstar; > + > + hvm_get_segment_register(v, x86_seg_fs, &seg); > + req->regs.fs_base = seg.base; > + > + hvm_get_segment_register(v, x86_seg_gs, &seg); > + req->regs.gs_base = seg.base; > + > + hvm_get_segment_register(v, x86_seg_cs, &seg); > + req->regs.cs_arbytes = seg.attr.bytes;; Stray double semicolon. > +} > + > bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long > gla, > bool_t access_r, bool_t access_w, bool_t access_x, > mem_event_request_t **req_ptr) > @@ -1407,6 +1464,9 @@ bool_t p2m_mem_access_check(paddr_t gpa, bool_t > gla_valid, unsigned long gla, > if ( p2ma != p2m_access_n2rwx ) > vcpu_pause_nosync(v); > > + if ( req ) > + p2m_mem_event_fill_regs(req); Should this not be part of the if ( req ) just above the pause_nosync() ? > + > /* VCPU may be paused, return whether we promoted automatically */ > return (p2ma == p2m_access_n2rwx); > } > diff --git a/xen/include/public/mem_event.h b/xen/include/public/mem_event.h > index 3831b41..b9af728 100644 > --- a/xen/include/public/mem_event.h > +++ b/xen/include/public/mem_event.h > @@ -48,6 +48,43 @@ > #define MEM_EVENT_REASON_MSR 7 /* MSR was hit: gfn is MSR value, > gla is MSR address; > does NOT honour > HVMPME_onchangeonly */ > > +/* Using a custom struct (not hvm_hw_cpu) so as to not fill > + * the mem_event ring buffer too quickly. */ > +typedef struct mem_event_regs_st { > + uint64_t rax; > + uint64_t rcx; > + uint64_t rdx; > + uint64_t rbx; > + uint64_t rsp; > + uint64_t rbp; > + uint64_t rsi; > + uint64_t rdi; > + uint64_t r8; > + uint64_t r9; > + uint64_t r10; > + uint64_t r11; > + uint64_t r12; > + uint64_t r13; > + uint64_t r14; > + uint64_t r15; > + uint64_t rflags; > + uint64_t dr7; > + uint64_t rip; > + uint64_t cr0; > + uint64_t cr2; > + uint64_t cr3; > + uint64_t cr4; > + uint64_t sysenter_cs; > + uint64_t sysenter_esp; > + uint64_t sysenter_eip; > + uint64_t msr_efer; > + uint64_t msr_star; > + uint64_t msr_lstar; > + uint64_t fs_base; > + uint64_t gs_base; > + uint32_t cs_arbytes; This trailing uint32_t means that sizeof(mem_event_regs_t) is different between 32 and 64 bit builds, breaking compatibility between a 64bit xen and 32bit dom0. The easiest fix is to add an explicit uint32_t _pad field at the end. ~Andrew > +} mem_event_regs_t; > + > typedef struct mem_event_st { > uint32_t flags; > uint32_t vcpu_id; > @@ -65,6 +102,7 @@ typedef struct mem_event_st { > uint16_t available:12; > > uint16_t reason; > + mem_event_regs_t regs; > } mem_event_request_t, mem_event_response_t; > > DEFINE_RING_TYPES(mem_event, mem_event_request_t, mem_event_response_t); _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |