Re: [PATCH v11 for-xen-4.5 11/20] x86/VPMU: Interface for setting PMU mode and flags

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Mon, Sep 22, 2014 at 07:57:52PM -0400, Boris Ostrovsky wrote:
> Add runtime interface for setting PMU mode and flags. Three main modes are
> provided:
> * XENPMU_MODE_OFF:  PMU is not virtualized
> * XENPMU_MODE_SELF: Guests can access PMU MSRs and receive PMU interrupts.
> * XENPMU_MODE_HV: Same as XENPMU_MODE_SELF for non-proviledged guests, dom0
>   can profile itself and the hypervisor.
> 
> Note that PMU modes are different from what can be provided at Xen's boot line
> with 'vpmu' argument. An 'off' (or '0') value is equivalent to XENPMU_MODE_OFF.
> Any other value, on the other hand, will cause VPMU mode to be set to
> XENPMU_MODE_SELF during boot.
> 
> For feature flags only Intel's BTS is currently supported.
> 
> Mode and flags are set via HYPERVISOR_xenpmu_op hypercall.

I forgot to mention that we should probably also include the XSM calls.
(And CC the maintainer of it).

For examples, see the claim (919f59b3b99e1d845c6a1f30125e79e828805d87) or
vnmua (40c7af684fc0bf89c7b643a5492b4abd920fba86) patches.

> +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:
> +    {
> +        static DEFINE_SPINLOCK(xenpmu_mode_lock);
> +        uint32_t current_mode;
> +
> +        if ( !is_control_domain(current->domain) )
> +            return -EPERM;
> +
> +        if ( copy_from_guest(&pmu_params, arg, 1) )
> +            return -EFAULT;
> +
> +        if ( pmu_params.val & ~(XENPMU_MODE_SELF | XENPMU_MODE_HV) )
> +            return -EINVAL;
> +
> +        /*
> +         * Return error is someone else is in the middle of changing mode ---
> +         * this is most likely indication of two system administrators
> +         * working against each other
> +         */
> +        if ( !spin_trylock(&xenpmu_mode_lock) )
> +            return -EAGAIN;
> +
> +        current_mode = vpmu_mode;
> +        vpmu_mode = pmu_params.val;
> +
> +        if ( vpmu_mode == XENPMU_MODE_OFF )
> +        {
> +            /*
> +             * Make sure all (non-dom0) VCPUs have unloaded their VPMUs. This
> +             * can be achieved by having all physical processors go through
> +             * context_switch().
> +             */
> +            ret = vpmu_force_context_switch();
> +            if ( ret )
> +                vpmu_mode = current_mode;
> +        }
> +        else
> +            ret = 0;
> +
> +        spin_unlock(&xenpmu_mode_lock);
> +        break;
> +    }
> +
> +    case XENPMU_mode_get:
> +        memset(&pmu_params, 0, sizeof(pmu_params));
> +        pmu_params.val = vpmu_mode;
> +        pmu_params.version.maj = XENPMU_VER_MAJ;
> +        pmu_params.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.val & ~XENPMU_FEATURE_INTEL_BTS )
> +            return -EINVAL;
> +
> +        vpmu_features = pmu_params.val;
> +
> +        ret = 0;
> +        break;
> +
> +    case XENPMU_feature_get:
> +        memset(&pmu_params, 0, sizeof(pmu_params));
> +        pmu_params.val = vpmu_features;
> +        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 ac594c9..8587c46 100644
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -417,6 +417,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
> @@ -465,6 +467,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 a3ed216..704f4d1 100644
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -762,6 +762,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
> @@ -810,6 +812,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/Makefile b/xen/include/Makefile
> index f7ccbc9..f97733a 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -26,7 +26,9 @@ headers-y := \
>  headers-$(CONFIG_X86)     += compat/arch-x86/xen-mca.h
>  headers-$(CONFIG_X86)     += compat/arch-x86/xen.h
>  headers-$(CONFIG_X86)     += compat/arch-x86/xen-$(compat-arch-y).h
> +headers-$(CONFIG_X86)     += compat/arch-x86/pmu.h
>  headers-y                 += compat/arch-$(compat-arch-y).h compat/xlat.h
> +headers-y                 += compat/pmu.h
>  headers-$(FLASK_ENABLE)   += compat/xsm/flask_op.h
>  
>  cppflags-y                := -include public/xen-compat.h
> diff --git a/xen/include/asm-x86/hvm/vpmu.h b/xen/include/asm-x86/hvm/vpmu.h
> index 6fa0def..c612e1a 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)
>  
> @@ -59,8 +52,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;
> @@ -116,5 +109,21 @@ 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;
> +
> +/* Context switch */
> +inline void vpmu_switch_from(struct vcpu *prev, struct vcpu *next)
> +{
> +    if ( vpmu_mode & (XENPMU_MODE_SELF | XENPMU_MODE_HV) )
> +        vpmu_save(prev);
> +}
> +
> +inline void vpmu_switch_to(struct vcpu *prev, struct vcpu *next)
> +{
> +    if ( vpmu_mode & (XENPMU_MODE_SELF | XENPMU_MODE_HV) )
> +        vpmu_load(next);
> +}
> +
>  #endif /* __ASM_X86_HVM_VPMU_H_*/
>  
> diff --git a/xen/include/public/pmu.h b/xen/include/public/pmu.h
> index e6f45ee..c2293be 100644
> --- a/xen/include/public/pmu.h
> +++ b/xen/include/public/pmu.h
> @@ -13,6 +13,50 @@
>  #define XENPMU_VER_MAJ    0
>  #define XENPMU_VER_MIN    1
>  
> +/*
> + * ` 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 */
> +    struct {
> +        uint32_t maj;
> +        uint32_t min;
> +    } version;
> +    uint64_t val;
> +
> +    /* 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_SELF:  Guests can profile themselves
> + * - XENPMU_MODE_HV:    Guests can profile themselves, dom0 profiles
> + *                      itself and Xen
> + */
> +#define XENPMU_MODE_OFF           0
> +#define XENPMU_MODE_SELF          (1<<0)
> +#define XENPMU_MODE_HV            (1<<1)
> +
> +/*
> + * 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 a6a2092..0766790 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
> diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
> index c8fafef..5809c60 100644
> --- a/xen/include/xlat.lst
> +++ b/xen/include/xlat.lst
> @@ -101,6 +101,10 @@
>  !	vcpu_set_singleshot_timer	vcpu.h
>  ?	xenoprof_init			xenoprof.h
>  ?	xenoprof_passive		xenoprof.h
> +?	pmu_params			pmu.h
> +?	pmu_intel_ctxt			arch-x86/pmu.h
> +?	pmu_amd_ctxt			arch-x86/pmu.h
> +?	pmu_cntr_pair			arch-x86/pmu.h
>  ?	flask_access			xsm/flask_op.h
>  !	flask_boolean			xsm/flask_op.h
>  ?	flask_cache_stats		xsm/flask_op.h
> -- 
> 1.8.1.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel