From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v4 2/2] x86/VPMU: implement ipc and arch filter flags Date: Mon, 7 Dec 2015 14:13:47 -0500 Message-ID: <5665DA6B.8010300@oracle.com> References: <1448930346-16902-1-git-send-email-bgregg@netflix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1448930346-16902-1-git-send-email-bgregg@netflix.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: Brendan Gregg , xen-devel@lists.xen.org Cc: Andrew Cooper , "Tian, Kevin" , Jan Beulich , Jun Nakajima , dietmar.hahn@ts.fujitsu.com List-Id: xen-devel@lists.xenproject.org On 11/30/2015 07:39 PM, Brendan Gregg wrote: > This introduces a way to have a restricted VPMU, by specifying one of two > predefined groups of PMCs to make available. For secure environments, this > allows the VPMU to be used without needing to enable all PMCs. > > Signed-off-by: Brendan Gregg > Reviewed-by: Boris Ostrovsky This needs to be reviewed also by Intel maintainers (copied). Plus x86 maintainers. -boris > --- > docs/misc/xen-command-line.markdown | 15 ++++++++++- > xen/arch/x86/cpu/vpmu.c | 51 +++++++++++++++++++++++++++++-------- > xen/arch/x86/cpu/vpmu_intel.c | 50 ++++++++++++++++++++++++++++++++++++ > xen/include/asm-x86/msr-index.h | 1 + > xen/include/public/pmu.h | 14 ++++++++-- > 5 files changed, 117 insertions(+), 14 deletions(-) > > diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown > index 70daa84..795661e 100644 > --- a/docs/misc/xen-command-line.markdown > +++ b/docs/misc/xen-command-line.markdown > @@ -1452,7 +1452,7 @@ Use Virtual Processor ID support if available. This prevents the need for TLB > flushes on VM entry and exit, increasing performance. > > ### vpmu > -> `= ( bts )` > +> `= ( | { bts | ipc | arch [, ...] } )` > > > Default: `off` > > @@ -1468,6 +1468,19 @@ wrong behaviour (see handle\_pmc\_quirk()). > If 'vpmu=bts' is specified the virtualisation of the Branch Trace Store (BTS) > feature is switched on on Intel processors supporting this feature. > > +vpmu=ipc enables performance monitoring, but restricts the counters to the > +most minimum set possible: instructions, cycles, and reference cycles. These > +can be used to calculate instructions per cycle (IPC). > + > +vpmu=arch enables performance monitoring, but restricts the counters to the > +pre-defined architectural events only. These are exposed by cpuid, and listed > +in the Pre-Defined Architectural Performance Events table from the Intel 64 > +and IA-32 Architectures Software Developer's Manual, Volume 3B, System > +Programming Guide, Part 2. > + > +If a boolean is not used, combinations of flags are allowed, comma separated. > +For example, vpmu=arch,bts. > + > Note that if **watchdog** option is also specified vpmu will be turned off. > > *Warning:* > diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c > index 2f5156a..46b5324 100644 > --- a/xen/arch/x86/cpu/vpmu.c > +++ b/xen/arch/x86/cpu/vpmu.c > @@ -43,34 +43,59 @@ CHECK_pmu_data; > CHECK_pmu_params; > > /* > - * "vpmu" : vpmu generally enabled > - * "vpmu=off" : vpmu generally disabled > - * "vpmu=bts" : vpmu enabled and Intel BTS feature switched on. > + * "vpmu" : vpmu generally enabled (all counters) > + * "vpmu=off" : vpmu generally disabled > + * "vpmu=bts" : vpmu enabled and Intel BTS feature switched on. > + * "vpmu=ipc" : vpmu enabled for IPC counters only (most restrictive) > + * "vpmu=arch" : vpmu enabled for predef arch counters only (restrictive) > + * flag combinations are allowed, eg, "vpmu=ipc,bts". > */ > static unsigned int __read_mostly opt_vpmu_enabled; > unsigned int __read_mostly vpmu_mode = XENPMU_MODE_OFF; > unsigned int __read_mostly vpmu_features = 0; > -static void parse_vpmu_param(char *s); > -custom_param("vpmu", parse_vpmu_param); > +static void parse_vpmu_params(char *s); > +custom_param("vpmu", parse_vpmu_params); > > static DEFINE_SPINLOCK(vpmu_lock); > static unsigned vpmu_count; > > static DEFINE_PER_CPU(struct vcpu *, last_vcpu); > > -static void __init parse_vpmu_param(char *s) > +static int parse_vpmu_param(char *s, int len) > { > + if ( ! *s || ! len ) > + return 0; > + if ( !strncmp(s, "bts", len) ) > + vpmu_features |= XENPMU_FEATURE_INTEL_BTS; > + else if ( !strncmp(s, "ipc", len) ) > + vpmu_features |= XENPMU_FEATURE_IPC_ONLY; > + else if ( !strncmp(s, "arch", len) ) > + vpmu_features |= XENPMU_FEATURE_ARCH_ONLY; > + else > + return 1; > + return 0; > +} > + > +static void __init parse_vpmu_params(char *s) > +{ > + char *sep, *p = s; > + > switch ( parse_bool(s) ) > { > case 0: > break; > default: > - if ( !strcmp(s, "bts") ) > - vpmu_features |= XENPMU_FEATURE_INTEL_BTS; > - else if ( *s ) > + while (1) > { > - printk("VPMU: unknown flag: %s - vpmu disabled!\n", s); > - break; > + sep = strchr(p, ','); > + if ( sep == NULL ) > + sep = strchr(p, 0); > + if ( parse_vpmu_param(p, sep - p) ) > + goto error; > + if ( *sep == 0 ) > + /* reached end of flags */ > + break; > + p = sep + 1; > } > /* fall through */ > case 1: > @@ -79,6 +104,10 @@ static void __init parse_vpmu_param(char *s) > opt_vpmu_enabled = 1; > break; > } > + return; > + > + error: > + printk("VPMU: unknown flags: %s - vpmu disabled!\n", s); > } > > void vpmu_lvtpc_update(uint32_t val) > diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c > index 8d83a1a..a6c5545 100644 > --- a/xen/arch/x86/cpu/vpmu_intel.c > +++ b/xen/arch/x86/cpu/vpmu_intel.c > @@ -602,12 +602,19 @@ static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, > "MSR_PERF_GLOBAL_STATUS(0x38E)!\n"); > return -EINVAL; > case MSR_IA32_PEBS_ENABLE: > + if ( vpmu_features & (XENPMU_FEATURE_IPC_ONLY | > + XENPMU_FEATURE_ARCH_ONLY) ) > + return -EINVAL; > if ( msr_content & 1 ) > gdprintk(XENLOG_WARNING, "Guest is trying to enable PEBS, " > "which is not supported.\n"); > core2_vpmu_cxt->pebs_enable = msr_content; > return 0; > case MSR_IA32_DS_AREA: > + if ( (vpmu_features & (XENPMU_FEATURE_IPC_ONLY | > + XENPMU_FEATURE_ARCH_ONLY)) && > + !(vpmu_features & XENPMU_FEATURE_INTEL_BTS) ) > + return -EINVAL; > if ( vpmu_is_set(vpmu, VPMU_CPU_HAS_DS) ) > { > if ( !is_canonical_address(msr_content) ) > @@ -652,12 +659,55 @@ static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, > tmp = msr - MSR_P6_EVNTSEL(0); > if ( tmp >= 0 && tmp < arch_pmc_cnt ) > { > + bool_t blocked = 0; > + uint64_t umaskevent; > struct xen_pmu_cntr_pair *xen_pmu_cntr_pair = > vpmu_reg_pointer(core2_vpmu_cxt, arch_counters); > > if ( msr_content & ARCH_CTRL_MASK ) > return -EINVAL; > > + /* PMC filters */ > + umaskevent = msr_content & MSR_IA32_CMT_EVTSEL_UE_MASK; > + if ( vpmu_features & XENPMU_FEATURE_IPC_ONLY || > + vpmu_features & XENPMU_FEATURE_ARCH_ONLY ) > + { > + blocked = 1; > + switch ( umaskevent ) > + { > + /* > + * See the Pre-Defined Architectural Performance Events table > + * from the Intel 64 and IA-32 Architectures Software > + * Developer's Manual, Volume 3B, System Programming Guide, > + * Part 2. > + */ > + case 0x003c: /* unhalted core cycles */ > + case 0x013c: /* unhalted ref cycles */ > + case 0x00c0: /* instruction retired */ > + blocked = 0; > + default: > + break; > + } > + } > + > + if ( vpmu_features & XENPMU_FEATURE_ARCH_ONLY ) > + { > + /* additional counters beyond IPC only; blocked already set */ > + switch ( umaskevent ) > + { > + case 0x4f2e: /* LLC reference */ > + case 0x412e: /* LLC misses */ > + case 0x00c4: /* branch instruction retired */ > + case 0x00c5: /* branch */ > + blocked = 0; > + default: > + break; > + } > + } > + > + if ( blocked ) > + return -EINVAL; > + > if ( has_hvm_container_vcpu(v) ) > vmx_read_guest_msr(MSR_CORE_PERF_GLOBAL_CTRL, > &core2_vpmu_cxt->global_ctrl); > diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h > index b8ad93c..0542064 100644 > --- a/xen/include/asm-x86/msr-index.h > +++ b/xen/include/asm-x86/msr-index.h > @@ -328,6 +328,7 @@ > > /* Platform Shared Resource MSRs */ > #define MSR_IA32_CMT_EVTSEL 0x00000c8d > +#define MSR_IA32_CMT_EVTSEL_UE_MASK 0x0000ffff > #define MSR_IA32_CMT_CTR 0x00000c8e > #define MSR_IA32_PSR_ASSOC 0x00000c8f > #define MSR_IA32_PSR_L3_QOS_CFG 0x00000c81 > diff --git a/xen/include/public/pmu.h b/xen/include/public/pmu.h > index 7753df0..f9ad7b4 100644 > --- a/xen/include/public/pmu.h > +++ b/xen/include/public/pmu.h > @@ -84,9 +84,19 @@ DEFINE_XEN_GUEST_HANDLE(xen_pmu_params_t); > > /* > * PMU features: > - * - XENPMU_FEATURE_INTEL_BTS: Intel BTS support (ignored on AMD) > + * - XENPMU_FEATURE_INTEL_BTS: Intel BTS support (ignored on AMD) > + * - XENPMU_FEATURE_IPC_ONLY: Restrict PMC to the most minimum set possible. > + * Instructions, cycles, and ref cycles. Can be > + * used to calculate instructions-per-cycle (IPC) > + * (ignored on AMD). > + * - XENPMU_FEATURE_ARCH_ONLY: Restrict PMCs to the Intel Pre-Defined > + * Architecteral Performance Events exposed by > + * cpuid and listed in the Intel developer's manual > + * (ignored on AMD). > */ > -#define XENPMU_FEATURE_INTEL_BTS 1 > +#define XENPMU_FEATURE_INTEL_BTS (1<<0) > +#define XENPMU_FEATURE_IPC_ONLY (1<<1) > +#define XENPMU_FEATURE_ARCH_ONLY (1<<2) > > /* > * Shared PMU data between hypervisor and PV(H) domains.