Re: [Xen-devel] [PATCH RFC 2/9] xen: Optimize introspection access to guest state

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.07.14 at 15:33, <rcojocaru@xxxxxxxxxxxxxxx> 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.

First of all I wonder whether all of the interface changes are really
permissible compatibility-wise.

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -6016,6 +6016,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)
> +{
> +    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];
> +}

This fills far not as many fields as the p2m function further down.
Why?

> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -425,6 +425,7 @@ static void vmx_vmcs_save(struct vcpu *v, struct 
> hvm_hw_cpu *c)
>      c->cr4 = v->arch.hvm_vcpu.guest_cr[4];
>  
>      c->msr_efer = v->arch.hvm_vcpu.guest_efer;
> +    c->guest_x86_mode = vmx_guest_x86_mode(v);

This seems unrelated and/or lacking an SVM counterpart.

> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1314,6 +1314,64 @@ 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));
> +
> +    /* 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;
> +
> +#ifdef __x86_64__

You don't need this anymore.

> +    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;
> +#endif
> +
> +    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;

These two segment bases may be sufficient to describe x86-64 state,
but what about a guest in 16- or 32-bit mode?

Jan

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