linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V5 0/9] perf: Driver specific configuration for PMU
@ 2016-08-11 16:20 Mathieu Poirier
  2016-08-11 16:20 ` [PATCH V5 1/9] tools: Copy the header file needed by perf tools Mathieu Poirier
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-11 16:20 UTC (permalink / raw)
  To: peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel, linux-arm-kernel

This fifth revision mostly address a comment by Peter Z. to do the initial
configuration option parsing in the perf core rather than individual
drivers.  Further parsing is still needed in the PMU since configuration
options are driver specific.  Let's use this as new starting point for
further enhancement (as I am sure some will be needed).

A patch documenting the new ioctl() will be sent out immediately after this
one.

I have also taken the liberty to roll in this set another serie[1] that was
not picked up for the 4.8 cycle.  This is based on mainline 4.8-rc1 since[2]
is still on 4.7.

This patchset adds the possiblity of specifying PMU driver configuration
directly from the perf command line.  Anything that falls within the
event specifiers '/.../' and that is preceded by the '@' symbol is
treated as a configurable.  Two formats are supported, @cfg and
@cfg=config.

For example:

perf record -e some_event/@cfg1/ ...

or

perf record -e some_event/@cfg2=config/ ...

or

perf record -e some_event/@cfg1,@cfg2=config/ ...

The above are all valid configuration and will see the strings 'cfg1'
and 'cfg2=config' sent to the PMU driver for parsing and interpretation
using the existing ioctl() mechanism.

The primary customers for this feature are the CoreSight drivers where
the selection of a sink (where trace data is accumulated) needs to be
done in a previous, and separated step, from the launching of the perf
command.

As such something that used to be a two-step process:

# echo 1 > /sys/bus/coresight/devices/20070000.etr/enable_sink
# perf record -e cs_etm//u --per-thread  uname

is integrated in a single command:

# perf record -e cs_etm/@sink=20070000.etr/u --per-thread  uname

Thanks,
Mathieu

Changes for V5:
- Made commit log in 5/9 more descriptive.
- Addressed missing return code in builtin-top.c.
- Overhauled the kernel portion to do parsing in the core.

Changes for V4:
- Pushing PMU driver configuration for 'perf top'.
- Rebased to the latest perf/core branch[1]. 

Changes for V3:
- Added comment for function drv_str() that explains the reason for
  keeping the entire token intact.
- Added driver config terms to the existing list of config terms.
- Added documenation for driver specific configuration.
- Pushing PMU driver configuration for 'perf stat' as well.  
- Preventing users from selecting a sink from sysFS _and_ perf.

Changes for V2:
- Rebased to [1] as per Jiri's request.

[1]. https://lkml.org/lkml/2016/7/20/519
[2]. git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf/core

Mathieu Poirier (9):
  tools: Copy the header file needed by perf tools
  perf tools: making coresight PMU listable
  perf tools: adding coresight etm PMU record capabilities
  perf/core: Adding PMU driver specific configuration
  perf: Passing struct perf_event to function setup_aux()
  perf tools: add infrastructure for PMU specific configuration
  perf tools: pushing driver configuration down to the kernel
  coresight: adding sink parameter to function coresight_build_path()
  coresight: etm-perf: incorporating sink definition from cmd line

 MAINTAINERS                                      |   5 +
 arch/x86/events/intel/bts.c                      |   4 +-
 arch/x86/events/intel/pt.c                       |   5 +-
 drivers/hwtracing/coresight/coresight-etm-perf.c | 143 +++++-
 drivers/hwtracing/coresight/coresight-priv.h     |   3 +-
 drivers/hwtracing/coresight/coresight.c          |  43 +-
 include/linux/perf_event.h                       |  33 +-
 include/uapi/linux/perf_event.h                  |   1 +
 kernel/events/core.c                             | 179 +++++++-
 kernel/events/ring_buffer.c                      |   2 +-
 tools/include/linux/coresight-pmu.h              |  39 ++
 tools/include/uapi/linux/perf_event.h            |   1 +
 tools/perf/Documentation/perf-record.txt         |  12 +
 tools/perf/MANIFEST                              |   1 +
 tools/perf/Makefile.config                       |  11 +-
 tools/perf/Makefile.perf                         |   3 +
 tools/perf/arch/arm/util/Build                   |   2 +
 tools/perf/arch/arm/util/auxtrace.c              |  54 +++
 tools/perf/arch/arm/util/cs-etm.c                | 559 +++++++++++++++++++++++
 tools/perf/arch/arm/util/cs-etm.h                |  23 +
 tools/perf/arch/arm/util/pmu.c                   |  34 ++
 tools/perf/arch/arm64/util/Build                 |   4 +
 tools/perf/builtin-record.c                      |   9 +
 tools/perf/builtin-stat.c                        |   8 +
 tools/perf/builtin-top.c                         |  12 +
 tools/perf/util/auxtrace.c                       |   1 +
 tools/perf/util/auxtrace.h                       |   1 +
 tools/perf/util/cs-etm.h                         |  74 +++
 tools/perf/util/evlist.c                         |  21 +
 tools/perf/util/evlist.h                         |   3 +
 tools/perf/util/evsel.c                          |  24 +
 tools/perf/util/evsel.h                          |   5 +
 tools/perf/util/parse-events.c                   |   7 +-
 tools/perf/util/parse-events.h                   |   1 +
 tools/perf/util/parse-events.l                   |  22 +
 tools/perf/util/parse-events.y                   |  11 +
 36 files changed, 1331 insertions(+), 29 deletions(-)
 create mode 100644 tools/include/linux/coresight-pmu.h
 create mode 100644 tools/perf/arch/arm/util/auxtrace.c
 create mode 100644 tools/perf/arch/arm/util/cs-etm.c
 create mode 100644 tools/perf/arch/arm/util/cs-etm.h
 create mode 100644 tools/perf/arch/arm/util/pmu.c
 create mode 100644 tools/perf/util/cs-etm.h

-- 
2.7.4

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH V5 1/9] tools: Copy the header file needed by perf tools
  2016-08-11 16:20 [PATCH V5 0/9] perf: Driver specific configuration for PMU Mathieu Poirier
@ 2016-08-11 16:20 ` Mathieu Poirier
  2016-08-24  9:23   ` [tip:perf/core] tools: Copy coresight-pmu.h " tip-bot for Mathieu Poirier
  2016-08-11 16:20 ` [PATCH V5 2/9] perf tools: making coresight PMU listable Mathieu Poirier
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-11 16:20 UTC (permalink / raw)
  To: peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel,
	linux-arm-kernel, Mathieu Poirier

Directly accessing kernel files is not allowed anymore.  As such
making file coresight-pmu.h accessible by the perf tools and complain
if this copy strays from the one found in the main kernel tree.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 tools/include/linux/coresight-pmu.h | 39 +++++++++++++++++++++++++++++++++++++
 tools/perf/MANIFEST                 |  1 +
 tools/perf/Makefile.perf            |  3 +++
 3 files changed, 43 insertions(+)
 create mode 100644 tools/include/linux/coresight-pmu.h

diff --git a/tools/include/linux/coresight-pmu.h b/tools/include/linux/coresight-pmu.h
new file mode 100644
index 000000000000..7d410260661b
--- /dev/null
+++ b/tools/include/linux/coresight-pmu.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright(C) 2015 Linaro Limited. All rights reserved.
+ * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _LINUX_CORESIGHT_PMU_H
+#define _LINUX_CORESIGHT_PMU_H
+
+#define CORESIGHT_ETM_PMU_NAME "cs_etm"
+#define CORESIGHT_ETM_PMU_SEED  0x10
+
+/* ETMv3.5/PTM's ETMCR config bit */
+#define ETM_OPT_CYCACC  12
+#define ETM_OPT_TS      28
+
+static inline int coresight_get_trace_id(int cpu)
+{
+	/*
+	 * A trace ID of value 0 is invalid, so let's start at some
+	 * random value that fits in 7 bits and go from there.  Since
+	 * the common convention is to have data trace IDs be I(N) + 1,
+	 * set instruction trace IDs as a function of the CPU number.
+	 */
+	return (CORESIGHT_ETM_PMU_SEED + (cpu * 2));
+}
+
+#endif
diff --git a/tools/perf/MANIFEST b/tools/perf/MANIFEST
index ad2534df4ba6..f181c613d9aa 100644
--- a/tools/perf/MANIFEST
+++ b/tools/perf/MANIFEST
@@ -77,4 +77,5 @@ tools/include/linux/stringify.h
 tools/include/linux/types.h
 tools/include/linux/err.h
 tools/include/linux/bitmap.h
+tools/include/linux/coresight-pmu.h
 tools/arch/*/include/uapi/asm/perf_regs.h
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 2d9087501633..aa7ab23fbc9f 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -429,6 +429,9 @@ $(PERF_IN): prepare FORCE
 	@(test -f ../../include/asm-generic/bitops/fls64.h && ( \
         (diff -B ../include/asm-generic/bitops/fls64.h ../../include/asm-generic/bitops/fls64.h >/dev/null) \
         || echo "Warning: tools/include/asm-generic/bitops/fls64.h differs from kernel" >&2 )) || true
+	@(test -f ../../include/linux/coresight-pmu.h && ( \
+	(diff -B ../include/linux/coresight-pmu.h ../../include/linux/coresight-pmu.h >/dev/null) \
+	|| echo "Warning: tools/include/linux/coresight-pmu.h differs from kernel" >&2 )) || true
 	$(Q)$(MAKE) $(build)=perf
 
 $(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(LIBTRACEEVENT_DYNAMIC_LIST)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V5 2/9] perf tools: making coresight PMU listable
  2016-08-11 16:20 [PATCH V5 0/9] perf: Driver specific configuration for PMU Mathieu Poirier
  2016-08-11 16:20 ` [PATCH V5 1/9] tools: Copy the header file needed by perf tools Mathieu Poirier
@ 2016-08-11 16:20 ` Mathieu Poirier
  2016-08-11 16:20 ` [PATCH V5 3/9] perf tools: adding coresight etm PMU record capabilities Mathieu Poirier
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-11 16:20 UTC (permalink / raw)
  To: peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel,
	linux-arm-kernel, Mathieu Poirier

Adding the required mechanic allowing 'perf list pmu' to
discover coresight ETM/PTM tracers.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 MAINTAINERS                    |  1 +
 tools/perf/Makefile.config     | 11 +++++++----
 tools/perf/arch/arm/util/Build |  2 ++
 tools/perf/arch/arm/util/pmu.c | 34 ++++++++++++++++++++++++++++++++++
 4 files changed, 44 insertions(+), 4 deletions(-)
 create mode 100644 tools/perf/arch/arm/util/pmu.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 20bb1d00098c..3b21796981f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1112,6 +1112,7 @@ F:	drivers/hwtracing/coresight/*
 F:	Documentation/trace/coresight.txt
 F:	Documentation/devicetree/bindings/arm/coresight.txt
 F:	Documentation/ABI/testing/sysfs-bus-coresight-devices-*
+F:	tools/perf/arch/arm/util/pmu.c
 
 ARM/CORGI MACHINE SUPPORT
 M:	Richard Purdie <rpurdie@rpsys.net>
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 24803c58049a..72edf83d76b7 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -746,10 +746,13 @@ ifdef LIBBABELTRACE
 endif
 
 ifndef NO_AUXTRACE
-  ifeq ($(feature-get_cpuid), 0)
-    msg := $(warning Your gcc lacks the __get_cpuid() builtin, disables support for auxtrace/Intel PT, please install a newer gcc);
-    NO_AUXTRACE := 1
-  else
+  ifeq ($(ARCH),x86)
+    ifeq ($(feature-get_cpuid), 0)
+      msg := $(warning Your gcc lacks the __get_cpuid() builtin, disables support for auxtrace/Intel PT, please install a newer gcc);
+      NO_AUXTRACE := 1
+    endif
+  endif
+  ifndef NO_AUXTRACE
     $(call detected,CONFIG_AUXTRACE)
     CFLAGS += -DHAVE_AUXTRACE_SUPPORT
   endif
diff --git a/tools/perf/arch/arm/util/Build b/tools/perf/arch/arm/util/Build
index f98da17357c0..4093fd146f46 100644
--- a/tools/perf/arch/arm/util/Build
+++ b/tools/perf/arch/arm/util/Build
@@ -2,3 +2,5 @@ libperf-$(CONFIG_DWARF) += dwarf-regs.o
 
 libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
 libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
+
+libperf-$(CONFIG_AUXTRACE) += pmu.o
diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c
new file mode 100644
index 000000000000..af9fb666b44f
--- /dev/null
+++ b/tools/perf/arch/arm/util/pmu.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright(C) 2015 Linaro Limited. All rights reserved.
+ * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <string.h>
+#include <linux/coresight-pmu.h>
+#include <linux/perf_event.h>
+
+#include "../../util/pmu.h"
+
+struct perf_event_attr
+*perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
+{
+#ifdef HAVE_AUXTRACE_SUPPORT
+	if (!strcmp(pmu->name, CORESIGHT_ETM_PMU_NAME)) {
+		/* add ETM default config here */
+		pmu->selectable = true;
+	}
+#endif
+	return NULL;
+}
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V5 3/9] perf tools: adding coresight etm PMU record capabilities
  2016-08-11 16:20 [PATCH V5 0/9] perf: Driver specific configuration for PMU Mathieu Poirier
  2016-08-11 16:20 ` [PATCH V5 1/9] tools: Copy the header file needed by perf tools Mathieu Poirier
  2016-08-11 16:20 ` [PATCH V5 2/9] perf tools: making coresight PMU listable Mathieu Poirier
@ 2016-08-11 16:20 ` Mathieu Poirier
  2016-08-11 16:20 ` [PATCH V5 4/9] perf/core: Adding PMU driver specific configuration Mathieu Poirier
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-11 16:20 UTC (permalink / raw)
  To: peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel,
	linux-arm-kernel, Mathieu Poirier

Coresight ETMs are IP blocks used to perform HW assisted tracing
on a CPU core.  This patch introduce the required auxiliary API
functions allowing the perf core to interact with a tracer.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 MAINTAINERS                         |   4 +
 tools/perf/arch/arm/util/Build      |   2 +-
 tools/perf/arch/arm/util/auxtrace.c |  54 ++++
 tools/perf/arch/arm/util/cs-etm.c   | 559 ++++++++++++++++++++++++++++++++++++
 tools/perf/arch/arm/util/cs-etm.h   |  23 ++
 tools/perf/arch/arm64/util/Build    |   4 +
 tools/perf/util/auxtrace.c          |   1 +
 tools/perf/util/auxtrace.h          |   1 +
 tools/perf/util/cs-etm.h            |  74 +++++
 9 files changed, 721 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/arch/arm/util/auxtrace.c
 create mode 100644 tools/perf/arch/arm/util/cs-etm.c
 create mode 100644 tools/perf/arch/arm/util/cs-etm.h
 create mode 100644 tools/perf/util/cs-etm.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 3b21796981f1..b7ab27d47e9f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1113,6 +1113,10 @@ F:	Documentation/trace/coresight.txt
 F:	Documentation/devicetree/bindings/arm/coresight.txt
 F:	Documentation/ABI/testing/sysfs-bus-coresight-devices-*
 F:	tools/perf/arch/arm/util/pmu.c
+F:	tools/perf/arch/arm/util/auxtrace.c
+F:	tools/perf/arch/arm/util/cs-etm.c
+F:	tools/perf/arch/arm/util/cs-etm.h
+F:	tools/perf/util/cs-etm.h
 
 ARM/CORGI MACHINE SUPPORT
 M:	Richard Purdie <rpurdie@rpsys.net>
diff --git a/tools/perf/arch/arm/util/Build b/tools/perf/arch/arm/util/Build
index 4093fd146f46..e64c5f216448 100644
--- a/tools/perf/arch/arm/util/Build
+++ b/tools/perf/arch/arm/util/Build
@@ -3,4 +3,4 @@ libperf-$(CONFIG_DWARF) += dwarf-regs.o
 libperf-$(CONFIG_LOCAL_LIBUNWIND)    += unwind-libunwind.o
 libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
 
-libperf-$(CONFIG_AUXTRACE) += pmu.o
+libperf-$(CONFIG_AUXTRACE) += pmu.o auxtrace.o cs-etm.o
diff --git a/tools/perf/arch/arm/util/auxtrace.c b/tools/perf/arch/arm/util/auxtrace.c
new file mode 100644
index 000000000000..8edf2cb71564
--- /dev/null
+++ b/tools/perf/arch/arm/util/auxtrace.c
@@ -0,0 +1,54 @@
+/*
+ * Copyright(C) 2015 Linaro Limited. All rights reserved.
+ * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdbool.h>
+#include <linux/coresight-pmu.h>
+
+#include "../../util/auxtrace.h"
+#include "../../util/evlist.h"
+#include "../../util/pmu.h"
+#include "cs-etm.h"
+
+struct auxtrace_record
+*auxtrace_record__init(struct perf_evlist *evlist, int *err)
+{
+	struct perf_pmu	*cs_etm_pmu;
+	struct perf_evsel *evsel;
+	bool found_etm = false;
+
+	cs_etm_pmu = perf_pmu__find(CORESIGHT_ETM_PMU_NAME);
+
+	if (evlist) {
+		evlist__for_each_entry(evlist, evsel) {
+			if (cs_etm_pmu &&
+			    evsel->attr.type == cs_etm_pmu->type)
+				found_etm = true;
+		}
+	}
+
+	if (found_etm)
+		return cs_etm_record_init(err);
+
+	/*
+	 * Clear 'err' even if we haven't found a cs_etm event - that way perf
+	 * record can still be used even if tracers aren't present.  The NULL
+	 * return value will take care of telling the infrastructure HW tracing
+	 * isn't available.
+	 */
+	*err = 0;
+	return NULL;
+}
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
new file mode 100644
index 000000000000..829c479614f1
--- /dev/null
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -0,0 +1,559 @@
+/*
+ * Copyright(C) 2015 Linaro Limited. All rights reserved.
+ * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <api/fs/fs.h>
+#include <linux/bitops.h>
+#include <linux/coresight-pmu.h>
+#include <linux/kernel.h>
+#include <linux/log2.h>
+#include <linux/types.h>
+
+#include "cs-etm.h"
+#include "../../perf.h"
+#include "../../util/auxtrace.h"
+#include "../../util/cpumap.h"
+#include "../../util/evlist.h"
+#include "../../util/pmu.h"
+#include "../../util/thread_map.h"
+#include "../../util/cs-etm.h"
+
+#include <stdlib.h>
+
+struct cs_etm_recording {
+	struct auxtrace_record	itr;
+	struct perf_pmu		*cs_etm_pmu;
+	struct perf_evlist	*evlist;
+	bool			snapshot_mode;
+	size_t			snapshot_size;
+};
+
+static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu);
+
+static int cs_etm_parse_snapshot_options(struct auxtrace_record *itr,
+					 struct record_opts *opts,
+					 const char *str)
+{
+	struct cs_etm_recording *ptr =
+				container_of(itr, struct cs_etm_recording, itr);
+	unsigned long long snapshot_size = 0;
+	char *endptr;
+
+	if (str) {
+		snapshot_size = strtoull(str, &endptr, 0);
+		if (*endptr || snapshot_size > SIZE_MAX)
+			return -1;
+	}
+
+	opts->auxtrace_snapshot_mode = true;
+	opts->auxtrace_snapshot_size = snapshot_size;
+	ptr->snapshot_size = snapshot_size;
+
+	return 0;
+}
+
+static int cs_etm_recording_options(struct auxtrace_record *itr,
+				    struct perf_evlist *evlist,
+				    struct record_opts *opts)
+{
+	struct cs_etm_recording *ptr =
+				container_of(itr, struct cs_etm_recording, itr);
+	struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
+	struct perf_evsel *evsel, *cs_etm_evsel = NULL;
+	const struct cpu_map *cpus = evlist->cpus;
+	bool privileged = (geteuid() == 0 || perf_event_paranoid() < 0);
+
+	ptr->evlist = evlist;
+	ptr->snapshot_mode = opts->auxtrace_snapshot_mode;
+
+	evlist__for_each_entry(evlist, evsel) {
+		if (evsel->attr.type == cs_etm_pmu->type) {
+			if (cs_etm_evsel) {
+				pr_err("There may be only one %s event\n",
+				       CORESIGHT_ETM_PMU_NAME);
+				return -EINVAL;
+			}
+			evsel->attr.freq = 0;
+			evsel->attr.sample_period = 1;
+			cs_etm_evsel = evsel;
+			opts->full_auxtrace = true;
+		}
+	}
+
+	/* no need to continue if at least one event of interest was found */
+	if (!cs_etm_evsel)
+		return 0;
+
+	if (opts->use_clockid) {
+		pr_err("Cannot use clockid (-k option) with %s\n",
+		       CORESIGHT_ETM_PMU_NAME);
+		return -EINVAL;
+	}
+
+	/* we are in snapshot mode */
+	if (opts->auxtrace_snapshot_mode) {
+		/*
+		 * No size were given to '-S' or '-m,', so go with
+		 * the default
+		 */
+		if (!opts->auxtrace_snapshot_size &&
+		    !opts->auxtrace_mmap_pages) {
+			if (privileged) {
+				opts->auxtrace_mmap_pages = MiB(4) / page_size;
+			} else {
+				opts->auxtrace_mmap_pages =
+							KiB(128) / page_size;
+				if (opts->mmap_pages == UINT_MAX)
+					opts->mmap_pages = KiB(256) / page_size;
+			}
+		} else if (!opts->auxtrace_mmap_pages && !privileged &&
+						opts->mmap_pages == UINT_MAX) {
+			opts->mmap_pages = KiB(256) / page_size;
+		}
+
+		/*
+		 * '-m,xyz' was specified but no snapshot size, so make the
+		 * snapshot size as big as the auxtrace mmap area.
+		 */
+		if (!opts->auxtrace_snapshot_size) {
+			opts->auxtrace_snapshot_size =
+				opts->auxtrace_mmap_pages * (size_t)page_size;
+		}
+
+		/*
+		 * -Sxyz was specified but no auxtrace mmap area, so make the
+		 * auxtrace mmap area big enough to fit the requested snapshot
+		 * size.
+		 */
+		if (!opts->auxtrace_mmap_pages) {
+			size_t sz = opts->auxtrace_snapshot_size;
+
+			sz = round_up(sz, page_size) / page_size;
+			opts->auxtrace_mmap_pages = roundup_pow_of_two(sz);
+		}
+
+		/* Snapshost size can't be bigger than the auxtrace area */
+		if (opts->auxtrace_snapshot_size >
+				opts->auxtrace_mmap_pages * (size_t)page_size) {
+			pr_err("Snapshot size %zu must not be greater than AUX area tracing mmap size %zu\n",
+			       opts->auxtrace_snapshot_size,
+			       opts->auxtrace_mmap_pages * (size_t)page_size);
+			return -EINVAL;
+		}
+
+		/* Something went wrong somewhere - this shouldn't happen */
+		if (!opts->auxtrace_snapshot_size ||
+		    !opts->auxtrace_mmap_pages) {
+			pr_err("Failed to calculate default snapshot size and/or AUX area tracing mmap pages\n");
+			return -EINVAL;
+		}
+	}
+
+	/* We are in full trace mode but '-m,xyz' wasn't specified */
+	if (opts->full_auxtrace && !opts->auxtrace_mmap_pages) {
+		if (privileged) {
+			opts->auxtrace_mmap_pages = MiB(4) / page_size;
+		} else {
+			opts->auxtrace_mmap_pages = KiB(128) / page_size;
+			if (opts->mmap_pages == UINT_MAX)
+				opts->mmap_pages = KiB(256) / page_size;
+		}
+
+	}
+
+	/* Validate auxtrace_mmap_pages provided by user */
+	if (opts->auxtrace_mmap_pages) {
+		unsigned int max_page = (KiB(128) / page_size);
+		size_t sz = opts->auxtrace_mmap_pages * (size_t)page_size;
+
+		if (!privileged &&
+		    opts->auxtrace_mmap_pages > max_page) {
+			opts->auxtrace_mmap_pages = max_page;
+			pr_err("auxtrace too big, truncating to %d\n",
+			       max_page);
+		}
+
+		if (!is_power_of_2(sz)) {
+			pr_err("Invalid mmap size for %s: must be a power of 2\n",
+			       CORESIGHT_ETM_PMU_NAME);
+			return -EINVAL;
+		}
+	}
+
+	if (opts->auxtrace_snapshot_mode)
+		pr_debug2("%s snapshot size: %zu\n", CORESIGHT_ETM_PMU_NAME,
+			  opts->auxtrace_snapshot_size);
+
+	if (cs_etm_evsel) {
+		/*
+		 * To obtain the auxtrace buffer file descriptor, the auxtrace
+		 * event must come first.
+		 */
+		perf_evlist__to_front(evlist, cs_etm_evsel);
+		/*
+		 * In the case of per-cpu mmaps, we need the CPU on the
+		 * AUX event.
+		 */
+		if (!cpu_map__empty(cpus))
+			perf_evsel__set_sample_bit(cs_etm_evsel, CPU);
+	}
+
+	/* Add dummy event to keep tracking */
+	if (opts->full_auxtrace) {
+		struct perf_evsel *tracking_evsel;
+		int err;
+
+		err = parse_events(evlist, "dummy:u", NULL);
+		if (err)
+			return err;
+
+		tracking_evsel = perf_evlist__last(evlist);
+		perf_evlist__set_tracking_event(evlist, tracking_evsel);
+
+		tracking_evsel->attr.freq = 0;
+		tracking_evsel->attr.sample_period = 1;
+
+		/* In per-cpu case, always need the time of mmap events etc */
+		if (!cpu_map__empty(cpus))
+			perf_evsel__set_sample_bit(tracking_evsel, TIME);
+	}
+
+	return 0;
+}
+
+static u64 cs_etm_get_config(struct auxtrace_record *itr)
+{
+	u64 config = 0;
+	struct cs_etm_recording *ptr =
+			container_of(itr, struct cs_etm_recording, itr);
+	struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
+	struct perf_evlist *evlist = ptr->evlist;
+	struct perf_evsel *evsel;
+
+	evlist__for_each_entry(evlist, evsel) {
+		if (evsel->attr.type == cs_etm_pmu->type) {
+			/*
+			 * Variable perf_event_attr::config is assigned to
+			 * ETMv3/PTM.  The bit fields have been made to match
+			 * the ETMv3.5 ETRMCR register specification.  See the
+			 * PMU_FORMAT_ATTR() declarations in
+			 * drivers/hwtracing/coresight/coresight-perf.c for
+			 * details.
+			 */
+			config = evsel->attr.config;
+			break;
+		}
+	}
+
+	return config;
+}
+
+static size_t
+cs_etm_info_priv_size(struct auxtrace_record *itr __maybe_unused,
+		      struct perf_evlist *evlist __maybe_unused)
+{
+	int i;
+	int etmv3 = 0, etmv4 = 0;
+	const struct cpu_map *cpus = evlist->cpus;
+
+	/* cpu map is not empty, we have specific CPUs to work with */
+	if (!cpu_map__empty(cpus)) {
+		for (i = 0; i < cpu_map__nr(cpus); i++) {
+			if (cs_etm_is_etmv4(itr, cpus->map[i]))
+				etmv4++;
+			else
+				etmv3++;
+		}
+	} else {
+		/* get configuration for all CPUs in the system */
+		for (i = 0; i < cpu__max_cpu(); i++) {
+			if (cs_etm_is_etmv4(itr, i))
+				etmv4++;
+			else
+				etmv3++;
+		}
+	}
+
+	return (CS_ETM_HEADER_SIZE +
+	       (etmv4 * CS_ETMV4_PRIV_SIZE) +
+	       (etmv3 * CS_ETMV3_PRIV_SIZE));
+}
+
+static const char *metadata_etmv3_ro[CS_ETM_PRIV_MAX] = {
+	[CS_ETM_ETMCCER]	= "mgmt/etmccer",
+	[CS_ETM_ETMIDR]		= "mgmt/etmidr",
+};
+
+static const char *metadata_etmv4_ro[CS_ETMV4_PRIV_MAX] = {
+	[CS_ETMV4_TRCIDR0]		= "trcidr/trcidr0",
+	[CS_ETMV4_TRCIDR1]		= "trcidr/trcidr1",
+	[CS_ETMV4_TRCIDR2]		= "trcidr/trcidr2",
+	[CS_ETMV4_TRCIDR8]		= "trcidr/trcidr8",
+	[CS_ETMV4_TRCAUTHSTATUS]	= "mgmt/trcauthstatus",
+};
+
+static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu)
+{
+	bool ret = false;
+	char path[PATH_MAX];
+	int scan;
+	unsigned int val;
+	struct cs_etm_recording *ptr =
+			container_of(itr, struct cs_etm_recording, itr);
+	struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
+
+	/* Take any of the RO files for ETMv4 and see if it present */
+	snprintf(path, PATH_MAX, "cpu%d/%s",
+		 cpu, metadata_etmv4_ro[CS_ETMV4_TRCIDR0]);
+	scan = perf_pmu__scan_file(cs_etm_pmu, path, "%x", &val);
+
+	/* The file was read successfully, we have a winner */
+	if (scan == 1)
+		ret = true;
+
+	return ret;
+}
+
+static int cs_etm_get_ro(struct perf_pmu *pmu, int cpu, const char *path)
+{
+	char pmu_path[PATH_MAX];
+	int scan;
+	unsigned int val = 0;
+
+	/* Get RO metadata from sysfs */
+	snprintf(pmu_path, PATH_MAX, "cpu%d/%s", cpu, path);
+
+	scan = perf_pmu__scan_file(pmu, pmu_path, "%x", &val);
+	if (scan != 1)
+		pr_err("%s: error reading: %s\n", __func__, pmu_path);
+
+	return val;
+}
+
+static void cs_etm_get_metadata(int cpu, u32 *offset,
+				struct auxtrace_record *itr,
+				struct auxtrace_info_event *info)
+{
+	u32 increment;
+	u64 magic;
+	struct cs_etm_recording *ptr =
+			container_of(itr, struct cs_etm_recording, itr);
+	struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
+
+	/* first see what kind of tracer this cpu is affined to */
+	if (cs_etm_is_etmv4(itr, cpu)) {
+		magic = __perf_cs_etmv4_magic;
+		/* Get trace configuration register */
+		info->priv[*offset + CS_ETMV4_TRCCONFIGR] =
+						cs_etm_get_config(itr);
+		/* Get traceID from the framework */
+		info->priv[*offset + CS_ETMV4_TRCTRACEIDR] =
+						coresight_get_trace_id(cpu);
+		/* Get read-only information from sysFS */
+		info->priv[*offset + CS_ETMV4_TRCIDR0] =
+			cs_etm_get_ro(cs_etm_pmu, cpu,
+				      metadata_etmv4_ro[CS_ETMV4_TRCIDR0]);
+		info->priv[*offset + CS_ETMV4_TRCIDR1] =
+			cs_etm_get_ro(cs_etm_pmu, cpu,
+				      metadata_etmv4_ro[CS_ETMV4_TRCIDR1]);
+		info->priv[*offset + CS_ETMV4_TRCIDR2] =
+			cs_etm_get_ro(cs_etm_pmu, cpu,
+				      metadata_etmv4_ro[CS_ETMV4_TRCIDR2]);
+		info->priv[*offset + CS_ETMV4_TRCIDR8] =
+			cs_etm_get_ro(cs_etm_pmu, cpu,
+				      metadata_etmv4_ro[CS_ETMV4_TRCIDR8]);
+		info->priv[*offset + CS_ETMV4_TRCAUTHSTATUS] =
+			cs_etm_get_ro(cs_etm_pmu, cpu,
+				      metadata_etmv4_ro
+				      [CS_ETMV4_TRCAUTHSTATUS]);
+
+		/* How much space was used */
+		increment = CS_ETMV4_PRIV_MAX;
+	} else {
+		magic = __perf_cs_etmv3_magic;
+		/* Get configuration register */
+		info->priv[*offset + CS_ETM_ETMCR] = cs_etm_get_config(itr);
+		/* Get traceID from the framework */
+		info->priv[*offset + CS_ETM_ETMTRACEIDR] =
+						coresight_get_trace_id(cpu);
+		/* Get read-only information from sysFS */
+		info->priv[*offset + CS_ETM_ETMCCER] =
+			cs_etm_get_ro(cs_etm_pmu, cpu,
+				      metadata_etmv3_ro[CS_ETM_ETMCCER]);
+		info->priv[*offset + CS_ETM_ETMIDR] =
+			cs_etm_get_ro(cs_etm_pmu, cpu,
+				      metadata_etmv3_ro[CS_ETM_ETMIDR]);
+
+		/* How much space was used */
+		increment = CS_ETM_PRIV_MAX;
+	}
+
+	/* Build generic header portion */
+	info->priv[*offset + CS_ETM_MAGIC] = magic;
+	info->priv[*offset + CS_ETM_CPU] = cpu;
+	/* Where the next CPU entry should start from */
+	*offset += increment;
+}
+
+static int cs_etm_info_fill(struct auxtrace_record *itr,
+			    struct perf_session *session,
+			    struct auxtrace_info_event *info,
+			    size_t priv_size)
+{
+	int i;
+	u32 offset;
+	u64 nr_cpu, type;
+	const struct cpu_map *cpus = session->evlist->cpus;
+	struct cs_etm_recording *ptr =
+			container_of(itr, struct cs_etm_recording, itr);
+	struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
+
+	if (priv_size != cs_etm_info_priv_size(itr, session->evlist))
+		return -EINVAL;
+
+	if (!session->evlist->nr_mmaps)
+		return -EINVAL;
+
+	/* If the cpu_map is empty all CPUs are involved */
+	nr_cpu = cpu_map__empty(cpus) ? cpu__max_cpu() : cpu_map__nr(cpus);
+	/* Get PMU type as dynamically assigned by the core */
+	type = cs_etm_pmu->type;
+
+	/* First fill out the session header */
+	info->type = PERF_AUXTRACE_CS_ETM;
+	info->priv[CS_HEADER_VERSION_0] = 0;
+	info->priv[CS_PMU_TYPE_CPUS] = type << 32;
+	info->priv[CS_PMU_TYPE_CPUS] |= nr_cpu;
+	info->priv[CS_ETM_SNAPSHOT] = ptr->snapshot_mode;
+
+	offset = CS_ETM_SNAPSHOT + 1;
+
+	/* cpu map is not empty, we have specific CPUs to work with */
+	if (!cpu_map__empty(cpus)) {
+		for (i = 0; i < cpu_map__nr(cpus) && offset < priv_size; i++)
+			cs_etm_get_metadata(cpus->map[i], &offset, itr, info);
+	} else {
+		/* get configuration for all CPUs in the system */
+		for (i = 0; i < cpu__max_cpu(); i++)
+			cs_etm_get_metadata(i, &offset, itr, info);
+	}
+
+	return 0;
+}
+
+static int cs_etm_find_snapshot(struct auxtrace_record *itr __maybe_unused,
+				int idx, struct auxtrace_mmap *mm,
+				unsigned char *data __maybe_unused,
+				u64 *head, u64 *old)
+{
+	pr_debug3("%s: mmap index %d old head %zu new head %zu size %zu\n",
+		  __func__, idx, (size_t)*old, (size_t)*head, mm->len);
+
+	*old = *head;
+	*head += mm->len;
+
+	return 0;
+}
+
+static int cs_etm_snapshot_start(struct auxtrace_record *itr)
+{
+	struct cs_etm_recording *ptr =
+			container_of(itr, struct cs_etm_recording, itr);
+	struct perf_evsel *evsel;
+
+	evlist__for_each_entry(ptr->evlist, evsel) {
+		if (evsel->attr.type == ptr->cs_etm_pmu->type)
+			return perf_evsel__disable(evsel);
+	}
+	return -EINVAL;
+}
+
+static int cs_etm_snapshot_finish(struct auxtrace_record *itr)
+{
+	struct cs_etm_recording *ptr =
+			container_of(itr, struct cs_etm_recording, itr);
+	struct perf_evsel *evsel;
+
+	evlist__for_each_entry(ptr->evlist, evsel) {
+		if (evsel->attr.type == ptr->cs_etm_pmu->type)
+			return perf_evsel__enable(evsel);
+	}
+	return -EINVAL;
+}
+
+static u64 cs_etm_reference(struct auxtrace_record *itr __maybe_unused)
+{
+	return (((u64) rand() <<  0) & 0x00000000FFFFFFFFull) |
+		(((u64) rand() << 32) & 0xFFFFFFFF00000000ull);
+}
+
+static void cs_etm_recording_free(struct auxtrace_record *itr)
+{
+	struct cs_etm_recording *ptr =
+			container_of(itr, struct cs_etm_recording, itr);
+	free(ptr);
+}
+
+static int cs_etm_read_finish(struct auxtrace_record *itr, int idx)
+{
+	struct cs_etm_recording *ptr =
+			container_of(itr, struct cs_etm_recording, itr);
+	struct perf_evsel *evsel;
+
+	evlist__for_each_entry(ptr->evlist, evsel) {
+		if (evsel->attr.type == ptr->cs_etm_pmu->type)
+			return perf_evlist__enable_event_idx(ptr->evlist,
+							     evsel, idx);
+	}
+
+	return -EINVAL;
+}
+
+struct auxtrace_record *cs_etm_record_init(int *err)
+{
+	struct perf_pmu *cs_etm_pmu;
+	struct cs_etm_recording *ptr;
+
+	cs_etm_pmu = perf_pmu__find(CORESIGHT_ETM_PMU_NAME);
+
+	if (!cs_etm_pmu) {
+		*err = -EINVAL;
+		goto out;
+	}
+
+	ptr = zalloc(sizeof(struct cs_etm_recording));
+	if (!ptr) {
+		*err = -ENOMEM;
+		goto out;
+	}
+
+	ptr->cs_etm_pmu			= cs_etm_pmu;
+	ptr->itr.parse_snapshot_options	= cs_etm_parse_snapshot_options;
+	ptr->itr.recording_options	= cs_etm_recording_options;
+	ptr->itr.info_priv_size		= cs_etm_info_priv_size;
+	ptr->itr.info_fill		= cs_etm_info_fill;
+	ptr->itr.find_snapshot		= cs_etm_find_snapshot;
+	ptr->itr.snapshot_start		= cs_etm_snapshot_start;
+	ptr->itr.snapshot_finish	= cs_etm_snapshot_finish;
+	ptr->itr.reference		= cs_etm_reference;
+	ptr->itr.free			= cs_etm_recording_free;
+	ptr->itr.read_finish		= cs_etm_read_finish;
+
+	*err = 0;
+	return &ptr->itr;
+out:
+	return NULL;
+}
diff --git a/tools/perf/arch/arm/util/cs-etm.h b/tools/perf/arch/arm/util/cs-etm.h
new file mode 100644
index 000000000000..909f486d02d1
--- /dev/null
+++ b/tools/perf/arch/arm/util/cs-etm.h
@@ -0,0 +1,23 @@
+/*
+ * Copyright(C) 2015 Linaro Limited. All rights reserved.
+ * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef INCLUDE__PERF_CS_ETM_H__
+#define INCLUDE__PERF_CS_ETM_H__
+
+struct auxtrace_record *cs_etm_record_init(int *err);
+
+#endif
diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
index 02f41dba4f4f..cef6fb38d17e 100644
--- a/tools/perf/arch/arm64/util/Build
+++ b/tools/perf/arch/arm64/util/Build
@@ -1,2 +1,6 @@
 libperf-$(CONFIG_DWARF)     += dwarf-regs.o
 libperf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o
+
+libperf-$(CONFIG_AUXTRACE) += ../../arm/util/pmu.o \
+			      ../../arm/util/auxtrace.o \
+			      ../../arm/util/cs-etm.o
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index c9169011e55e..c0aba8e839aa 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -892,6 +892,7 @@ int perf_event__process_auxtrace_info(struct perf_tool *tool __maybe_unused,
 		return intel_pt_process_auxtrace_info(event, session);
 	case PERF_AUXTRACE_INTEL_BTS:
 		return intel_bts_process_auxtrace_info(event, session);
+	case PERF_AUXTRACE_CS_ETM:
 	case PERF_AUXTRACE_UNKNOWN:
 	default:
 		return -EINVAL;
diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
index ac5f0d7167e6..09286f193532 100644
--- a/tools/perf/util/auxtrace.h
+++ b/tools/perf/util/auxtrace.h
@@ -41,6 +41,7 @@ enum auxtrace_type {
 	PERF_AUXTRACE_UNKNOWN,
 	PERF_AUXTRACE_INTEL_PT,
 	PERF_AUXTRACE_INTEL_BTS,
+	PERF_AUXTRACE_CS_ETM,
 };
 
 enum itrace_period_type {
diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h
new file mode 100644
index 000000000000..3cc6bc3263fe
--- /dev/null
+++ b/tools/perf/util/cs-etm.h
@@ -0,0 +1,74 @@
+/*
+ * Copyright(C) 2015 Linaro Limited. All rights reserved.
+ * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef INCLUDE__UTIL_PERF_CS_ETM_H__
+#define INCLUDE__UTIL_PERF_CS_ETM_H__
+
+/* Versionning header in case things need tro change in the future.  That way
+ * decoding of old snapshot is still possible.
+ */
+enum {
+	/* Starting with 0x0 */
+	CS_HEADER_VERSION_0,
+	/* PMU->type (32 bit), total # of CPUs (32 bit) */
+	CS_PMU_TYPE_CPUS,
+	CS_ETM_SNAPSHOT,
+	CS_HEADER_VERSION_0_MAX,
+};
+
+/* Beginning of header common to both ETMv3 and V4 */
+enum {
+	CS_ETM_MAGIC,
+	CS_ETM_CPU,
+};
+
+/* ETMv3/PTM metadata */
+enum {
+	/* Dynamic, configurable parameters */
+	CS_ETM_ETMCR = CS_ETM_CPU + 1,
+	CS_ETM_ETMTRACEIDR,
+	/* RO, taken from sysFS */
+	CS_ETM_ETMCCER,
+	CS_ETM_ETMIDR,
+	CS_ETM_PRIV_MAX,
+};
+
+/* ETMv4 metadata */
+enum {
+	/* Dynamic, configurable parameters */
+	CS_ETMV4_TRCCONFIGR = CS_ETM_CPU + 1,
+	CS_ETMV4_TRCTRACEIDR,
+	/* RO, taken from sysFS */
+	CS_ETMV4_TRCIDR0,
+	CS_ETMV4_TRCIDR1,
+	CS_ETMV4_TRCIDR2,
+	CS_ETMV4_TRCIDR8,
+	CS_ETMV4_TRCAUTHSTATUS,
+	CS_ETMV4_PRIV_MAX,
+};
+
+#define KiB(x) ((x) * 1024)
+#define MiB(x) ((x) * 1024 * 1024)
+
+#define CS_ETM_HEADER_SIZE (CS_HEADER_VERSION_0_MAX * sizeof(u64))
+
+static const u64 __perf_cs_etmv3_magic   = 0x3030303030303030ULL;
+static const u64 __perf_cs_etmv4_magic   = 0x4040404040404040ULL;
+#define CS_ETMV3_PRIV_SIZE (CS_ETM_PRIV_MAX * sizeof(u64))
+#define CS_ETMV4_PRIV_SIZE (CS_ETMV4_PRIV_MAX * sizeof(u64))
+
+#endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V5 4/9] perf/core: Adding PMU driver specific configuration
  2016-08-11 16:20 [PATCH V5 0/9] perf: Driver specific configuration for PMU Mathieu Poirier
                   ` (2 preceding siblings ...)
  2016-08-11 16:20 ` [PATCH V5 3/9] perf tools: adding coresight etm PMU record capabilities Mathieu Poirier
@ 2016-08-11 16:20 ` Mathieu Poirier
  2016-08-22 16:18   ` Alexander Shishkin
  2016-08-11 16:21 ` [PATCH V5 5/9] perf: Passing struct perf_event to function setup_aux() Mathieu Poirier
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-11 16:20 UTC (permalink / raw)
  To: peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel,
	linux-arm-kernel, Mathieu Poirier

This patch somewhat mimics the work done on address filters to
add the infrastructure needed to pass PMU specific HW
configuration to the driver before a session starts.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 include/linux/perf_event.h            |  31 ++++++
 include/uapi/linux/perf_event.h       |   1 +
 kernel/events/core.c                  | 179 +++++++++++++++++++++++++++++++++-
 tools/include/uapi/linux/perf_event.h |   1 +
 4 files changed, 211 insertions(+), 1 deletion(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 8ed4326164cc..1ea2028d9848 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -181,6 +181,10 @@ struct hw_perf_event {
 	/* Last sync'ed generation of filters */
 	unsigned long			addr_filters_gen;
 
+	/* HW specific configuration */
+	void				*drv_configs;
+	raw_spinlock_t			drv_configs_lock;
+
 /*
  * hw_perf_event::state flags; used to track the PERF_EF_* state.
  */
@@ -455,6 +459,13 @@ struct pmu {
 	 * Filter events for PMU-specific reasons.
 	 */
 	int (*filter_match)		(struct perf_event *event); /* optional */
+
+	/*
+	 * PMU driver specific configuration.
+	 */
+	int (*set_drv_configs)		(struct perf_event *event,
+					 struct list_head *drv_configs);
+					/* optional */
 };
 
 /**
@@ -492,6 +503,18 @@ struct perf_addr_filters_head {
 };
 
 /**
+ * struct perf_drv_config - PMU specific configuration as specified by users
+ * @entry:	link to the list.
+ * @config:	name of the configuration option.
+ * @option:	value associated to @config as set by users(optional).
+ */
+struct perf_drv_config {
+	struct list_head	entry;
+	char			*config;
+	char			*option;
+};
+
+/**
  * enum perf_event_active_state - the states of a event
  */
 enum perf_event_active_state {
@@ -1194,6 +1217,11 @@ static inline bool has_addr_filter(struct perf_event *event)
 	return event->pmu->nr_addr_filters;
 }
 
+static inline bool has_drv_config(struct perf_event *event)
+{
+	return event->pmu->set_drv_configs;
+}
+
 /*
  * An inherited event uses parent's filters
  */
@@ -1209,6 +1237,9 @@ perf_event_addr_filters(struct perf_event *event)
 }
 
 extern void perf_event_addr_filters_sync(struct perf_event *event);
+extern void *perf_event_drv_configs_set(struct perf_event *event,
+					void *drv_configs);
+extern void *perf_event_drv_configs_get(struct perf_event *event);
 
 extern int perf_output_begin(struct perf_output_handle *handle,
 			     struct perf_event *event, unsigned int size);
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index c66a485a24ac..90fbc5fd3925 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -407,6 +407,7 @@ struct perf_event_attr {
 #define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
 #define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
 #define PERF_EVENT_IOC_PAUSE_OUTPUT	_IOW('$', 9, __u32)
+#define PERF_EVENT_IOC_SET_DRV_CONFIGS	_IOW('$', 10, char *)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index b5022836649a..c5c66cc7cd8c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4457,6 +4457,8 @@ static int perf_event_set_output(struct perf_event *event,
 				 struct perf_event *output_event);
 static int perf_event_set_filter(struct perf_event *event, void __user *arg);
 static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd);
+static int perf_event_set_drv_configs(struct perf_event *event,
+				      void __user *arg);
 
 static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned long arg)
 {
@@ -4526,6 +4528,10 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
 		rcu_read_unlock();
 		return 0;
 	}
+
+	case PERF_EVENT_IOC_SET_DRV_CONFIGS:
+		return perf_event_set_drv_configs(event, (void __user *)arg);
+
 	default:
 		return -ENOTTY;
 	}
@@ -4558,6 +4564,7 @@ static long perf_compat_ioctl(struct file *file, unsigned int cmd,
 	switch (_IOC_NR(cmd)) {
 	case _IOC_NR(PERF_EVENT_IOC_SET_FILTER):
 	case _IOC_NR(PERF_EVENT_IOC_ID):
+	case _IOC_NR(PERF_EVENT_IOC_SET_DRV_CONFIGS):
 		/* Fix up pointer size (usually 4 -> 8 in 32-on-64-bit case */
 		if (_IOC_SIZE(cmd) == sizeof(compat_uptr_t)) {
 			cmd &= ~IOCSIZE_MASK;
@@ -8044,10 +8051,180 @@ static int perf_event_set_filter(struct perf_event *event, void __user *arg)
 	return ret;
 }
 
+static struct perf_drv_config *
+perf_drv_config_new(int cpu, struct list_head *drv_config_list)
+{
+	int node = cpu_to_node(cpu == -1 ? 0 : cpu);
+	struct perf_drv_config *drv_config;
+
+	drv_config = kzalloc_node(sizeof(*drv_config), GFP_KERNEL, node);
+	if (!drv_config)
+		return ERR_PTR(-ENOMEM);
+
+	INIT_LIST_HEAD(&drv_config->entry);
+	list_add_tail(&drv_config->entry, drv_config_list);
+
+	return drv_config;
+}
+
+static void free_drv_config_list(struct list_head *drv_config_list)
+{
+	struct perf_drv_config *drv_config, *itr;
+
+	list_for_each_entry_safe(drv_config, itr, drv_config_list, entry) {
+		list_del(&drv_config->entry);
+		kfree(drv_config->config);
+		kfree(drv_config->option);
+		kfree(drv_config);
+	}
+}
+
+/* How long does a configuration option really need to be?  */
+#define PERF_DRV_CONFIG_MAX	128
+
 /*
- * hrtimer based swevent callback
+ * PMU specific driver configuration as specified from user space.
+ * The data come in the form of an ascii string pushed down to the kernel
+ * using an ioctl() call.
+ *
+ * Two format are accepted: a singleton and in pairs.  All of the following
+ * are valid: cfg1, cfg2=config2, cfg3=anything_is_possible.
+ *
+ * It is up to each PMU driver to make sure they can work with the
+ * submitted configurables.
  */
+static int
+perf_event_parse_drv_config(struct perf_event *event, char *options,
+			    struct list_head *drv_config_list)
+{
+	char *token;
+	int ret;
+	struct perf_drv_config *drv_config;
+
+	/*
+	 * First split the @options string in nibbles.  Using the above
+	 * example "cfg1", "cfg2=option2" and "cfg3=anything_is_possible"
+	 * will be processed.
+	 */
+	while ((token = strsep(&options, ",")) != NULL) {
+		char *nibble, *config, *option;
+
+		if (!*token)
+			continue;
+
+		/* Allocate a new driver config structure and queue it. */
+		drv_config = perf_drv_config_new(event->cpu, drv_config_list);
+		if (IS_ERR(drv_config)) {
+			ret = PTR_ERR(drv_config);
+			goto fail;
+		}
+
+		/*
+		 * The nibbles are either a "config" or a "config=option"
+		 * pair.  First get the config part.  Since strsep() sets
+		 * @nibble to the next valid token, nibble will be equal to
+		 * the option part or NULL after the first call.
+		 */
+		nibble = token;
+		config = strsep(&nibble, "=");
+		option = nibble;
+
+		/* This shouldn't be happening */
+		if (!config) {
+			ret = -EINVAL;
+			goto fail;
+		}
+
+		drv_config->config = kstrndup(config,
+					      PERF_DRV_CONFIG_MAX, GFP_KERNEL);
+		if (!drv_config->config) {
+			ret = -ENOMEM;
+			goto fail;
+		}
+
+		if (option) {
+			drv_config->option = kstrndup(option,
+						      PERF_DRV_CONFIG_MAX,
+						      GFP_KERNEL);
+			if (!drv_config->option) {
+				ret = -ENOMEM;
+				goto fail;
+			}
+		}
+	}
 
+	return 0;
+
+fail:
+	free_drv_config_list(drv_config_list);
+	return ret;
+}
+
+static int
+perf_event_set_drv_configs(struct perf_event *event, void __user *arg)
+{
+	char *drv_config_str;
+	int ret = 0;
+	LIST_HEAD(drv_config_list);
+
+	/*
+	 * No point in going further if the PMU driver doesn't support
+	 * cmd line configuration.
+	 */
+	if (!has_drv_config(event))
+		return -EINVAL;
+
+	/* Get a handle on user specified configuration */
+	drv_config_str = strndup_user(arg, PAGE_SIZE);
+	if (IS_ERR(drv_config_str))
+		return PTR_ERR(drv_config_str);
+
+	/* Make sure we have the right format */
+	ret = perf_event_parse_drv_config(event,
+					  drv_config_str, &drv_config_list);
+	if (ret)
+		goto out;
+
+	ret = event->pmu->set_drv_configs(event, &drv_config_list);
+	if (ret)
+		goto out;
+
+out:
+	kfree(drv_config_str);
+	free_drv_config_list(&drv_config_list);
+	return ret;
+}
+
+void *perf_event_drv_configs_set(struct perf_event *event, void *drv_configs)
+{
+	unsigned long flags;
+	void *old_drv_configs;
+
+	if (!event)
+		return ERR_PTR(-EINVAL);
+
+	raw_spin_lock_irqsave(&event->hw.drv_configs_lock, flags);
+	old_drv_configs = event->hw.drv_configs;
+	event->hw.drv_configs = drv_configs;
+	raw_spin_unlock_irqrestore(&event->hw.drv_configs_lock, flags);
+
+	return old_drv_configs;
+}
+EXPORT_SYMBOL_GPL(perf_event_drv_configs_set);
+
+void *perf_event_drv_configs_get(struct perf_event *event)
+{
+	if (!event)
+		return ERR_PTR(-EINVAL);
+
+	return perf_event_drv_configs_set(event, event->hw.drv_configs);
+
+}
+EXPORT_SYMBOL_GPL(perf_event_drv_configs_get);
+
+/*
+ * hrtimer based swevent callback
+ */
 static enum hrtimer_restart perf_swevent_hrtimer(struct hrtimer *hrtimer)
 {
 	enum hrtimer_restart ret = HRTIMER_RESTART;
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index c66a485a24ac..90fbc5fd3925 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -407,6 +407,7 @@ struct perf_event_attr {
 #define PERF_EVENT_IOC_ID		_IOR('$', 7, __u64 *)
 #define PERF_EVENT_IOC_SET_BPF		_IOW('$', 8, __u32)
 #define PERF_EVENT_IOC_PAUSE_OUTPUT	_IOW('$', 9, __u32)
+#define PERF_EVENT_IOC_SET_DRV_CONFIGS	_IOW('$', 10, char *)
 
 enum perf_event_ioc_flags {
 	PERF_IOC_FLAG_GROUP		= 1U << 0,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V5 5/9] perf: Passing struct perf_event to function setup_aux()
  2016-08-11 16:20 [PATCH V5 0/9] perf: Driver specific configuration for PMU Mathieu Poirier
                   ` (3 preceding siblings ...)
  2016-08-11 16:20 ` [PATCH V5 4/9] perf/core: Adding PMU driver specific configuration Mathieu Poirier
@ 2016-08-11 16:21 ` Mathieu Poirier
  2016-08-11 16:21 ` [PATCH V5 6/9] perf tools: add infrastructure for PMU specific configuration Mathieu Poirier
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-11 16:21 UTC (permalink / raw)
  To: peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel,
	linux-arm-kernel, Mathieu Poirier, Alexander Shishkin

Some information, like driver specific configuration, is found
in the hw_perf_event structure.  As such pass a 'struct perf_event'
to function setup_aux() rather than just the CPU number so that
individual drivers can make the right configuration when setting
up a session.

For example the sink definition for a given CoreSight session can be
embedded in the perf cmd line:

perf record -e cs_etm/@sink=20070000.etr/ ...

The string "sink=20070000.etr" is conveyed to the kernel by way of
of an ioctl() call.  From there is is handed over to the PMU driver
for parsing and processing.  If the format is valid the substring
"20070000.etr" is kept in the hw_perf_event::drv_configs for future
reference.

When pmu::setup_aux() is called information about both the
perf_event::cpu and the hw_perf_event::drv_configs is needed to setup
a path from source to sink.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/events/intel/bts.c                      | 4 +++-
 arch/x86/events/intel/pt.c                       | 5 +++--
 drivers/hwtracing/coresight/coresight-etm-perf.c | 4 ++--
 include/linux/perf_event.h                       | 2 +-
 kernel/events/ring_buffer.c                      | 2 +-
 5 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/arch/x86/events/intel/bts.c b/arch/x86/events/intel/bts.c
index 0a6e393a2e62..98155c2dfcce 100644
--- a/arch/x86/events/intel/bts.c
+++ b/arch/x86/events/intel/bts.c
@@ -68,8 +68,10 @@ static size_t buf_size(struct page *page)
 }
 
 static void *
-bts_buffer_setup_aux(int cpu, void **pages, int nr_pages, bool overwrite)
+bts_buffer_setup_aux(struct perf_event *event, void **pages,
+		     int nr_pages, bool overwrite)
 {
+	int cpu = event->cpu;
 	struct bts_buffer *buf;
 	struct page *page;
 	int node = (cpu == -1) ? cpu : cpu_to_node(cpu);
diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 04bb5fb5a8d7..5178c5de0b19 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -1003,10 +1003,11 @@ static int pt_buffer_init_topa(struct pt_buffer *buf, unsigned long nr_pages,
  * Return:	Our private PT buffer structure.
  */
 static void *
-pt_buffer_setup_aux(int cpu, void **pages, int nr_pages, bool snapshot)
+pt_buffer_setup_aux(struct perf_event *event, void **pages,
+		    int nr_pages, bool snapshot)
 {
 	struct pt_buffer *buf;
-	int node, ret;
+	int node, ret, cpu = event->cpu;
 
 	if (!nr_pages)
 		return NULL;
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 755125f7917f..f4174f36c5a0 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -155,7 +155,7 @@ static void etm_free_aux(void *data)
 	schedule_work(&event_data->work);
 }
 
-static void *etm_setup_aux(int event_cpu, void **pages,
+static void *etm_setup_aux(struct perf_event *event, void **pages,
 			   int nr_pages, bool overwrite)
 {
 	int cpu;
@@ -163,7 +163,7 @@ static void *etm_setup_aux(int event_cpu, void **pages,
 	struct coresight_device *sink;
 	struct etm_event_data *event_data = NULL;
 
-	event_data = alloc_event_data(event_cpu);
+	event_data = alloc_event_data(event->cpu);
 	if (!event_data)
 		return NULL;
 
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 1ea2028d9848..16f797561907 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -421,7 +421,7 @@ struct pmu {
 	/*
 	 * Set up pmu-private data structures for an AUX area
 	 */
-	void *(*setup_aux)		(int cpu, void **pages,
+	void *(*setup_aux)		(struct perf_event *event, void **pages,
 					 int nr_pages, bool overwrite);
 					/* optional */
 
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index ae9b90dc9a5a..56aba90af437 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -616,7 +616,7 @@ int rb_alloc_aux(struct ring_buffer *rb, struct perf_event *event,
 			goto out;
 	}
 
-	rb->aux_priv = event->pmu->setup_aux(event->cpu, rb->aux_pages, nr_pages,
+	rb->aux_priv = event->pmu->setup_aux(event, rb->aux_pages, nr_pages,
 					     overwrite);
 	if (!rb->aux_priv)
 		goto out;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V5 6/9] perf tools: add infrastructure for PMU specific configuration
  2016-08-11 16:20 [PATCH V5 0/9] perf: Driver specific configuration for PMU Mathieu Poirier
                   ` (4 preceding siblings ...)
  2016-08-11 16:21 ` [PATCH V5 5/9] perf: Passing struct perf_event to function setup_aux() Mathieu Poirier
@ 2016-08-11 16:21 ` Mathieu Poirier
  2016-08-11 16:21 ` [PATCH V5 7/9] perf tools: pushing driver configuration down to the kernel Mathieu Poirier
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-11 16:21 UTC (permalink / raw)
  To: peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel,
	linux-arm-kernel, Mathieu Poirier

This patch adds PMU driver specific configuration to the parser
infrastructure by preceding any term with the '@' letter.  As such
doing something like:

perf record -e some_event/@cfg1,@cfg2=config/ ...

will see 'cfg1' and 'cfg2=config' being added to the list of evsel config
terms.  Token 'cfg1' and 'cfg2=config' are not processed in user space
and are meant to be interpreted by the PMU driver.

First the lexer/parser are supplemented with the required definitions to
recognise the driver specific configuration.  From there they are simply
added to the list of event terms.  The bulk of the work is done in
function "parse_events_add_pmu()" where driver config event terms are
added to a new list of driver config terms, which in turn spliced with
the event's new driver configuration list.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/Documentation/perf-record.txt | 12 ++++++++++++
 tools/perf/util/evsel.h                  |  2 ++
 tools/perf/util/parse-events.c           |  7 ++++++-
 tools/perf/util/parse-events.h           |  1 +
 tools/perf/util/parse-events.l           | 22 ++++++++++++++++++++++
 tools/perf/util/parse-events.y           | 11 +++++++++++
 6 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 379a2bed07c0..1a24f4d64328 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -60,6 +60,18 @@ OPTIONS
 	  Note: If user explicitly sets options which conflict with the params,
 	  the value set by the params will be overridden.
 
+	  Also not defined in .../<pmu>/format/* are PMU driver specific
+	  configuration parameters.  Any configuration parameter preceded by
+	  the letter '@' is not interpreted in user space and sent down directly
+	  to the PMU driver.  For example:
+
+	  perf record -e some_event/@cfg1,@cfg2=config/ ...
+
+	  will see 'cfg1' and 'cfg2=config' pushed to the PMU driver associated
+	  with the event for further processing.  There is no restriction on
+	  what the configuration parameters are, as long as their semantic is
+	  understood and supported by the PMU driver.
+
         - a hardware breakpoint event in the form of '\mem:addr[/len][:access]'
           where addr is the address in memory you want to break in.
           Access is the memory access type (read, write, execute) it can
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 4d44129e050b..323806082c58 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -46,6 +46,7 @@ enum {
 	PERF_EVSEL__CONFIG_TERM_INHERIT,
 	PERF_EVSEL__CONFIG_TERM_MAX_STACK,
 	PERF_EVSEL__CONFIG_TERM_OVERWRITE,
+	PERF_EVSEL__CONFIG_TERM_DRV_CFG,
 	PERF_EVSEL__CONFIG_TERM_MAX,
 };
 
@@ -57,6 +58,7 @@ struct perf_evsel_config_term {
 		u64	freq;
 		bool	time;
 		char	*callgraph;
+		char	*drv_cfg;
 		u64	stack_user;
 		int	max_stack;
 		bool	inherit;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 6c913c3914fb..2eb8b1ed4cc8 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -904,6 +904,7 @@ static const char *config_term_names[__PARSE_EVENTS__TERM_TYPE_NR] = {
 	[PARSE_EVENTS__TERM_TYPE_MAX_STACK]		= "max-stack",
 	[PARSE_EVENTS__TERM_TYPE_OVERWRITE]		= "overwrite",
 	[PARSE_EVENTS__TERM_TYPE_NOOVERWRITE]		= "no-overwrite",
+	[PARSE_EVENTS__TERM_TYPE_DRV_CFG]		= "driver-config",
 };
 
 static bool config_term_shrinked;
@@ -1034,7 +1035,8 @@ static int config_term_pmu(struct perf_event_attr *attr,
 			   struct parse_events_term *term,
 			   struct parse_events_error *err)
 {
-	if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER)
+	if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER ||
+	    term->type_term == PARSE_EVENTS__TERM_TYPE_DRV_CFG)
 		/*
 		 * Always succeed for sysfs terms, as we dont know
 		 * at this point what type they need to have.
@@ -1134,6 +1136,9 @@ do {								\
 		case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
 			ADD_CONFIG_TERM(OVERWRITE, overwrite, term->val.num ? 0 : 1);
 			break;
+		case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
+			ADD_CONFIG_TERM(DRV_CFG, drv_cfg, term->val.str);
+			break;
 		default:
 			break;
 		}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index d1edbf8cc66a..8d09a976fca8 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -71,6 +71,7 @@ enum {
 	PARSE_EVENTS__TERM_TYPE_MAX_STACK,
 	PARSE_EVENTS__TERM_TYPE_NOOVERWRITE,
 	PARSE_EVENTS__TERM_TYPE_OVERWRITE,
+	PARSE_EVENTS__TERM_TYPE_DRV_CFG,
 	__PARSE_EVENTS__TERM_TYPE_NR,
 };
 
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 7a2519435da0..9f43fda2570f 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -53,6 +53,26 @@ static int str(yyscan_t scanner, int token)
 	return token;
 }
 
+/*
+ * This function is called when the parser gets two kind of input:
+ *
+ * 	@cfg1 or @cfg2=config
+ *
+ * The leading '@' is stripped off before 'cfg1' and 'cfg2=config' are given to
+ * bison.  In the latter case it is necessary to keep the string intact so that
+ * the PMU kernel driver can determine what configurable is associated to
+ * 'config'.
+ */
+static int drv_str(yyscan_t scanner, int token)
+{
+	YYSTYPE *yylval = parse_events_get_lval(scanner);
+	char *text = parse_events_get_text(scanner);
+
+	/* Strip off the '@' */
+	yylval->str = strdup(text + 1);
+	return token;
+}
+
 #define REWIND(__alloc)				\
 do {								\
 	YYSTYPE *__yylval = parse_events_get_lval(yyscanner);	\
@@ -124,6 +144,7 @@ num_hex		0x[a-fA-F0-9]+
 num_raw_hex	[a-fA-F0-9]+
 name		[a-zA-Z_*?][a-zA-Z0-9_*?.]*
 name_minus	[a-zA-Z_*?][a-zA-Z0-9\-_*?.:]*
+drv_cfg_term	[a-zA-Z0-9_\.]+(=[a-zA-Z0-9_*?\.:]+)?
 /* If you add a modifier you need to update check_modifier() */
 modifier_event	[ukhpPGHSDI]+
 modifier_bp	[rwx]{1,3}
@@ -209,6 +230,7 @@ no-overwrite		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_NOOVERWRITE); }
 {name_minus}		{ return str(yyscanner, PE_NAME); }
 \[all\]			{ return PE_ARRAY_ALL; }
 "["			{ BEGIN(array); return '['; }
+@{drv_cfg_term}		{ return drv_str(yyscanner, PE_DRV_CFG_TERM); }
 }
 
 <mem>{
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 5be4a5f216d6..879115f93edc 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -49,6 +49,7 @@ static void inc_group_count(struct list_head *list,
 %token PE_ERROR
 %token PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT
 %token PE_ARRAY_ALL PE_ARRAY_RANGE
+%token PE_DRV_CFG_TERM
 %type <num> PE_VALUE
 %type <num> PE_VALUE_SYM_HW
 %type <num> PE_VALUE_SYM_SW
@@ -63,6 +64,7 @@ static void inc_group_count(struct list_head *list,
 %type <str> PE_MODIFIER_BP
 %type <str> PE_EVENT_NAME
 %type <str> PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT
+%type <str> PE_DRV_CFG_TERM
 %type <num> value_sym
 %type <head> event_config
 %type <head> opt_event_config
@@ -599,6 +601,15 @@ PE_NAME array '=' PE_VALUE
 	term->array = $2;
 	$$ = term;
 }
+|
+PE_DRV_CFG_TERM
+{
+	struct parse_events_term *term;
+
+	ABORT_ON(parse_events_term__str(&term, PARSE_EVENTS__TERM_TYPE_DRV_CFG,
+					$1, $1, &@1, NULL));
+	$$ = term;
+}
 
 array:
 '[' array_terms ']'
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V5 7/9] perf tools: pushing driver configuration down to the kernel
  2016-08-11 16:20 [PATCH V5 0/9] perf: Driver specific configuration for PMU Mathieu Poirier
                   ` (5 preceding siblings ...)
  2016-08-11 16:21 ` [PATCH V5 6/9] perf tools: add infrastructure for PMU specific configuration Mathieu Poirier
@ 2016-08-11 16:21 ` Mathieu Poirier
  2016-08-11 16:21 ` [PATCH V5 8/9] coresight: adding sink parameter to function coresight_build_path() Mathieu Poirier
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-11 16:21 UTC (permalink / raw)
  To: peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel,
	linux-arm-kernel, Mathieu Poirier

Now that PMU specific driver configuration are queued in
evsel::config_terms, all we need to do is re-use the current
ioctl() mechanism to push down the information to the kernel
driver.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 tools/perf/builtin-record.c |  9 +++++++++
 tools/perf/builtin-stat.c   |  8 ++++++++
 tools/perf/builtin-top.c    | 12 ++++++++++++
 tools/perf/util/evlist.c    | 21 +++++++++++++++++++++
 tools/perf/util/evlist.h    |  3 +++
 tools/perf/util/evsel.c     | 24 ++++++++++++++++++++++++
 tools/perf/util/evsel.h     |  3 +++
 7 files changed, 80 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 6355902fbfc8..74927d6e147e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -383,6 +383,7 @@ static int record__open(struct record *rec)
 	struct perf_evlist *evlist = rec->evlist;
 	struct perf_session *session = rec->session;
 	struct record_opts *opts = &rec->opts;
+	struct perf_evsel_config_term *err_term;
 	int rc = 0;
 
 	perf_evlist__config(evlist, opts, &callchain_param);
@@ -412,6 +413,14 @@ try_again:
 		goto out;
 	}
 
+	if (perf_evlist__apply_drv_configs(evlist, &pos, &err_term)) {
+		error("failed to set config \"%s\" on event %s with %d (%s)\n",
+		      err_term->val.drv_cfg, perf_evsel__name(pos), errno,
+		      strerror_r(errno, msg, sizeof(msg)));
+		rc = -1;
+		goto out;
+	}
+
 	rc = record__mmap(rec);
 	if (rc)
 		goto out;
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 0c16d20d7e32..f003a9e50f4e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -533,6 +533,7 @@ static int __run_perf_stat(int argc, const char **argv)
 	int status = 0;
 	const bool forks = (argc > 0);
 	bool is_pipe = STAT_RECORD ? perf_stat.file.is_pipe : false;
+	struct perf_evsel_config_term *err_term;
 
 	if (interval) {
 		ts.tv_sec  = interval / 1000;
@@ -604,6 +605,13 @@ try_again:
 		return -1;
 	}
 
+	if (perf_evlist__apply_drv_configs(evsel_list, &counter, &err_term)) {
+		error("failed to set config \"%s\" on event %s with %d (%s)\n",
+		      err_term->val.drv_cfg, perf_evsel__name(counter), errno,
+		      strerror_r(errno, msg, sizeof(msg)));
+		return -1;
+	}
+
 	if (STAT_RECORD) {
 		int err, fd = perf_data_file__fd(&perf_stat.file);
 
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 418ed94756d3..0e566fe95e1b 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -940,6 +940,10 @@ static int callchain_param__setup_sample_type(struct callchain_param *callchain)
 
 static int __cmd_top(struct perf_top *top)
 {
+	char msg[512];
+	struct perf_evsel *pos;
+	struct perf_evsel_config_term *err_term;
+	struct perf_evlist *evlist = top->evlist;
 	struct record_opts *opts = &top->record_opts;
 	pthread_t thread;
 	int ret;
@@ -976,6 +980,14 @@ static int __cmd_top(struct perf_top *top)
 	if (ret)
 		goto out_delete;
 
+	ret = perf_evlist__apply_drv_configs(evlist, &pos, &err_term);
+	if (ret) {
+		error("failed to set config \"%s\" on event %s with %d (%s)\n",
+			err_term->val.drv_cfg, perf_evsel__name(pos), errno,
+			strerror_r(errno, msg, sizeof(msg)));
+		goto out_delete;
+	}
+
 	top->session->evlist = top->evlist;
 	perf_session__set_id_hdr_size(top->session);
 
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 097b3ed77fdd..e907877b06b9 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1415,6 +1415,27 @@ int perf_evlist__apply_filters(struct perf_evlist *evlist, struct perf_evsel **e
 	return err;
 }
 
+int perf_evlist__apply_drv_configs(struct perf_evlist *evlist,
+				   struct perf_evsel **err_evsel,
+				   struct perf_evsel_config_term **err_term)
+{
+	struct perf_evsel *evsel;
+	int err = 0;
+	const int ncpus = cpu_map__nr(evlist->cpus),
+		  nthreads = thread_map__nr(evlist->threads);
+
+	evlist__for_each_entry(evlist, evsel) {
+		err = perf_evsel__apply_drv_configs(evsel, ncpus,
+						    nthreads, err_term);
+		if (err) {
+			*err_evsel = evsel;
+			break;
+		}
+	}
+
+	return err;
+}
+
 int perf_evlist__set_filter(struct perf_evlist *evlist, const char *filter)
 {
 	struct perf_evsel *evsel;
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 4fd034f22d2f..bf7ed0be3f33 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -232,6 +232,9 @@ void perf_evlist__set_maps(struct perf_evlist *evlist, struct cpu_map *cpus,
 			   struct thread_map *threads);
 int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target);
 int perf_evlist__apply_filters(struct perf_evlist *evlist, struct perf_evsel **err_evsel);
+int perf_evlist__apply_drv_configs(struct perf_evlist *evlist,
+				   struct perf_evsel **err_evsel,
+				   struct perf_evsel_config_term **term);
 
 void __perf_evlist__set_leader(struct list_head *list);
 void perf_evlist__set_leader(struct perf_evlist *evlist);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d9b80ef881cd..69984a2ea162 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1062,6 +1062,30 @@ int perf_evsel__append_filter(struct perf_evsel *evsel,
 	return -1;
 }
 
+int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
+				  int ncpus, int nthreads,
+				  struct perf_evsel_config_term **err_term)
+{
+	int err = 0;
+	struct perf_evsel_config_term *term;
+
+	list_for_each_entry(term, &evsel->config_terms, list) {
+		if (term->type != PERF_EVSEL__CONFIG_TERM_DRV_CFG)
+			continue;
+
+		err = perf_evsel__run_ioctl(evsel, ncpus, nthreads,
+					    PERF_EVENT_IOC_SET_DRV_CONFIGS,
+					    (void *)term->val.drv_cfg);
+
+		if (err) {
+			*err_term = term;
+			break;
+		}
+	}
+
+	return err;
+}
+
 int perf_evsel__enable(struct perf_evsel *evsel)
 {
 	int nthreads = thread_map__nr(evsel->threads);
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 323806082c58..73cff3b2c533 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -239,6 +239,9 @@ int perf_evsel__append_filter(struct perf_evsel *evsel,
 			      const char *op, const char *filter);
 int perf_evsel__apply_filter(struct perf_evsel *evsel, int ncpus, int nthreads,
 			     const char *filter);
+int perf_evsel__apply_drv_configs(struct perf_evsel *evsel,
+				  int ncpus, int nthreads,
+				  struct perf_evsel_config_term **err_term);
 int perf_evsel__enable(struct perf_evsel *evsel);
 int perf_evsel__disable(struct perf_evsel *evsel);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V5 8/9] coresight: adding sink parameter to function coresight_build_path()
  2016-08-11 16:20 [PATCH V5 0/9] perf: Driver specific configuration for PMU Mathieu Poirier
                   ` (6 preceding siblings ...)
  2016-08-11 16:21 ` [PATCH V5 7/9] perf tools: pushing driver configuration down to the kernel Mathieu Poirier
@ 2016-08-11 16:21 ` Mathieu Poirier
  2016-08-11 16:21 ` [PATCH V5 9/9] coresight: etm-perf: incorporating sink definition from cmd line Mathieu Poirier
  2016-08-22 15:15 ` [PATCH V5 0/9] perf: Driver specific configuration for PMU Alexander Shishkin
  9 siblings, 0 replies; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-11 16:21 UTC (permalink / raw)
  To: peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel,
	linux-arm-kernel, Mathieu Poirier

Up to now function coresight_build_path() was counting on a sink to
have been selected (from sysFS) prior to being called.  This patch
adds a string argument so that a sink matching the argument can be
selected.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm-perf.c |  2 +-
 drivers/hwtracing/coresight/coresight-priv.h     |  3 +-
 drivers/hwtracing/coresight/coresight.c          | 43 ++++++++++++++++--------
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index f4174f36c5a0..f8c7a8733b23 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -184,7 +184,7 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
 		 * list of devices from source to sink that can be
 		 * referenced later when the path is actually needed.
 		 */
-		event_data->path[cpu] = coresight_build_path(csdev);
+		event_data->path[cpu] = coresight_build_path(csdev, NULL);
 		if (!event_data->path[cpu])
 			goto err;
 	}
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 39841d1f58e0..8a35dbf4cd1b 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -103,7 +103,8 @@ static inline void CS_UNLOCK(void __iomem *addr)
 void coresight_disable_path(struct list_head *path);
 int coresight_enable_path(struct list_head *path, u32 mode);
 struct coresight_device *coresight_get_sink(struct list_head *path);
-struct list_head *coresight_build_path(struct coresight_device *csdev);
+struct list_head *coresight_build_path(struct coresight_device *csdev,
+				       const char *sink);
 void coresight_release_path(struct list_head *path);
 
 #ifdef CONFIG_CORESIGHT_SOURCE_ETM3X
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index bb20ee9747e1..d52e1ed94210 100644
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -372,30 +372,44 @@ struct coresight_device *coresight_get_sink(struct list_head *path)
  * _coresight_build_path - recursively build a path from a @csdev to a sink.
  * @csdev:	The device to start from.
  * @path:	The list to add devices to.
+ * @sink:	The name of the sink this path should connect with.
  *
- * The tree of Coresight device is traversed until an activated sink is
- * found.  From there the sink is added to the list along with all the
- * devices that led to that point - the end result is a list from source
- * to sink. In that list the source is the first device and the sink the
- * last one.
+ * The tree of Coresight device is traversed until an activated sink or
+ * the one specified by @sink is found.
+ * From there the sink is added to the list along with all the devices that
+ * led to that point - the end result is a list from source to sink. In that
+ * list the source is the first device and the sink the last one.
  */
 static int _coresight_build_path(struct coresight_device *csdev,
-				 struct list_head *path)
+				 struct list_head *path, const char *sink)
 {
 	int i;
 	bool found = false;
 	struct coresight_node *node;
 
-	/* An activated sink has been found.  Enqueue the element */
-	if ((csdev->type == CORESIGHT_DEV_TYPE_SINK ||
-	     csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) && csdev->activated)
-		goto out;
+	/*
+	 * First see if we are dealing with a sink.  If we have one check if
+	 * it was selected via sysFS or the perf cmd line.
+	 */
+	if (csdev->type == CORESIGHT_DEV_TYPE_SINK ||
+	    csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) {
+		/* Discourage meddling in sysFS if using the perf interface */
+		if (sink && csdev->activated)
+			return -EINVAL;
+		/* Activated via perf cmd line */
+		if (sink && !strcmp(dev_name(&csdev->dev), sink))
+			goto out;
+		/* Activated via sysFS */
+		if (csdev->activated)
+			goto out;
+	}
 
 	/* Not a sink - recursively explore each port found on this element */
 	for (i = 0; i < csdev->nr_outport; i++) {
 		struct coresight_device *child_dev = csdev->conns[i].child_dev;
 
-		if (child_dev && _coresight_build_path(child_dev, path) == 0) {
+		if (child_dev &&
+		    _coresight_build_path(child_dev, path, sink) == 0) {
 			found = true;
 			break;
 		}
@@ -422,7 +436,8 @@ out:
 	return 0;
 }
 
-struct list_head *coresight_build_path(struct coresight_device *csdev)
+struct list_head *coresight_build_path(struct coresight_device *csdev,
+				       const char *sink)
 {
 	struct list_head *path;
 	int rc;
@@ -433,7 +448,7 @@ struct list_head *coresight_build_path(struct coresight_device *csdev)
 
 	INIT_LIST_HEAD(path);
 
-	rc = _coresight_build_path(csdev, path);
+	rc = _coresight_build_path(csdev, path, sink);
 	if (rc) {
 		kfree(path);
 		return ERR_PTR(rc);
@@ -508,7 +523,7 @@ int coresight_enable(struct coresight_device *csdev)
 	if (csdev->enable)
 		goto out;
 
-	path = coresight_build_path(csdev);
+	path = coresight_build_path(csdev, NULL);
 	if (IS_ERR(path)) {
 		pr_err("building path(s) failed\n");
 		ret = PTR_ERR(path);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH V5 9/9] coresight: etm-perf: incorporating sink definition from cmd line
  2016-08-11 16:20 [PATCH V5 0/9] perf: Driver specific configuration for PMU Mathieu Poirier
                   ` (7 preceding siblings ...)
  2016-08-11 16:21 ` [PATCH V5 8/9] coresight: adding sink parameter to function coresight_build_path() Mathieu Poirier
@ 2016-08-11 16:21 ` Mathieu Poirier
  2016-08-22 16:40   ` Alexander Shishkin
  2016-08-22 15:15 ` [PATCH V5 0/9] perf: Driver specific configuration for PMU Alexander Shishkin
  9 siblings, 1 reply; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-11 16:21 UTC (permalink / raw)
  To: peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel,
	linux-arm-kernel, Mathieu Poirier

Now that PMU specific configuration is available as part of the event,
lookup the sink identified by users from the perf command line and build
a path from source to sink.

With this functionality it is no longer required to select a sink in a
separate step (from sysFS) before a perf trace session can be started.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm-perf.c | 139 ++++++++++++++++++++++-
 1 file changed, 138 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index f8c7a8733b23..eab038805cbe 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -22,6 +22,7 @@
 #include <linux/list.h>
 #include <linux/mm.h>
 #include <linux/init.h>
+#include <linux/parser.h>
 #include <linux/perf_event.h>
 #include <linux/slab.h>
 #include <linux/types.h>
@@ -71,11 +72,25 @@ static const struct attribute_group *etm_pmu_attr_groups[] = {
 
 static void etm_event_read(struct perf_event *event) {}
 
+static void etm_event_destroy(struct perf_event *event)
+{
+	void *sink;
+
+	sink = perf_event_drv_configs_set(event, NULL);
+	kfree(sink);
+}
+
 static int etm_event_init(struct perf_event *event)
 {
+	struct hw_perf_event *hw_event = &event->hw;
+
 	if (event->attr.type != etm_pmu.type)
 		return -ENOENT;
 
+	event->destroy = etm_event_destroy;
+	raw_spin_lock_init(&hw_event->drv_configs_lock);
+	hw_event->drv_configs = NULL;
+
 	return 0;
 }
 
@@ -159,6 +174,7 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
 			   int nr_pages, bool overwrite)
 {
 	int cpu;
+	char *cmdl_sink;
 	cpumask_t *mask;
 	struct coresight_device *sink;
 	struct etm_event_data *event_data = NULL;
@@ -171,6 +187,14 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
 
 	mask = &event_data->mask;
 
+	/*
+	 * If a sink was specified from the perf cmdline it will be part of
+	 * the event's drv_configs.
+	 */
+	cmdl_sink = (char *)perf_event_drv_configs_get(event);
+	if (IS_ERR(cmdl_sink))
+		cmdl_sink = NULL;
+
 	/* Setup the path for each CPU in a trace session */
 	for_each_cpu(cpu, mask) {
 		struct coresight_device *csdev;
@@ -184,7 +208,7 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
 		 * list of devices from source to sink that can be
 		 * referenced later when the path is actually needed.
 		 */
-		event_data->path[cpu] = coresight_build_path(csdev, NULL);
+		event_data->path[cpu] = coresight_build_path(csdev, cmdl_sink);
 		if (!event_data->path[cpu])
 			goto err;
 	}
@@ -342,6 +366,118 @@ static void etm_event_del(struct perf_event *event, int mode)
 	etm_event_stop(event, PERF_EF_UPDATE);
 }
 
+enum {
+	ETM_TOKEN_SINK_CPU,
+	ETM_TOKEN_SINK,
+	ETM_TOKEN_ERR,
+};
+
+static const match_table_t drv_cfg_tokens = {
+	{ETM_TOKEN_SINK_CPU, "sink=cpu%d:%s"},
+	{ETM_TOKEN_SINK, "sink=%s"},
+	{ETM_TOKEN_ERR, NULL},
+};
+
+/**
+ * etm_parse_drv_configs - parse configuration string
+ *
+ * Returns: ERR_PTR() upon error,  NULL if the config string CPU is different
+ *	    from the event's CPU and the address of a valid string if one has
+ *	    been found.
+ */
+static char *etm_parse_drv_configs(int event_cpu, char *config_str)
+{
+	char *sink = NULL, *config = config_str;
+	int cpu, token;
+	substring_t args[MAX_OPT_ARGS];
+
+	/* See above declared @drv_cfg_tokens for the usable formats */
+	token = match_token(config, drv_cfg_tokens, args);
+	switch (token) {
+	case ETM_TOKEN_SINK:
+		/* Simply return the sink that was specified */
+		sink = match_strdup(&args[0]);
+		break;
+	case ETM_TOKEN_SINK_CPU:
+		/*
+		 * We have a sink and a CPU, so we need to make sure the
+		 * given CPU matches the event's CPU.  If not this sink is
+		 * destined to another tracer.  This isn't an error hence not
+		 * setting @ret.
+		 */
+
+		/* Get the cpu */
+		if (match_int(&args[0], &cpu)) {
+			sink = ERR_PTR(-EINVAL);
+			goto out;
+		}
+
+		/* This sink configuration is meant for another CPU */
+		if (event_cpu != cpu)
+			goto out;
+
+		sink = match_strdup(&args[1]);
+		break;
+	default:
+		sink = ERR_PTR(-EINVAL);
+	}
+
+out:
+	return sink;
+}
+
+static int
+etm_set_drv_configs(struct perf_event *event,
+		    struct list_head *drv_configs)
+{
+	char *config, *sink;
+	int len;
+	struct perf_drv_config *drv_config;
+	void *old_sink;
+
+	list_for_each_entry(drv_config, drv_configs, entry) {
+		/* ETM HW configuration needs a sink specification */
+		if (!drv_config->option)
+			return -EINVAL;
+
+		len = strlen(drv_config->config) + strlen("=") +
+		      strlen(drv_config->option) + 1;
+
+		config = kmalloc(len, GFP_KERNEL);
+		if (!config)
+			return -ENOMEM;
+
+		/* Reconstruct user configuration */
+		snprintf(config, len, "%s=%s",
+			 drv_config->config, drv_config->option);
+
+		sink = etm_parse_drv_configs(event->cpu, config);
+		kfree(config);
+
+		/* No need to continue if the parse encountered and error */
+		if (IS_ERR(sink))
+			return PTR_ERR(sink);
+
+		/* A valid sink was found */
+		if (sink)
+			goto found;
+	}
+
+	/*
+	 * A sink wasn't found, which isn't automatically an error.  Other
+	 * options on the cmd line may still need to be parsed.
+	 */
+	return 0;
+
+found:
+	/* Record the sink that was found */
+	old_sink = perf_event_drv_configs_set(event, sink);
+	/* And get rid of whatever we had before */
+	kfree(old_sink);
+
+	return 0;
+}
+
 int etm_perf_symlink(struct coresight_device *csdev, bool link)
 {
 	char entry[sizeof("cpu9999999")];
@@ -383,6 +519,7 @@ static int __init etm_perf_init(void)
 	etm_pmu.stop		= etm_event_stop;
 	etm_pmu.add		= etm_event_add;
 	etm_pmu.del		= etm_event_del;
+	etm_pmu.set_drv_configs	= etm_set_drv_configs;
 
 	ret = perf_pmu_register(&etm_pmu, CORESIGHT_ETM_PMU_NAME, -1);
 	if (ret == 0)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH V5 0/9] perf: Driver specific configuration for PMU
  2016-08-11 16:20 [PATCH V5 0/9] perf: Driver specific configuration for PMU Mathieu Poirier
                   ` (8 preceding siblings ...)
  2016-08-11 16:21 ` [PATCH V5 9/9] coresight: etm-perf: incorporating sink definition from cmd line Mathieu Poirier
@ 2016-08-22 15:15 ` Alexander Shishkin
  2016-08-23 14:51   ` Mathieu Poirier
  9 siblings, 1 reply; 19+ messages in thread
From: Alexander Shishkin @ 2016-08-22 15:15 UTC (permalink / raw)
  To: Mathieu Poirier, peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel, linux-arm-kernel

Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> As such something that used to be a two-step process:
>
> # echo 1 > /sys/bus/coresight/devices/20070000.etr/enable_sink
> # perf record -e cs_etm//u --per-thread  uname
>
> is integrated in a single command:
>
> # perf record -e cs_etm/@sink=20070000.etr/u --per-thread  uname

Can't we simply teach perf record to write 1 to that sysfs attribute and
avoid parsing more ascii strings in the kernel? I suspect that would also
take way less code.

Are there any other use cases for this besides specifying @sink for a
ETM?

Regards,
--
Alex

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V5 4/9] perf/core: Adding PMU driver specific configuration
  2016-08-11 16:20 ` [PATCH V5 4/9] perf/core: Adding PMU driver specific configuration Mathieu Poirier
@ 2016-08-22 16:18   ` Alexander Shishkin
  2016-08-23 14:44     ` Mathieu Poirier
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Shishkin @ 2016-08-22 16:18 UTC (permalink / raw)
  To: Mathieu Poirier, peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel,
	linux-arm-kernel, Mathieu Poirier

Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> This patch somewhat mimics the work done on address filters to
> add the infrastructure needed to pass PMU specific HW
> configuration to the driver before a session starts.

Looks like a lot of work to do something that can be taken care of
entirely in userspace. A few comments below.

Btw, please don't forget to CC me on the kernel perf patches.

> +static struct perf_drv_config *
> +perf_drv_config_new(int cpu, struct list_head *drv_config_list)
> +{
> +	int node = cpu_to_node(cpu == -1 ? 0 : cpu);
> +	struct perf_drv_config *drv_config;
> +
> +	drv_config = kzalloc_node(sizeof(*drv_config), GFP_KERNEL, node);
> +	if (!drv_config)
> +		return ERR_PTR(-ENOMEM);

So it's the only error that this function may return.

> +
> +	INIT_LIST_HEAD(&drv_config->entry);
> +	list_add_tail(&drv_config->entry, drv_config_list);
> +
> +	return drv_config;
> +}
> +
> +static void free_drv_config_list(struct list_head *drv_config_list)
> +{
> +	struct perf_drv_config *drv_config, *itr;
> +
> +	list_for_each_entry_safe(drv_config, itr, drv_config_list, entry) {
> +		list_del(&drv_config->entry);
> +		kfree(drv_config->config);
> +		kfree(drv_config->option);
> +		kfree(drv_config);
> +	}
> +}
> +
> +/* How long does a configuration option really need to be?  */
> +#define PERF_DRV_CONFIG_MAX	128

Considering that you're already limiting the entire input buffer to
PAGE_SIZE, this is redundant.

> +
>  /*
> - * hrtimer based swevent callback
> + * PMU specific driver configuration as specified from user space.
> + * The data come in the form of an ascii string pushed down to the kernel
> + * using an ioctl() call.
> + *
> + * Two format are accepted: a singleton and in pairs.  All of the following
> + * are valid: cfg1, cfg2=config2, cfg3=anything_is_possible.

What's the difference between 'config2' and 'anything_is_possible'?

> + *
> + * It is up to each PMU driver to make sure they can work with the
> + * submitted configurables.
>   */
> +static int
> +perf_event_parse_drv_config(struct perf_event *event, char *options,
> +			    struct list_head *drv_config_list)
> +{
> +	char *token;
> +	int ret;
> +	struct perf_drv_config *drv_config;
> +
> +	/*
> +	 * First split the @options string in nibbles.  Using the above
> +	 * example "cfg1", "cfg2=option2" and "cfg3=anything_is_possible"

"cfg2=config2", if you're referring to the comment at the top.

> +	 * will be processed.
> +	 */
> +	while ((token = strsep(&options, ",")) != NULL) {
> +		char *nibble, *config, *option;
> +
> +		if (!*token)
> +			continue;

So empty configs are valid?

> +
> +		/* Allocate a new driver config structure and queue it. */
> +		drv_config = perf_drv_config_new(event->cpu, drv_config_list);
> +		if (IS_ERR(drv_config)) {
> +			ret = PTR_ERR(drv_config);

We know it's ENOMEM, no need for ERR_PTR()->PTR_ERR().

> +			goto fail;
> +		}
> +
> +		/*
> +		 * The nibbles are either a "config" or a "config=option"
> +		 * pair.  First get the config part.  Since strsep() sets
> +		 * @nibble to the next valid token, nibble will be equal to
> +		 * the option part or NULL after the first call.
> +		 */
> +		nibble = token;
> +		config = strsep(&nibble, "=");
> +		option = nibble;

So '@,=,=,=,=,=,=,=,=,' is a valid driver configuration, by the looks of
it?

> +
> +		/* This shouldn't be happening */

Indeed.

> +		if (!config) {
> +			ret = -EINVAL;
> +			goto fail;
> +		}

Regards,
--
Alex

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V5 9/9] coresight: etm-perf: incorporating sink definition from cmd line
  2016-08-11 16:21 ` [PATCH V5 9/9] coresight: etm-perf: incorporating sink definition from cmd line Mathieu Poirier
@ 2016-08-22 16:40   ` Alexander Shishkin
  2016-08-23 14:46     ` Mathieu Poirier
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Shishkin @ 2016-08-22 16:40 UTC (permalink / raw)
  To: Mathieu Poirier, peterz
  Cc: acme, jolsa, mingo, vince, mtk.manpages, linux-kernel,
	linux-arm-kernel, Mathieu Poirier

Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> +enum {
> +	ETM_TOKEN_SINK_CPU,
> +	ETM_TOKEN_SINK,
> +	ETM_TOKEN_ERR,
> +};
> +
> +static const match_table_t drv_cfg_tokens = {
> +	{ETM_TOKEN_SINK_CPU, "sink=cpu%d:%s"},
> +	{ETM_TOKEN_SINK, "sink=%s"},
> +	{ETM_TOKEN_ERR, NULL},
> +};

Wait, but we just parsed away the '=' and the whole thing is now a
linked list of { key, value }?

This also answers my question from the other email about the use cases
for sending in ascii strings. In my opinion, all this is completely
unnecessary.

> +static int
> +etm_set_drv_configs(struct perf_event *event,
> +		    struct list_head *drv_configs)
> +{
> +	char *config, *sink;
> +	int len;
> +	struct perf_drv_config *drv_config;
> +	void *old_sink;
> +
> +	list_for_each_entry(drv_config, drv_configs, entry) {
> +		/* ETM HW configuration needs a sink specification */
> +		if (!drv_config->option)
> +			return -EINVAL;
> +
> +		len = strlen(drv_config->config) + strlen("=") +
> +		      strlen(drv_config->option) + 1;
> +
> +		config = kmalloc(len, GFP_KERNEL);
> +		if (!config)
> +			return -ENOMEM;
> +
> +		/* Reconstruct user configuration */
> +		snprintf(config, len, "%s=%s",
> +			 drv_config->config, drv_config->option);

Wait, what? We parse this *twice*?

There's basically a malloc+snprintf[which could have been
kasprintf()]+match_token just to see if drv_config::option starts with a
'cpu%d:'?

Regards,
--
Alex

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V5 4/9] perf/core: Adding PMU driver specific configuration
  2016-08-22 16:18   ` Alexander Shishkin
@ 2016-08-23 14:44     ` Mathieu Poirier
  0 siblings, 0 replies; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-23 14:44 UTC (permalink / raw)
  To: Alexander Shishkin
  Cc: Peter Zijlstra, Arnaldo Carvalho de Melo, jolsa, Ingo Molnar,
	Vince Weaver, Michael Kerrisk-manpages, linux-kernel,
	linux-arm-kernel

On 22 August 2016 at 10:18, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
>
>> This patch somewhat mimics the work done on address filters to
>> add the infrastructure needed to pass PMU specific HW
>> configuration to the driver before a session starts.
>
> Looks like a lot of work to do something that can be taken care of
> entirely in userspace. A few comments below.
>
> Btw, please don't forget to CC me on the kernel perf patches.
>
>> +static struct perf_drv_config *
>> +perf_drv_config_new(int cpu, struct list_head *drv_config_list)
>> +{
>> +     int node = cpu_to_node(cpu == -1 ? 0 : cpu);
>> +     struct perf_drv_config *drv_config;
>> +
>> +     drv_config = kzalloc_node(sizeof(*drv_config), GFP_KERNEL, node);
>> +     if (!drv_config)
>> +             return ERR_PTR(-ENOMEM);
>
> So it's the only error that this function may return.

You are correct.  On the flip side I am failing to see what you want
to highlight.

>
>> +
>> +     INIT_LIST_HEAD(&drv_config->entry);
>> +     list_add_tail(&drv_config->entry, drv_config_list);
>> +
>> +     return drv_config;
>> +}
>> +
>> +static void free_drv_config_list(struct list_head *drv_config_list)
>> +{
>> +     struct perf_drv_config *drv_config, *itr;
>> +
>> +     list_for_each_entry_safe(drv_config, itr, drv_config_list, entry) {
>> +             list_del(&drv_config->entry);
>> +             kfree(drv_config->config);
>> +             kfree(drv_config->option);
>> +             kfree(drv_config);
>> +     }
>> +}
>> +
>> +/* How long does a configuration option really need to be?  */
>> +#define PERF_DRV_CONFIG_MAX  128
>
> Considering that you're already limiting the entire input buffer to
> PAGE_SIZE, this is redundant.
>
>> +
>>  /*
>> - * hrtimer based swevent callback
>> + * PMU specific driver configuration as specified from user space.
>> + * The data come in the form of an ascii string pushed down to the kernel
>> + * using an ioctl() call.
>> + *
>> + * Two format are accepted: a singleton and in pairs.  All of the following
>> + * are valid: cfg1, cfg2=config2, cfg3=anything_is_possible.
>
> What's the difference between 'config2' and 'anything_is_possible'?

There isn't any - this is simply a comment to inform reviewers that
values can be anything for as long as the driver recognises it.

>
>> + *
>> + * It is up to each PMU driver to make sure they can work with the
>> + * submitted configurables.
>>   */
>> +static int
>> +perf_event_parse_drv_config(struct perf_event *event, char *options,
>> +                         struct list_head *drv_config_list)
>> +{
>> +     char *token;
>> +     int ret;
>> +     struct perf_drv_config *drv_config;
>> +
>> +     /*
>> +      * First split the @options string in nibbles.  Using the above
>> +      * example "cfg1", "cfg2=option2" and "cfg3=anything_is_possible"
>
> "cfg2=config2", if you're referring to the comment at the top.
>
>> +      * will be processed.
>> +      */
>> +     while ((token = strsep(&options, ",")) != NULL) {
>> +             char *nibble, *config, *option;
>> +
>> +             if (!*token)
>> +                     continue;
>
> So empty configs are valid?

They will simply be ignored.

>
>> +
>> +             /* Allocate a new driver config structure and queue it. */
>> +             drv_config = perf_drv_config_new(event->cpu, drv_config_list);
>> +             if (IS_ERR(drv_config)) {
>> +                     ret = PTR_ERR(drv_config);
>
> We know it's ENOMEM, no need for ERR_PTR()->PTR_ERR().

I don't see the arm in using PTR_ERR?  In fact someone would have
likely called me out should I didn't use it.

>
>> +                     goto fail;
>> +             }
>> +
>> +             /*
>> +              * The nibbles are either a "config" or a "config=option"
>> +              * pair.  First get the config part.  Since strsep() sets
>> +              * @nibble to the next valid token, nibble will be equal to
>> +              * the option part or NULL after the first call.
>> +              */
>> +             nibble = token;
>> +             config = strsep(&nibble, "=");
>> +             option = nibble;
>
> So '@,=,=,=,=,=,=,=,=,' is a valid driver configuration, by the looks of
> it?

>From a core framework point of view yes.  It is then up to the
individual drivers to validate configuration options.  This used to be
in the driver but Peter asked to get more parsing in the core,
something this code does.

After sending this set I was thinking we can parse for "sink=xyz" in
the core and simply pass the "xyz" part to the driver.  This isn't
generic but we do the same for filters.  Peter, would you like this
way better?

>
>> +
>> +             /* This shouldn't be happening */
>
> Indeed.
>
>> +             if (!config) {
>> +                     ret = -EINVAL;
>> +                     goto fail;
>> +             }
>
> Regards,
> --
> Alex

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V5 9/9] coresight: etm-perf: incorporating sink definition from cmd line
  2016-08-22 16:40   ` Alexander Shishkin
@ 2016-08-23 14:46     ` Mathieu Poirier
  0 siblings, 0 replies; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-23 14:46 UTC (permalink / raw)
  To: Alexander Shishkin
  Cc: Peter Zijlstra, Arnaldo Carvalho de Melo, jolsa, Ingo Molnar,
	Vince Weaver, Michael Kerrisk-manpages, linux-kernel,
	linux-arm-kernel

On 22 August 2016 at 10:40, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
>
>> +enum {
>> +     ETM_TOKEN_SINK_CPU,
>> +     ETM_TOKEN_SINK,
>> +     ETM_TOKEN_ERR,
>> +};
>> +
>> +static const match_table_t drv_cfg_tokens = {
>> +     {ETM_TOKEN_SINK_CPU, "sink=cpu%d:%s"},
>> +     {ETM_TOKEN_SINK, "sink=%s"},
>> +     {ETM_TOKEN_ERR, NULL},
>> +};
>
> Wait, but we just parsed away the '=' and the whole thing is now a
> linked list of { key, value }?

You're correct.  From a parsing point of view it was better to
reassemble the string and parse the whole thing again.  As suggested
in my previous email if the parsing for "sink=xyz"  is done in the
core we won't have to do this part.

>
> This also answers my question from the other email about the use cases
> for sending in ascii strings. In my opinion, all this is completely
> unnecessary.
>
>> +static int
>> +etm_set_drv_configs(struct perf_event *event,
>> +                 struct list_head *drv_configs)
>> +{
>> +     char *config, *sink;
>> +     int len;
>> +     struct perf_drv_config *drv_config;
>> +     void *old_sink;
>> +
>> +     list_for_each_entry(drv_config, drv_configs, entry) {
>> +             /* ETM HW configuration needs a sink specification */
>> +             if (!drv_config->option)
>> +                     return -EINVAL;
>> +
>> +             len = strlen(drv_config->config) + strlen("=") +
>> +                   strlen(drv_config->option) + 1;
>> +
>> +             config = kmalloc(len, GFP_KERNEL);
>> +             if (!config)
>> +                     return -ENOMEM;
>> +
>> +             /* Reconstruct user configuration */
>> +             snprintf(config, len, "%s=%s",
>> +                      drv_config->config, drv_config->option);
>
> Wait, what? We parse this *twice*?
>
> There's basically a malloc+snprintf[which could have been
> kasprintf()]+match_token just to see if drv_config::option starts with a
> 'cpu%d:'?
>
> Regards,
> --
> Alex

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V5 0/9] perf: Driver specific configuration for PMU
  2016-08-22 15:15 ` [PATCH V5 0/9] perf: Driver specific configuration for PMU Alexander Shishkin
@ 2016-08-23 14:51   ` Mathieu Poirier
  2016-08-23 17:02     ` Alexander Shishkin
  0 siblings, 1 reply; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-23 14:51 UTC (permalink / raw)
  To: Alexander Shishkin
  Cc: Peter Zijlstra, Arnaldo Carvalho de Melo, jolsa, Ingo Molnar,
	Vince Weaver, Michael Kerrisk-manpages, linux-kernel,
	linux-arm-kernel

On 22 August 2016 at 09:15, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
>
>> As such something that used to be a two-step process:
>>
>> # echo 1 > /sys/bus/coresight/devices/20070000.etr/enable_sink
>> # perf record -e cs_etm//u --per-thread  uname
>>
>> is integrated in a single command:
>>
>> # perf record -e cs_etm/@sink=20070000.etr/u --per-thread  uname
>
> Can't we simply teach perf record to write 1 to that sysfs attribute and
> avoid parsing more ascii strings in the kernel? I suspect that would also
> take way less code.

That, in my opinion, would be a big hack.  Peter and Jiri, any thoughts on this?

>
> Are there any other use cases for this besides specifying @sink for a
> ETM?

Not at this time but there is so many configuration option for the
ETM/PTM tracers (that aren't filters) that I wanted the right
infrastructure to be there should/when we need to expand.

Thanks for taking the time to review this set,
Mathieu

>
> Regards,
> --
> Alex

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V5 0/9] perf: Driver specific configuration for PMU
  2016-08-23 14:51   ` Mathieu Poirier
@ 2016-08-23 17:02     ` Alexander Shishkin
  2016-08-23 19:47       ` Mathieu Poirier
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Shishkin @ 2016-08-23 17:02 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Peter Zijlstra, Arnaldo Carvalho de Melo, jolsa, Ingo Molnar,
	Vince Weaver, Michael Kerrisk-manpages, linux-kernel,
	linux-arm-kernel

Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> On 22 August 2016 at 09:15, Alexander Shishkin
> <alexander.shishkin@linux.intel.com> wrote:
>> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
>>
>>> As such something that used to be a two-step process:
>>>
>>> # echo 1 > /sys/bus/coresight/devices/20070000.etr/enable_sink
>>> # perf record -e cs_etm//u --per-thread  uname
>>>
>>> is integrated in a single command:
>>>
>>> # perf record -e cs_etm/@sink=20070000.etr/u --per-thread  uname
>>
>> Can't we simply teach perf record to write 1 to that sysfs attribute and
>> avoid parsing more ascii strings in the kernel? I suspect that would also
>> take way less code.
>
> That, in my opinion, would be a big hack.  Peter and Jiri, any thoughts on this?

Why would you say it's a hack? The whole tracer->sink configuration is
outside of scope of perf framework, there is absolutely no reason why
perf core should do this, especially, add this to perf ABI.

It would have somewhat made sense if you could configure different
events on the same pmu to send data to different sinks, but even that
won't work, because you simply cannot guarantee sink's availability if
you have to release it when your event gets scheduled out.

>> Are there any other use cases for this besides specifying @sink for a
>> ETM?
>
> Not at this time but there is so many configuration option for the
> ETM/PTM tracers (that aren't filters) that I wanted the right
> infrastructure to be there should/when we need to expand.

As I've asked before, what exactly is the need for expansion? That is,
something more than purely hypothetical that needs to be configured in
the tracer, on per event basis, *and* does not fit into the event
attribute. Note that the sink configuration also doesn't fit the bill.

Regards,
--
Alex

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH V5 0/9] perf: Driver specific configuration for PMU
  2016-08-23 17:02     ` Alexander Shishkin
@ 2016-08-23 19:47       ` Mathieu Poirier
  0 siblings, 0 replies; 19+ messages in thread
From: Mathieu Poirier @ 2016-08-23 19:47 UTC (permalink / raw)
  To: Alexander Shishkin
  Cc: Peter Zijlstra, Arnaldo Carvalho de Melo, jolsa, Ingo Molnar,
	Vince Weaver, Michael Kerrisk-manpages, linux-kernel,
	linux-arm-kernel

On 23 August 2016 at 11:02, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
>
>> On 22 August 2016 at 09:15, Alexander Shishkin
>> <alexander.shishkin@linux.intel.com> wrote:
>>> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
>>>
>>>> As such something that used to be a two-step process:
>>>>
>>>> # echo 1 > /sys/bus/coresight/devices/20070000.etr/enable_sink
>>>> # perf record -e cs_etm//u --per-thread  uname
>>>>
>>>> is integrated in a single command:
>>>>
>>>> # perf record -e cs_etm/@sink=20070000.etr/u --per-thread  uname
>>>
>>> Can't we simply teach perf record to write 1 to that sysfs attribute and
>>> avoid parsing more ascii strings in the kernel? I suspect that would also
>>> take way less code.
>>
>> That, in my opinion, would be a big hack.  Peter and Jiri, any thoughts on this?
>
> Why would you say it's a hack? The whole tracer->sink configuration is
> outside of scope of perf framework, there is absolutely no reason why
> perf core should do this, especially, add this to perf ABI.

That is why the solution was made generic - sink configuration is
simply using it.

>
> It would have somewhat made sense if you could configure different
> events on the same pmu to send data to different sinks, but even that
> won't work, because you simply cannot guarantee sink's availability if
> you have to release it when your event gets scheduled out.
>
>>> Are there any other use cases for this besides specifying @sink for a
>>> ETM?
>>
>> Not at this time but there is so many configuration option for the
>> ETM/PTM tracers (that aren't filters) that I wanted the right
>> infrastructure to be there should/when we need to expand.
>
> As I've asked before, what exactly is the need for expansion? That is,
> something more than purely hypothetical that needs to be configured in
> the tracer, on per event basis, *and* does not fit into the event
> attribute. Note that the sink configuration also doesn't fit the bill.

The first thing that comes to mind is counters and state machine,
something that I simply don't see fitting into the event attributes.
But those could also be set via sysFS - the end result is exactly the
same.

>
> Regards,
> --
> Alex

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [tip:perf/core] tools: Copy coresight-pmu.h header file needed by perf tools
  2016-08-11 16:20 ` [PATCH V5 1/9] tools: Copy the header file needed by perf tools Mathieu Poirier
@ 2016-08-24  9:23   ` tip-bot for Mathieu Poirier
  0 siblings, 0 replies; 19+ messages in thread
From: tip-bot for Mathieu Poirier @ 2016-08-24  9:23 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, vince, jolsa, peterz, linux-kernel, acme, mathieu.poirier,
	mingo, hpa

Commit-ID:  39ff526350059e61234d58676c13bcfcaac3a451
Gitweb:     http://git.kernel.org/tip/39ff526350059e61234d58676c13bcfcaac3a451
Author:     Mathieu Poirier <mathieu.poirier@linaro.org>
AuthorDate: Thu, 11 Aug 2016 10:20:56 -0600
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 23 Aug 2016 15:37:33 -0300

tools: Copy coresight-pmu.h header file needed by perf tools

Directly accessing kernel files is not allowed anymore.  As such making
file coresight-pmu.h accessible by the perf tools and complain if this
copy strays from the one found in the main kernel tree.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Vince Weaver <vince@deater.net>
Link: http://lkml.kernel.org/r/1470932464-726-2-git-send-email-mathieu.poirier@linaro.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 {include => tools/include}/linux/coresight-pmu.h | 0
 tools/perf/MANIFEST                              | 1 +
 tools/perf/Makefile.perf                         | 3 +++
 3 files changed, 4 insertions(+)

diff --git a/include/linux/coresight-pmu.h b/tools/include/linux/coresight-pmu.h
similarity index 100%
copy from include/linux/coresight-pmu.h
copy to tools/include/linux/coresight-pmu.h
diff --git a/tools/perf/MANIFEST b/tools/perf/MANIFEST
index f23a5e7..ff200c6 100644
--- a/tools/perf/MANIFEST
+++ b/tools/perf/MANIFEST
@@ -60,6 +60,7 @@ tools/include/asm-generic/bitops.h
 tools/include/linux/atomic.h
 tools/include/linux/bitops.h
 tools/include/linux/compiler.h
+tools/include/linux/coresight-pmu.h
 tools/include/linux/filter.h
 tools/include/linux/hash.h
 tools/include/linux/kernel.h
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 2d90875..aa7ab23 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -429,6 +429,9 @@ $(PERF_IN): prepare FORCE
 	@(test -f ../../include/asm-generic/bitops/fls64.h && ( \
         (diff -B ../include/asm-generic/bitops/fls64.h ../../include/asm-generic/bitops/fls64.h >/dev/null) \
         || echo "Warning: tools/include/asm-generic/bitops/fls64.h differs from kernel" >&2 )) || true
+	@(test -f ../../include/linux/coresight-pmu.h && ( \
+	(diff -B ../include/linux/coresight-pmu.h ../../include/linux/coresight-pmu.h >/dev/null) \
+	|| echo "Warning: tools/include/linux/coresight-pmu.h differs from kernel" >&2 )) || true
 	$(Q)$(MAKE) $(build)=perf
 
 $(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(LIBTRACEEVENT_DYNAMIC_LIST)

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-08-24  9:24 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-11 16:20 [PATCH V5 0/9] perf: Driver specific configuration for PMU Mathieu Poirier
2016-08-11 16:20 ` [PATCH V5 1/9] tools: Copy the header file needed by perf tools Mathieu Poirier
2016-08-24  9:23   ` [tip:perf/core] tools: Copy coresight-pmu.h " tip-bot for Mathieu Poirier
2016-08-11 16:20 ` [PATCH V5 2/9] perf tools: making coresight PMU listable Mathieu Poirier
2016-08-11 16:20 ` [PATCH V5 3/9] perf tools: adding coresight etm PMU record capabilities Mathieu Poirier
2016-08-11 16:20 ` [PATCH V5 4/9] perf/core: Adding PMU driver specific configuration Mathieu Poirier
2016-08-22 16:18   ` Alexander Shishkin
2016-08-23 14:44     ` Mathieu Poirier
2016-08-11 16:21 ` [PATCH V5 5/9] perf: Passing struct perf_event to function setup_aux() Mathieu Poirier
2016-08-11 16:21 ` [PATCH V5 6/9] perf tools: add infrastructure for PMU specific configuration Mathieu Poirier
2016-08-11 16:21 ` [PATCH V5 7/9] perf tools: pushing driver configuration down to the kernel Mathieu Poirier
2016-08-11 16:21 ` [PATCH V5 8/9] coresight: adding sink parameter to function coresight_build_path() Mathieu Poirier
2016-08-11 16:21 ` [PATCH V5 9/9] coresight: etm-perf: incorporating sink definition from cmd line Mathieu Poirier
2016-08-22 16:40   ` Alexander Shishkin
2016-08-23 14:46     ` Mathieu Poirier
2016-08-22 15:15 ` [PATCH V5 0/9] perf: Driver specific configuration for PMU Alexander Shishkin
2016-08-23 14:51   ` Mathieu Poirier
2016-08-23 17:02     ` Alexander Shishkin
2016-08-23 19:47       ` Mathieu Poirier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).