Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933124AbdGKOeM (ORCPT ); Tue, 11 Jul 2017 10:34:12 -0400 Received: from mga05.intel.com ([192.55.52.43]:43377 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932722AbdGKOeL (ORCPT ); Tue, 11 Jul 2017 10:34:11 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,347,1496127600"; d="scan'208";a="125777092" From: kan.liang@intel.com To: peterz@infradead.org, mingo@redhat.com, linux-kernel@vger.kernel.org Cc: jolsa@redhat.com, acme@kernel.org, eranian@google.com, ak@linux.intel.com, Kan Liang Subject: [PATCH V3] perf/x86/intel/uncore: remove nonexistent clockticks event for client uncore Date: Tue, 11 Jul 2017 07:33:46 -0700 Message-Id: <20170711143346.2396-1-kan.liang@intel.com> X-Mailer: git-send-email 2.11.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5549 Lines: 172 From: Kan Liang For client uncore, all CBOXes seems have their own clockticks event uncore_cbox_*/clockticks/. However, only the clockticks event for CBOX_0 can work. Accessing other CBOX clockticks evnet will always return EINVAL. Here is an example. $ perf stat -x, -e uncore_cbox_0/clockticks/,uncore_cbox_1/clockticks/ sleep 1 10660314,,uncore_cbox_0/clockticks/,1001596935,100.00,,,, ,,uncore_cbox_1/clockticks/,0,100.00,,,, There is only one clockticks fixed counter in Hardware. Current kernel only allow the first CBOX to access the fixed counter. However, it does not expose the limitation to user space. User can still see the nonexistent clockticks event. Simply removing the clockticks event for the rest CBOX does not work. Because all boxes share the same attr_groups. If it is set for one box, it will applied to other boxes. The clockticks fixed counter is a standalone counter. It should be removed from the CBOX PMUs. A new type of PMU is added which only supports fixed counter events. User observable change with the patch. clockticks event is removed from CBOX. User may need to change their script to use uncore_clock/clockticks/ instead. $ perf stat -x, -e uncore_clock/clockticks/ sleep 1 10421811,,uncore_clock/clockticks/,1001581963,100.00,,,, Reported-by: Jiri Olsa Tested-by: Jiri Olsa Signed-off-by: Kan Liang --- The report and discussion can be found here. https://lkml.org/lkml/2016/11/30/277 Changes since V2: - Refine changelog Changes since V1: - Refine changelog - Add Tested-by arch/x86/events/intel/uncore.c | 13 +++++++------ arch/x86/events/intel/uncore_snb.c | 38 +++++++++++++++++++++++++++----------- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c index 8106df7db758..26a93f7a6003 100644 --- a/arch/x86/events/intel/uncore.c +++ b/arch/x86/events/intel/uncore.c @@ -658,6 +658,10 @@ static int uncore_pmu_event_init(struct perf_event *event) if (hwc->sample_period) return -EINVAL; + /* Single fixed PMU only has fixed event */ + if (pmu->type->single_fixed && (event->attr.config != UNCORE_FIXED_EVENT)) + return -EINVAL; + /* * Place all uncore events for a particular physical package * onto a single cpu @@ -681,12 +685,9 @@ static int uncore_pmu_event_init(struct perf_event *event) /* no fixed counter */ if (!pmu->type->fixed_ctl) return -EINVAL; - /* - * if there is only one fixed counter, only the first pmu - * can access the fixed counter - */ - if (pmu->type->single_fixed && pmu->pmu_idx > 0) - return -EINVAL; + + if (pmu->type->single_fixed) + event->hw.idx = UNCORE_PMC_IDX_FIXED; /* fixed counters have event field hardcoded to zero */ hwc->config = 0ULL; diff --git a/arch/x86/events/intel/uncore_snb.c b/arch/x86/events/intel/uncore_snb.c index dda44302207a..4090d872883c 100644 --- a/arch/x86/events/intel/uncore_snb.c +++ b/arch/x86/events/intel/uncore_snb.c @@ -122,7 +122,7 @@ static void snb_uncore_msr_exit_box(struct intel_uncore_box *box) } static struct uncore_event_desc snb_uncore_events[] = { - INTEL_UNCORE_EVENT_DESC(clockticks, "event=0xff,umask=0x00"), + INTEL_UNCORE_EVENT_DESC(clockticks, "event=0xff"), { /* end: all zeroes */ }, }; @@ -160,17 +160,12 @@ static struct intel_uncore_type snb_uncore_cbox = { .num_counters = 2, .num_boxes = 4, .perf_ctr_bits = 44, - .fixed_ctr_bits = 48, .perf_ctr = SNB_UNC_CBO_0_PER_CTR0, .event_ctl = SNB_UNC_CBO_0_PERFEVTSEL0, - .fixed_ctr = SNB_UNC_FIXED_CTR, - .fixed_ctl = SNB_UNC_FIXED_CTR_CTRL, - .single_fixed = 1, .event_mask = SNB_UNC_RAW_EVENT_MASK, .msr_offset = SNB_UNC_CBO_MSR_OFFSET, .ops = &snb_uncore_msr_ops, .format_group = &snb_uncore_format_group, - .event_descs = snb_uncore_events, }; static struct intel_uncore_type snb_uncore_arb = { @@ -187,9 +182,34 @@ static struct intel_uncore_type snb_uncore_arb = { .format_group = &snb_uncore_format_group, }; +static struct attribute *snb_uncore_clock_formats_attr[] = { + &format_attr_event.attr, + NULL, +}; + +static struct attribute_group snb_uncore_clock_format_group = { + .name = "format", + .attrs = snb_uncore_clock_formats_attr, +}; + +static struct intel_uncore_type snb_uncore_clockbox = { + .name = "clock", + .num_counters = 1, + .num_boxes = 1, + .fixed_ctr_bits = 48, + .fixed_ctr = SNB_UNC_FIXED_CTR, + .fixed_ctl = SNB_UNC_FIXED_CTR_CTRL, + .single_fixed = 1, + .event_mask = SNB_UNC_CTL_EV_SEL_MASK, + .format_group = &snb_uncore_clock_format_group, + .ops = &snb_uncore_msr_ops, + .event_descs = snb_uncore_events, +}; + static struct intel_uncore_type *snb_msr_uncores[] = { &snb_uncore_cbox, &snb_uncore_arb, + &snb_uncore_clockbox, NULL, }; @@ -234,22 +254,18 @@ static struct intel_uncore_type skl_uncore_cbox = { .num_counters = 4, .num_boxes = 5, .perf_ctr_bits = 44, - .fixed_ctr_bits = 48, .perf_ctr = SNB_UNC_CBO_0_PER_CTR0, .event_ctl = SNB_UNC_CBO_0_PERFEVTSEL0, - .fixed_ctr = SNB_UNC_FIXED_CTR, - .fixed_ctl = SNB_UNC_FIXED_CTR_CTRL, - .single_fixed = 1, .event_mask = SNB_UNC_RAW_EVENT_MASK, .msr_offset = SNB_UNC_CBO_MSR_OFFSET, .ops = &skl_uncore_msr_ops, .format_group = &snb_uncore_format_group, - .event_descs = snb_uncore_events, }; static struct intel_uncore_type *skl_msr_uncores[] = { &skl_uncore_cbox, &snb_uncore_arb, + &snb_uncore_clockbox, NULL, }; -- 2.11.0