lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 12 Dec 2016 14:49:03 +0000
From:   "Liang, Kan" <kan.liang@...el.com>
To:     Peter Zijlstra <peterz@...radead.org>,
        "Jiri Olsa (jolsa@...nel.org)" <jolsa@...nel.org>,
        "Jiri Olsa (jolsa@...hat.com)" <jolsa@...hat.com>
CC:     Andi Kleen <andi@...stfloor.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Michael Petlan <mpetlan@...hat.com>
Subject: RE: [RFC] perf/x86/intel/uncore: pmu->type->single_fixed question



> I really would prefer to move the thing to its own PMU.

The patch as below creates a new PMU to fix the issue.

Jirka, could you please try the patch on your machine?


Thanks,
Kan
-------
>From 2de8b2eda6b54734e08a608b5fc8c367b94369d3 Mon Sep 17 00:00:00 2001
From: Kan Liang <kan.liang@...el.com>
Date: Mon, 12 Dec 2016 09:03:35 -0500
Subject: [PATCH] perf/x86/intel/uncore: fix nonexistent clockticks event for
 client uncore

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@...el.com>
---
 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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ