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

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 02/07/14 14:33, 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.
>
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

The public API already has struct hvm_hw_cpu in
xen/include/public/arch-x86/hvm/save.h

It might be better to reuse that rather than defining a new structure to
contain a subset of the information.

> ---
>  xen/arch/x86/hvm/hvm.c                 |   33 +++++++++++++++++
>  xen/arch/x86/hvm/vmx/vmx.c             |    1 +
>  xen/arch/x86/mm/p2m.c                  |   61 ++++++++++++++++++++++++++++++++
>  xen/include/public/arch-x86/hvm/save.h |    4 +++
>  xen/include/public/mem_event.h         |   36 +++++++++++++++++++
>  5 files changed, 135 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 17ff011..f65a5f5 100644
> --- 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];
> +}
> +
>  static int hvm_memory_event_traps(long p, uint32_t reason,
>                                    unsigned long value, unsigned long old, 
>                                    bool_t gla_valid, unsigned long gla) 
> @@ -6060,6 +6092,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/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 2caa04a..fed21b6 100644
> --- 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);

guest_x86_mode is a linear function of cr0, eflags and efer.  It can be
calculated by userspace doesn't need to transmitted individually.

>  
>      __vmread(GUEST_SYSENTER_CS, &c->sysenter_cs);
>      __vmread(GUEST_SYSENTER_ESP, &c->sysenter_esp);
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 642ec28..93252d9 100644
> --- 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__

There is no need to code for __i386__ inside xen/arch/x86

~Andrew