Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753775AbbHJOCm (ORCPT ); Mon, 10 Aug 2015 10:02:42 -0400 Received: from mga11.intel.com ([192.55.52.93]:7120 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751592AbbHJOCj convert rfc822-to-8bit (ORCPT ); Mon, 10 Aug 2015 10:02:39 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,645,1432623600"; d="scan'208";a="781214944" From: "Liang, Kan" To: Peter Zijlstra , Andy Lutomirski CC: Thomas Gleixner , "linux-kernel@vger.kernel.org" , "H. Peter Anvin" , "Andrew Lutomirski" , Linus Torvalds , Ingo Molnar , "linux-tip-commits@vger.kernel.org" Subject: RE: [tip:perf/core] perf/x86: Add an MSR PMU driver Thread-Topic: [tip:perf/core] perf/x86: Add an MSR PMU driver Thread-Index: AQHQzpQ65l/Dz0xJ8EeCrMA8u7d3Np37ZoGAgACIDlD//60vAIAAhnPA//99DICAAI7+8IACZbyAgAACUQCAAAhEgIABFe4AgAWRm7A= Date: Mon, 10 Aug 2015 14:02:17 +0000 Message-ID: <37D7C6CF3E00A74B8858931C1DB2F077018DF46D@SHSMSX103.ccr.corp.intel.com> References: <37D7C6CF3E00A74B8858931C1DB2F077018D16F1@SHSMSX103.ccr.corp.intel.com> <37D7C6CF3E00A74B8858931C1DB2F077018D206A@SHSMSX103.ccr.corp.intel.com> <37D7C6CF3E00A74B8858931C1DB2F077018D2103@SHSMSX103.ccr.corp.intel.com> <20150806152151.GG25159@twins.programming.kicks-ass.net> <20150806155943.GH25159@twins.programming.kicks-ass.net> <20150807083428.GB18673@twins.programming.kicks-ass.net> In-Reply-To: <20150807083428.GB18673@twins.programming.kicks-ass.net> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7374 Lines: 254 > On Thu, Aug 06, 2015 at 05:59:43PM +0200, Peter Zijlstra wrote: > > On Thu, Aug 06, 2015 at 08:30:08AM -0700, Andy Lutomirski wrote: > > > On Thu, Aug 6, 2015 at 8:21 AM, Peter Zijlstra > wrote: > > > > On Tue, Aug 04, 2015 at 08:39:27PM +0000, Liang, Kan wrote: > > > > - default: > > > > - err = -ENOTSUPP; > > > > + if (!msr[i].test() || rdmsrl_safe(msr[i].msr, &val)) > > > > + msr[i].attr = NULL; > > > > > > IIRC rdmsrl_safe literally never fails under QEMU TCG, and I'm not > > > > *sigh* the borkage never stops does it :-( > > > > > entirely sure what happens under KVM if emulation kicks in. It > > > might pay to keep the model check for the non-architectural stuff, > > > or at least check for a nonzero return value. > > > > Of course, 0 might be a valid value.. Esp. for the SMI counter. > > This then.. > > --- > arch/x86/kernel/cpu/perf_event_msr.c | 170 +++++++++++++++++------- > ----------- > 1 file changed, 85 insertions(+), 85 deletions(-) > > --- a/arch/x86/kernel/cpu/perf_event_msr.c > +++ b/arch/x86/kernel/cpu/perf_event_msr.c > @@ -10,17 +10,63 @@ enum perf_msr_id { > PERF_MSR_EVENT_MAX, > }; > > +bool test_aperfmperf(int idx) > +{ > + return boot_cpu_has(X86_FEATURE_APERFMPERF); > +} > + > +bool test_intel(int idx) > +{ > + if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL || > + boot_cpu_data.x86 != 6) > + return false; > + > + switch (boot_cpu_data.x86_model) { > + case 30: /* 45nm Nehalem */ > + case 26: /* 45nm Nehalem-EP */ > + case 46: /* 45nm Nehalem-EX */ > + > + case 37: /* 32nm Westmere */ > + case 44: /* 32nm Westmere-EP */ > + case 47: /* 32nm Westmere-EX */ > + > + case 42: /* 32nm SandyBridge */ > + case 45: /* 32nm SandyBridge-E/EN/EP */ > + > + case 58: /* 22nm IvyBridge */ > + case 62: /* 22nm IvyBridge-EP/EX */ > + > + case 60: /* 22nm Haswell Core */ > + case 63: /* 22nm Haswell Server */ > + case 69: /* 22nm Haswell ULT */ > + case 70: /* 22nm Haswell + GT3e (Intel Iris Pro graphics) */ > + > + case 61: /* 14nm Broadwell Core-M */ > + case 86: /* 14nm Broadwell Xeon D */ > + case 71: /* 14nm Broadwell + GT3e (Intel Iris Pro graphics) */ > + case 79: /* 14nm Broadwell Server */ > + > + case 55: /* 22nm Atom "Silvermont" */ > + case 77: /* 22nm Atom "Silvermont Avoton/Rangely" */ > + case 76: /* 14nm Atom "Airmont" */ > + if (idx == PERF_MSR_SMI) > + return true; > + break; > + > + case 78: /* 14nm Skylake Mobile */ > + case 94: /* 14nm Skylake Desktop */ > + if (idx == PERF_MSR_SMI || idx = PERF_MSR_PPERF) Here is a typo. The rest is OK for me. I did some simple test on HSX. Tested-by: Kan Liang > + return true; > + break; > + } > + > + return false; > +} > + > struct perf_msr { > - int id; > u64 msr; > -}; > - > -static struct perf_msr msr[] = { > - { PERF_MSR_TSC, 0 }, > - { PERF_MSR_APERF, MSR_IA32_APERF }, > - { PERF_MSR_MPERF, MSR_IA32_MPERF }, > - { PERF_MSR_PPERF, MSR_PPERF }, > - { PERF_MSR_SMI, MSR_SMI_COUNT }, > + struct perf_pmu_events_attr *attr; > + bool (*test)(int idx); > }; > > PMU_EVENT_ATTR_STRING(tsc, evattr_tsc, "event=0x00"); > @@ -29,8 +75,16 @@ PMU_EVENT_ATTR_STRING(mperf, evattr_mper > PMU_EVENT_ATTR_STRING(pperf, evattr_pperf, "event=0x03"); > PMU_EVENT_ATTR_STRING(smi, evattr_smi, "event=0x04"); > > +static struct perf_msr msr[] = { > + [PERF_MSR_TSC] = { 0, &evattr_tsc, > NULL, }, > + [PERF_MSR_APERF] = { MSR_IA32_APERF, &evattr_aperf, > test_aperfmperf, }, > + [PERF_MSR_MPERF] = { MSR_IA32_MPERF, &evattr_mperf, > test_aperfmperf, }, > + [PERF_MSR_PPERF] = { MSR_PPERF, &evattr_pperf, > test_intel, }, > + [PERF_MSR_SMI] = { MSR_SMI_COUNT, &evattr_smi, > test_intel, }, > +}; > + > static struct attribute *events_attrs[PERF_MSR_EVENT_MAX + 1] = { > - &evattr_tsc.attr.attr, > + NULL, > }; > > static struct attribute_group events_attr_group = { @@ -74,6 +128,9 @@ > static int msr_event_init(struct perf_ev > event->attr.sample_period) /* no sampling */ > return -EINVAL; > > + if (!msr[cfg].attr) > + return -EINVAL; > + > event->hw.idx = -1; > event->hw.event_base = msr[cfg].msr; > event->hw.config = cfg; > @@ -151,89 +208,32 @@ static struct pmu pmu_msr = { > .capabilities = PERF_PMU_CAP_NO_INTERRUPT, > }; > > -static int __init intel_msr_init(int idx) -{ > - if (boot_cpu_data.x86 != 6) > - return 0; > - > - switch (boot_cpu_data.x86_model) { > - case 30: /* 45nm Nehalem */ > - case 26: /* 45nm Nehalem-EP */ > - case 46: /* 45nm Nehalem-EX */ > - > - case 37: /* 32nm Westmere */ > - case 44: /* 32nm Westmere-EP */ > - case 47: /* 32nm Westmere-EX */ > - > - case 42: /* 32nm SandyBridge */ > - case 45: /* 32nm SandyBridge-E/EN/EP */ > - > - case 58: /* 22nm IvyBridge */ > - case 62: /* 22nm IvyBridge-EP/EX */ > - > - case 60: /* 22nm Haswell Core */ > - case 63: /* 22nm Haswell Server */ > - case 69: /* 22nm Haswell ULT */ > - case 70: /* 22nm Haswell + GT3e (Intel Iris Pro graphics) */ > - > - case 61: /* 14nm Broadwell Core-M */ > - case 86: /* 14nm Broadwell Xeon D */ > - case 71: /* 14nm Broadwell + GT3e (Intel Iris Pro graphics) */ > - case 79: /* 14nm Broadwell Server */ > - events_attrs[idx++] = &evattr_smi.attr.attr; > - break; > - > - case 78: /* 14nm Skylake Mobile */ > - case 94: /* 14nm Skylake Desktop */ > - events_attrs[idx++] = &evattr_pperf.attr.attr; > - events_attrs[idx++] = &evattr_smi.attr.attr; > - break; > - > - case 55: /* 22nm Atom "Silvermont" */ > - case 76: /* 14nm Atom "Airmont" */ > - case 77: /* 22nm Atom "Silvermont Avoton/Rangely" */ > - events_attrs[idx++] = &evattr_smi.attr.attr; > - break; > - } > - > - events_attrs[idx] = NULL; > - > - return 0; > -} > - > -static int __init amd_msr_init(int idx) -{ > - return 0; > -} > - > static int __init msr_init(void) > { > - int err; > - int idx = 1; > + int i, j = 0; > > - if (boot_cpu_has(X86_FEATURE_APERFMPERF)) { > - events_attrs[idx++] = &evattr_aperf.attr.attr; > - events_attrs[idx++] = &evattr_mperf.attr.attr; > - events_attrs[idx] = NULL; > + if (!boot_cpu_has(X86_FEATURE_TSC)) { > + pr_cont("no MSR PMU driver.\n"); > + return 0; > } > > - switch (boot_cpu_data.x86_vendor) { > - case X86_VENDOR_INTEL: > - err = intel_msr_init(idx); > - break; > - > - case X86_VENDOR_AMD: > - err = amd_msr_init(idx); > - break; > - > - default: > - err = -ENOTSUPP; > + /* Probe the MSRs. */ > + for (i = PERF_MSR_TSC + 1; i < PERF_MSR_EVENT_MAX; i++) { > + u64 val; > + > + /* > + * Virt sucks arse; you cannot tell if a R/O MSR is present :/ > + */ > + if (!msr[i].test(i) || rdmsrl_safe(msr[i].msr, &val)) > + msr[i].attr = NULL; > } > > - if (err != 0) { > - pr_cont("no msr PMU driver.\n"); > - return 0; > + /* List remaining MSRs in the sysfs attrs. */ > + for (i = 0; i < PERF_MSR_EVENT_MAX; i++) { > + if (msr[i].attr) > + events_attrs[j++] = &msr[i].attr->attr.attr; > } > + events_attrs[j] = NULL; > > perf_pmu_register(&pmu_msr, "msr", -1); > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/