Re: [Xen-devel] [PATCH 1/2] trace: improve usefulness of hypercall trace record

[Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>]

On Thu, 2012-05-24 at 11:37 +0100, David Vrabel wrote:
> From: David Vrabel <david.vrabel@xxxxxxxxxx>
> 
> Trace hypercalls using a more useful trace record format.
> 
> The EIP field is removed (it was always somewhere in the hypercall
> page) and include selected hypercall arguments (the number of calls in
> a multicall, and the number of PTE updates in an mmu_update).
> 

I think that EIP is quite useful as it allow to understand which code in
dom0 call that hypercall.

There is also space for an additional parameter without changing trace
version (adding information in a record should not be a problem).

Frediano

> To allow tracing tools to distinguish between the two formats, the new
> format uses a new event ID (TRC_PV_HYPERCALL_V2).
> 
> Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
> ---
>  xen/arch/x86/trace.c       |   33 +++++++++++++--------------------
>  xen/common/trace.c         |   23 +++++++++++++++++++++++
>  xen/include/public/trace.h |    1 +
>  xen/include/xen/trace.h    |    2 ++
>  4 files changed, 39 insertions(+), 20 deletions(-)
> 
> diff --git a/xen/arch/x86/trace.c b/xen/arch/x86/trace.c
> index 404f293..2b59017 100644
> --- a/xen/arch/x86/trace.c
> +++ b/xen/arch/x86/trace.c
> @@ -14,35 +14,28 @@
>  void trace_hypercall(void)
>  {
>      struct cpu_user_regs *regs = guest_cpu_user_regs();
> +    unsigned long args[5];
>  
>  #ifdef __x86_64__
>      if ( is_pv_32on64_vcpu(current) )
>      {
> -        struct {
> -            u32 eip,eax;
> -        } __attribute__((packed)) d;
> -            
> -        d.eip = regs->eip;
> -        d.eax = regs->eax;
> -
> -        __trace_var(TRC_PV_HYPERCALL, 1, sizeof(d), &d);
> +        args[0] = regs->ebx;
> +        args[1] = regs->ecx;
> +        args[2] = regs->edx;
> +        args[3] = regs->esi;
> +        args[4] = regs->edi;
>      }
>      else
>  #endif
>      {
> -        struct {
> -            unsigned long eip;
> -            u32 eax;
> -        } __attribute__((packed)) d;
> -        u32 event;
> -
> -        event = TRC_PV_HYPERCALL;
> -        event |= TRC_64_FLAG;
> -        d.eip = regs->eip;
> -        d.eax = regs->eax;
> -
> -        __trace_var(event, 1/*tsc*/, sizeof(d), &d);
> +        args[0] = regs->rdi;
> +        args[1] = regs->rsi;
> +        args[2] = regs->rdx;
> +        args[3] = regs->r10;
> +        args[4] = regs->r11;
>      }
> +
> +    __trace_hypercall(regs->eax, args);
>  }
>  
>  void __trace_pv_trap(int trapnr, unsigned long eip,
> diff --git a/xen/common/trace.c b/xen/common/trace.c
> index cacaeb2..47c9a6a 100644
> --- a/xen/common/trace.c
> +++ b/xen/common/trace.c
> @@ -816,6 +816,29 @@ unlock:
>          tasklet_schedule(&trace_notify_dom0_tasklet);
>  }
>  
> +void __trace_hypercall(unsigned long op, const unsigned long *args)
> +{
> +    struct {
> +        uint32_t op;
> +        uint32_t args[5];
> +    } __attribute__((packed)) d;
> +    uint32_t *a = d.args;
> +
> +    d.op = op;
> +
> +    switch (op) {
> +    case __HYPERVISOR_multicall:
> +        *a++ = args[1]; /* count */
> +        break;
> +    case __HYPERVISOR_mmu_update:
> +        *a++ = args[1]; /* count */
> +        break;
> +    }
> +
> +    __trace_var(TRC_PV_HYPERCALL_V2, 1,
> +                sizeof(uint32_t) * (1 + (a - d.args)), &d);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/public/trace.h b/xen/include/public/trace.h
> index 0dfabe9..38a3f83 100644
> --- a/xen/include/public/trace.h
> +++ b/xen/include/public/trace.h
> @@ -106,6 +106,7 @@
>  #define TRC_PV_GDT_LDT_MAPPING_FAULT (TRC_PV + 10)
>  #define TRC_PV_PTWR_EMULATION        (TRC_PV + 11)
>  #define TRC_PV_PTWR_EMULATION_PAE    (TRC_PV + 12)
> +#define TRC_PV_HYPERCALL_V2          (TRC_PV + 14)
>    /* Indicates that addresses in trace record are 64 bits */
>  #define TRC_64_FLAG               (0x100) 
>  
> diff --git a/xen/include/xen/trace.h b/xen/include/xen/trace.h
> index b32f6c5..f601aeb 100644
> --- a/xen/include/xen/trace.h
> +++ b/xen/include/xen/trace.h
> @@ -44,6 +44,8 @@ static inline void trace_var(u32 event, int cycles, int 
> extra,
>          __trace_var(event, cycles, extra, extra_data);
>  }
>  
> +void __trace_hypercall(unsigned long call, const unsigned long *args);
> +
>  /* Convenience macros for calling the trace function. */
>  #define TRACE_0D(_e)                            \
>      do {                                        \

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