From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v11 for-xen-4.5 11/20] x86/VPMU: Interface for setting PMU mode and flags Date: Tue, 23 Sep 2014 15:24:59 -0400 Message-ID: <5421C90B.5080505@oracle.com> References: <1411430281-6132-1-git-send-email-boris.ostrovsky@oracle.com> <1411430281-6132-12-git-send-email-boris.ostrovsky@oracle.com> <20140923185810.GW3007@laptop.dumpdata.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20140923185810.GW3007@laptop.dumpdata.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk Cc: kevin.tian@intel.com, keir@xen.org, jbeulich@suse.com, jun.nakajima@intel.com, andrew.cooper3@citrix.com, tim@xen.org, dietmar.hahn@ts.fujitsu.com, xen-devel@lists.xen.org, Aravind.Gopalakrishnan@amd.com, suravee.suthikulpanit@amd.com, Daniel De Graaf List-Id: xen-devel@lists.xenproject.org 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 >> >> -/* >> - * 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 >> #include >> #include >> +#include >> #include >> #include >> >> @@ -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