[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v5 RESEND 09/17] x86/VPMU: Interface for setting PMU mode and flags



> From: Boris Ostrovsky [mailto:boris.ostrovsky@xxxxxxxxxx]
> Sent: Wednesday, April 23, 2014 8:51 PM
> 
> Add runtime interface for setting PMU mode and flags. Three main modes are
> provided:
> * PMU off
> * PMU on: Guests can access PMU MSRs and receive PMU interrupts. dom0
>   profiles itself and the hypervisor.
> * dom0-only PMU: dom0 collects samples for both itself and guests.
> 
> For feature flags only Intel's BTS is currently supported.
> 
> Mode and flags are set via HYPERVISOR_xenpmu_op hypercall.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>

Similarly, why do you want so many "XEN" and "xen_" prefixes? :-)

> ---
>  xen/arch/x86/domain.c              |   4 +-
>  xen/arch/x86/hvm/svm/vpmu.c        |   4 +-
>  xen/arch/x86/hvm/vmx/vpmu_core2.c  |  10 +--
>  xen/arch/x86/hvm/vpmu.c            | 121
> ++++++++++++++++++++++++++++++++++---
>  xen/arch/x86/x86_64/compat/entry.S |   4 ++
>  xen/arch/x86/x86_64/entry.S        |   4 ++
>  xen/include/asm-x86/hvm/vpmu.h     |  14 ++---
>  xen/include/public/pmu.h           |  48 +++++++++++++++
>  xen/include/public/xen.h           |   1 +
>  xen/include/xen/hypercall.h        |   4 ++
>  10 files changed, 188 insertions(+), 26 deletions(-)
> 
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 6618ae6..b615c07 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1465,7 +1465,7 @@ void context_switch(struct vcpu *prev, struct vcpu
> *next)
> 
>      if ( is_hvm_vcpu(prev) )
>      {
> -        if (prev != next)
> +        if ( (prev != next) && (vpmu_mode & XENPMU_MODE_ON) )
>              vpmu_save(prev);
> 
>          if ( !list_empty(&prev->arch.hvm_vcpu.tm_list) )
> @@ -1508,7 +1508,7 @@ void context_switch(struct vcpu *prev, struct vcpu
> *next)
>                             (next->domain->domain_id != 0));
>      }
> 
> -    if (is_hvm_vcpu(next) && (prev != next) )
> +    if ( is_hvm_vcpu(next) && (prev != next) && (vpmu_mode &
> XENPMU_MODE_ON) )
>          /* Must be done with interrupts enabled */
>          vpmu_load(next);
> 
> diff --git a/xen/arch/x86/hvm/svm/vpmu.c b/xen/arch/x86/hvm/svm/vpmu.c
> index d199f08..97cd6e5 100644
> --- a/xen/arch/x86/hvm/svm/vpmu.c
> +++ b/xen/arch/x86/hvm/svm/vpmu.c
> @@ -472,14 +472,14 @@ struct arch_vpmu_ops amd_vpmu_ops = {
>      .arch_vpmu_dump = amd_vpmu_dump
>  };
> 
> -int svm_vpmu_initialise(struct vcpu *v, unsigned int vpmu_flags)
> +int svm_vpmu_initialise(struct vcpu *v)
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
>      uint8_t family = current_cpu_data.x86;
>      int ret = 0;
> 
>      /* vpmu enabled? */
> -    if ( !vpmu_flags )
> +    if ( vpmu_mode == XENPMU_MODE_OFF )
>          return 0;
> 
>      switch ( family )
> diff --git a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> index 856281d..efbebe2 100644
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> @@ -703,13 +703,13 @@ static int core2_vpmu_do_interrupt(struct
> cpu_user_regs *regs)
>      return 1;
>  }
> 
> -static int core2_vpmu_initialise(struct vcpu *v, unsigned int vpmu_flags)
> +static int core2_vpmu_initialise(struct vcpu *v)
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
>      u64 msr_content;
>      struct cpuinfo_x86 *c = &current_cpu_data;
> 
> -    if ( !(vpmu_flags & VPMU_BOOT_BTS) )
> +    if ( !(vpmu_features & XENPMU_FEATURE_INTEL_BTS) )
>          goto func_out;
>      /* Check the 'Debug Store' feature in the CPUID.EAX[1]:EDX[21] */
>      if ( cpu_has(c, X86_FEATURE_DS) )
> @@ -820,7 +820,7 @@ struct arch_vpmu_ops core2_no_vpmu_ops = {
>      .do_cpuid = core2_no_vpmu_do_cpuid,
>  };
> 
> -int vmx_vpmu_initialise(struct vcpu *v, unsigned int vpmu_flags)
> +int vmx_vpmu_initialise(struct vcpu *v)
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
>      uint8_t family = current_cpu_data.x86;
> @@ -828,7 +828,7 @@ int vmx_vpmu_initialise(struct vcpu *v, unsigned int
> vpmu_flags)
>      int ret = 0;
> 
>      vpmu->arch_vpmu_ops = &core2_no_vpmu_ops;
> -    if ( !vpmu_flags )
> +    if ( vpmu_mode == XENPMU_MODE_OFF )
>          return 0;
> 
>      if ( family == 6 )
> @@ -867,7 +867,7 @@ int vmx_vpmu_initialise(struct vcpu *v, unsigned int
> vpmu_flags)
>          case 0x3f:
>          case 0x45:
>          case 0x46:
> -            ret = core2_vpmu_initialise(v, vpmu_flags);
> +            ret = core2_vpmu_initialise(v);
>              if ( !ret )
>                  vpmu->arch_vpmu_ops = &core2_vpmu_ops;
>              return ret;
> diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
> index a6e933a..50f6bb8 100644
> --- a/xen/arch/x86/hvm/vpmu.c
> +++ b/xen/arch/x86/hvm/vpmu.c
> @@ -21,6 +21,7 @@
>  #include <xen/config.h>
>  #include <xen/sched.h>
>  #include <xen/xenoprof.h>
> +#include <xen/guest_access.h>
>  #include <asm/regs.h>
>  #include <asm/types.h>
>  #include <asm/msr.h>
> @@ -38,7 +39,8 @@
>   * "vpmu=off" : vpmu generally disabled
>   * "vpmu=bts" : vpmu enabled and Intel BTS feature switched on.
>   */
> -static unsigned int __read_mostly opt_vpmu_enabled;
> +uint64_t __read_mostly vpmu_mode = XENPMU_MODE_OFF;
> +uint64_t __read_mostly vpmu_features = 0;
>  static void parse_vpmu_param(char *s);
>  custom_param("vpmu", parse_vpmu_param);
> 
> @@ -52,7 +54,7 @@ static void __init parse_vpmu_param(char *s)
>          break;
>      default:
>          if ( !strcmp(s, "bts") )
> -            opt_vpmu_enabled |= VPMU_BOOT_BTS;
> +            vpmu_features |= XENPMU_FEATURE_INTEL_BTS;
>          else if ( *s )
>          {
>              printk("VPMU: unknown flag: %s - vpmu disabled!\n", s);
> @@ -60,7 +62,7 @@ static void __init parse_vpmu_param(char *s)
>          }
>          /* fall through */
>      case 1:
> -        opt_vpmu_enabled |= VPMU_BOOT_ENABLED;
> +        vpmu_mode = XENPMU_MODE_ON;
>          break;
>      }
>  }
> @@ -77,6 +79,9 @@ int vpmu_do_wrmsr(unsigned int msr, uint64_t
> msr_content)
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(current);
> 
> +    if ( !(vpmu_mode & XENPMU_MODE_ON) )
> +        return 0;
> +
>      if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
>          return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content);
>      return 0;
> @@ -86,6 +91,9 @@ int vpmu_do_rdmsr(unsigned int msr, uint64_t
> *msr_content)
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(current);
> 
> +    if ( !(vpmu_mode & XENPMU_MODE_ON) )
> +        return 0;
> +
>      if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_rdmsr )
>          return vpmu->arch_vpmu_ops->do_rdmsr(msr, msr_content);
>      return 0;
> @@ -237,19 +245,19 @@ void vpmu_initialise(struct vcpu *v)
>      switch ( vendor )
>      {
>      case X86_VENDOR_AMD:
> -        if ( svm_vpmu_initialise(v, opt_vpmu_enabled) != 0 )
> -            opt_vpmu_enabled = 0;
> +        if ( svm_vpmu_initialise(v) != 0 )
> +            vpmu_mode = XENPMU_MODE_OFF;
>          return;
> 
>      case X86_VENDOR_INTEL:
> -        if ( vmx_vpmu_initialise(v, opt_vpmu_enabled) != 0 )
> -            opt_vpmu_enabled = 0;
> +        if ( vmx_vpmu_initialise(v) != 0 )
> +            vpmu_mode = XENPMU_MODE_OFF;
>          return;
> 
>      default:
>          printk("VPMU: Initialization failed. "
>                 "Unknown CPU vendor %d\n", vendor);
> -        opt_vpmu_enabled = 0;
> +        vpmu_mode = XENPMU_MODE_OFF;
>          return;
>      }
>  }
> @@ -271,3 +279,100 @@ void vpmu_dump(struct vcpu *v)
>          vpmu->arch_vpmu_ops->arch_vpmu_dump(v);
>  }
> 
> +/* Unload VPMU contexts */
> +static void vpmu_unload_all(void)
> +{
> +    struct domain *d;
> +    struct vcpu *v;
> +    struct vpmu_struct *vpmu;
> +
> +    for_each_domain(d)
> +    {
> +        for_each_vcpu ( d, v )
> +        {
> +            if ( v != current )
> +                vcpu_pause(v);
> +            vpmu = vcpu_vpmu(v);
> +
> +            if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
> +            {
> +                if ( v != current )
> +                    vcpu_unpause(v);
> +                continue;
> +            }
> +
> +            vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
> +            on_selected_cpus(cpumask_of(vpmu->last_pcpu),
> +                             vpmu_save_force, (void *)v, 1);
> +            vpmu_reset(vpmu, VPMU_CONTEXT_LOADED);
> +
> +            if ( v != current )
> +                vcpu_unpause(v);
> +        }
> +    }
> +}
> +
> +
> +long do_xenpmu_op(int op,
> XEN_GUEST_HANDLE_PARAM(xen_pmu_params_t) arg)
> +{
> +    int ret = -EINVAL;
> +    xen_pmu_params_t pmu_params;
> +
> +    switch ( op )
> +    {
> +    case XENPMU_mode_set:
> +        if ( !is_control_domain(current->domain) )
> +            return -EPERM;
> +
> +        if ( copy_from_guest(&pmu_params, arg, 1) )
> +            return -EFAULT;
> +
> +        if ( pmu_params.d.val & ~XENPMU_MODE_ON )
> +            return -EINVAL;
> +
> +        vpmu_mode = pmu_params.d.val;
> +        if ( vpmu_mode == XENPMU_MODE_OFF )
> +            /*
> +             * After this VPMU context will never be loaded during
> context
> +             * switch. We also prevent PMU MSR accesses (which can
> load
> +             * context) when VPMU is disabled.
> +             */
> +            vpmu_unload_all();
> +
> +        ret = 0;
> +        break;
> +
> +    case XENPMU_mode_get:
> +        pmu_params.d.val = vpmu_mode;
> +        pmu_params.v.version.maj = XENPMU_VER_MAJ;
> +        pmu_params.v.version.min = XENPMU_VER_MIN;
> +        if ( copy_to_guest(arg, &pmu_params, 1) )
> +            return -EFAULT;
> +        ret = 0;
> +        break;
> +
> +    case XENPMU_feature_set:
> +        if ( !is_control_domain(current->domain) )
> +            return -EPERM;
> +
> +        if ( copy_from_guest(&pmu_params, arg, 1) )
> +            return -EFAULT;
> +
> +        if ( pmu_params.d.val & ~XENPMU_FEATURE_INTEL_BTS )
> +            return -EINVAL;
> +
> +        vpmu_features = pmu_params.d.val;
> +
> +        ret = 0;
> +        break;
> +
> +    case XENPMU_feature_get:
> +        pmu_params.d.val = vpmu_mode;
> +        if ( copy_to_guest(arg, &pmu_params, 1) )
> +            return -EFAULT;
> +        ret = 0;
> +        break;
> +     }
> +
> +    return ret;
> +}
> diff --git a/xen/arch/x86/x86_64/compat/entry.S
> b/xen/arch/x86/x86_64/compat/entry.S
> index 594b0b9..07c736d 100644
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -416,6 +416,8 @@ ENTRY(compat_hypercall_table)
>          .quad do_domctl
>          .quad compat_kexec_op
>          .quad do_tmem_op
> +        .quad do_ni_hypercall           /* reserved for XenClient */
> +        .quad do_xenpmu_op              /* 40 */
>          .rept __HYPERVISOR_arch_0-((.-compat_hypercall_table)/8)
>          .quad compat_ni_hypercall
>          .endr
> @@ -464,6 +466,8 @@ ENTRY(compat_hypercall_args_table)
>          .byte 1 /* do_domctl                */
>          .byte 2 /* compat_kexec_op          */
>          .byte 1 /* do_tmem_op               */
> +        .byte 0 /* reserved for XenClient   */
> +        .byte 2 /* do_xenpmu_op             */  /* 40 */
>          .rept __HYPERVISOR_arch_0-(.-compat_hypercall_args_table)
>          .byte 0 /* compat_ni_hypercall      */
>          .endr
> diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
> index 3ea4683..c36ffce 100644
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -757,6 +757,8 @@ ENTRY(hypercall_table)
>          .quad do_domctl
>          .quad do_kexec_op
>          .quad do_tmem_op
> +        .quad do_ni_hypercall       /* reserved for XenClient */
> +        .quad do_xenpmu_op          /* 40 */
>          .rept __HYPERVISOR_arch_0-((.-hypercall_table)/8)
>          .quad do_ni_hypercall
>          .endr
> @@ -805,6 +807,8 @@ ENTRY(hypercall_args_table)
>          .byte 1 /* do_domctl            */
>          .byte 2 /* do_kexec             */
>          .byte 1 /* do_tmem_op           */
> +        .byte 0 /* reserved for XenClient */
> +        .byte 2 /* do_xenpmu_op         */  /* 40 */
>          .rept __HYPERVISOR_arch_0-(.-hypercall_args_table)
>          .byte 0 /* do_ni_hypercall      */
>          .endr
> diff --git a/xen/include/asm-x86/hvm/vpmu.h
> b/xen/include/asm-x86/hvm/vpmu.h
> index edc67f6..b945e8f 100644
> --- a/xen/include/asm-x86/hvm/vpmu.h
> +++ b/xen/include/asm-x86/hvm/vpmu.h
> @@ -24,13 +24,6 @@
> 
>  #include <public/pmu.h>
> 
> -/*
> - * Flag bits given as a string on the hypervisor boot parameter 'vpmu'.
> - * See arch/x86/hvm/vpmu.c.
> - */
> -#define VPMU_BOOT_ENABLED 0x1    /* vpmu generally enabled. */
> -#define VPMU_BOOT_BTS     0x2    /* Intel BTS feature wanted. */
> -
>  #define vcpu_vpmu(vcpu)   (&(vcpu)->arch.vpmu)
>  #define vpmu_vcpu(vpmu)   container_of(vpmu, struct vcpu, arch.vpmu)
> 
> @@ -58,8 +51,8 @@ struct arch_vpmu_ops {
>      void (*arch_vpmu_dump)(const struct vcpu *);
>  };
> 
> -int vmx_vpmu_initialise(struct vcpu *, unsigned int flags);
> -int svm_vpmu_initialise(struct vcpu *, unsigned int flags);
> +int vmx_vpmu_initialise(struct vcpu *);
> +int svm_vpmu_initialise(struct vcpu *);
> 
>  struct vpmu_struct {
>      u32 flags;
> @@ -98,5 +91,8 @@ void vpmu_dump(struct vcpu *v);
>  extern int acquire_pmu_ownership(int pmu_ownership);
>  extern void release_pmu_ownership(int pmu_ownership);
> 
> +extern uint64_t vpmu_mode;
> +extern uint64_t vpmu_features;
> +
>  #endif /* __ASM_X86_HVM_VPMU_H_*/
> 
> diff --git a/xen/include/public/pmu.h b/xen/include/public/pmu.h
> index 3ffd2cf..f91d935 100644
> --- a/xen/include/public/pmu.h
> +++ b/xen/include/public/pmu.h
> @@ -13,6 +13,54 @@
>  #define XENPMU_VER_MAJ    0
>  #define XENPMU_VER_MIN    0
> 
> +/*
> + * ` enum neg_errnoval
> + * ` HYPERVISOR_xenpmu_op(enum xenpmu_op cmd, struct xenpmu_params
> *args);
> + *
> + * @cmd  == XENPMU_* (PMU operation)
> + * @args == struct xenpmu_params
> + */
> +/* ` enum xenpmu_op { */
> +#define XENPMU_mode_get        0 /* Also used for getting PMU version
> */
> +#define XENPMU_mode_set        1
> +#define XENPMU_feature_get     2
> +#define XENPMU_feature_set     3
> +/* ` } */
> +
> +/* Parameters structure for HYPERVISOR_xenpmu_op call */
> +struct xen_pmu_params {
> +    /* IN/OUT parameters */
> +    union {
> +        struct version {
> +            uint8_t maj;
> +            uint8_t min;
> +        } version;
> +        uint64_t pad;
> +    } v;
> +    union {
> +        uint64_t val;
> +        XEN_GUEST_HANDLE(void) valp;
> +    } d;
> +
> +    /* IN parameters */
> +    uint64_t vcpu;
> +};
> +typedef struct xen_pmu_params xen_pmu_params_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_pmu_params_t);
> +
> +/* PMU modes:
> + * - XENPMU_MODE_OFF:   No PMU virtualization
> + * - XENPMU_MODE_ON:    Guests can profile themselves, dom0 profiles
> + *                      itself and Xen
> + */
> +#define XENPMU_MODE_OFF           0
> +#define XENPMU_MODE_ON            (1<<0)
> +
> +/*
> + * PMU features:
> + * - XENPMU_FEATURE_INTEL_BTS: Intel BTS support (ignored on AMD)
> + */
> +#define XENPMU_FEATURE_INTEL_BTS  1
> 
>  /* Shared between hypervisor and PV domain */
>  struct xen_pmu_data {
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index 8c5697e..a00ab21 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -101,6 +101,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
>  #define __HYPERVISOR_kexec_op             37
>  #define __HYPERVISOR_tmem_op              38
>  #define __HYPERVISOR_xc_reserved_op       39 /* reserved for XenClient
> */
> +#define __HYPERVISOR_xenpmu_op            40
> 
>  /* Architecture-specific hypercall definitions. */
>  #define __HYPERVISOR_arch_0               48
> diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
> index a9e5229..cf34547 100644
> --- a/xen/include/xen/hypercall.h
> +++ b/xen/include/xen/hypercall.h
> @@ -14,6 +14,7 @@
>  #include <public/event_channel.h>
>  #include <public/tmem.h>
>  #include <public/version.h>
> +#include <public/pmu.h>
>  #include <asm/hypercall.h>
>  #include <xsm/xsm.h>
> 
> @@ -139,6 +140,9 @@ do_tmem_op(
>  extern long
>  do_xenoprof_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);
> 
> +extern long
> +do_xenpmu_op(int op, XEN_GUEST_HANDLE_PARAM(xen_pmu_params_t)
> arg);
> +
>  #ifdef CONFIG_COMPAT
> 
>  extern int
> --
> 1.8.3.1


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.