[PATCH] perf/x86/intel/uncore: fix nonexistent clockticks event for client uncore

From: Kan Liang
Date: Mon Dec 12 2016 - 09:03:35 EST


The clockticks event can only be used by the first Cbox pmu. Other
Cboxes don't allow to open clockticks event, eventhough it's announced
via /sys/../events/..

For client uncore, there is only one clocktick fixed counter. Current
kernel code forces that only the first box can access the fixed counter
in uncore_pmu_event_init. But it doesn't take care of the the
attr_groups. All the pmus of same type share the same attr_groups. If
the clockticks event is set for the first box, user can also observe the
event in other boxes.

The clocktick 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 changes with the patch.
clockticks event is removed from Cbox. It will return unsupported, if
uncore_cbox_0/clockticks/ is accessed. User may need to change their
script to use uncore_clock/clockticks/ to instead.

Signed-off-by: Kan Liang <kan.liang@xxxxxxxxx>
---
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 dbaaf7dc..03afeca 100644
--- a/arch/x86/events/intel/uncore.c
+++ b/arch/x86/events/intel/uncore.c
@@ -652,6 +652,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
@@ -675,12 +679,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 a3dcc12..1b037ea 100644
--- a/arch/x86/events/intel/uncore_snb.c
+++ b/arch/x86/events/intel/uncore_snb.c
@@ -117,7 +117,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 */ },
};

@@ -155,17 +155,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 = {
@@ -182,9 +177,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,
};

@@ -229,22 +249,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.4.3