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

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 09/23/2014 02:58 PM, Konrad Rzeszutek Wilk wrote:
> 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.

(+ Daniel)

I haven't looked at XSM in a few months but I remember when I tried to 
use XSM for this I found that we ran out of bits for a new hypercall in 
the permission vector. (I actually mention this in the cover letter, but 
it's way way down)

The two commits that you refer to are using existing hypercalls.

-boris

>
>> +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