[PATCH v8 0/4] libperf userspace counter access

[PATCH v8 0/4] libperf userspace counter access

[Rob Herring]

I'm resending just the libperf userspace counter access without the Arm 
bits so hopefully it can be picked up for 5.13. The Arm bits seem to be 
a never ending review filled with long periods of silence. :(

Prior versions are here[1][2][3][4][5][6][7][8].

Rob

[1] https://lore.kernel.org/r/20190822144220.27860-1-raphael.gault@arm.com/
[2] https://lore.kernel.org/r/20200707205333.624938-1-robh@kernel.org/
[3] https://lore.kernel.org/r/20200828205614.3391252-1-robh@kernel.org/
[4] https://lore.kernel.org/r/20200911215118.2887710-1-robh@kernel.org/
[5] https://lore.kernel.org/r/20201001140116.651970-1-robh@kernel.org/
[6] https://lore.kernel.org/r/20210114020605.3943992-1-robh@kernel.org/
[7] https://lore.kernel.org/r/20210311000837.3630499-1-robh@kernel.org/
[8] https://lore.kernel.org/r/20210413171606.1825808-1-robh@kernel.org/

Rob Herring (4):
  tools/include: Add an initial math64.h
  libperf: Add evsel mmap support
  libperf: tests: Add support for verbose printing
  libperf: Add support for user space counter access

 tools/include/linux/math64.h             | 75 ++++++++++++++++++++
 tools/lib/perf/Documentation/libperf.txt |  3 +
 tools/lib/perf/evsel.c                   | 80 +++++++++++++++++++++
 tools/lib/perf/include/internal/evsel.h  |  1 +
 tools/lib/perf/include/internal/mmap.h   |  3 +
 tools/lib/perf/include/internal/tests.h  | 32 +++++++++
 tools/lib/perf/include/perf/evsel.h      |  3 +
 tools/lib/perf/libperf.map               |  3 +
 tools/lib/perf/mmap.c                    | 88 ++++++++++++++++++++++++
 tools/lib/perf/tests/Makefile            |  6 +-
 tools/lib/perf/tests/test-evsel.c        | 66 ++++++++++++++++++
 11 files changed, 358 insertions(+), 2 deletions(-)
 create mode 100644 tools/include/linux/math64.h

-- 
2.27.0

[PATCH v8 1/4] tools/include: Add an initial math64.h

[Rob Herring]

Add an initial math64.h similar to linux/math64.h with functions
mul_u64_u64_div64() and mul_u64_u32_shr(). This isn't a direct copy of
include/linux/math64.h as that doesn't define mul_u64_u64_div64().

Implementation was written by Peter Zilkstra based on linux/math64.h
and div64.h[1]. The original implementation was not optimal on arm64 as
__int128 division is not optimal with a call out to __udivti3, so I
dropped the __int128 variant of mul_u64_u64_div64().

[1] https://lore.kernel.org/lkml/20200322101848.GF2452@worktop.programming.kicks-ass.net/

Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 tools/include/linux/math64.h | 75 ++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 tools/include/linux/math64.h

diff --git a/tools/include/linux/math64.h b/tools/include/linux/math64.h
new file mode 100644
index 000000000000..4ad45d5943dc
--- /dev/null
+++ b/tools/include/linux/math64.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_MATH64_H
+#define _LINUX_MATH64_H
+
+#include <linux/types.h>
+
+#ifdef __x86_64__
+static inline u64 mul_u64_u64_div64(u64 a, u64 b, u64 c)
+{
+	u64 q;
+
+	asm ("mulq %2; divq %3" : "=a" (q)
+				: "a" (a), "rm" (b), "rm" (c)
+				: "rdx");
+
+	return q;
+}
+#define mul_u64_u64_div64 mul_u64_u64_div64
+#endif
+
+#ifdef __SIZEOF_INT128__
+static inline u64 mul_u64_u32_shr(u64 a, u32 b, unsigned int shift)
+{
+	return (u64)(((unsigned __int128)a * b) >> shift);
+}
+
+#else
+
+#ifdef __i386__
+static inline u64 mul_u32_u32(u32 a, u32 b)
+{
+	u32 high, low;
+
+	asm ("mull %[b]" : "=a" (low), "=d" (high)
+			 : [a] "a" (a), [b] "rm" (b) );
+
+	return low | ((u64)high) << 32;
+}
+#else
+static inline u64 mul_u32_u32(u32 a, u32 b)
+{
+	return (u64)a * b;
+}
+#endif
+
+static inline u64 mul_u64_u32_shr(u64 a, u32 b, unsigned int shift)
+{
+	u32 ah, al;
+	u64 ret;
+
+	al = a;
+	ah = a >> 32;
+
+	ret = mul_u32_u32(al, b) >> shift;
+	if (ah)
+		ret += mul_u32_u32(ah, b) << (32 - shift);
+
+	return ret;
+}
+
+#endif	/* __SIZEOF_INT128__ */
+
+#ifndef mul_u64_u64_div64
+static inline u64 mul_u64_u64_div64(u64 a, u64 b, u64 c)
+{
+	u64 quot, rem;
+
+	quot = a / c;
+	rem = a % c;
+
+	return quot * b + (rem * b) / c;
+}
+#endif
+
+#endif /* _LINUX_MATH64_H */
-- 
2.27.0

[PATCH v8 2/4] libperf: Add evsel mmap support

[Rob Herring]

In order to support usersapce access, an event must be mmapped. While
there's already mmap support for evlist, the usecase is a bit different
than the self monitoring with userspace access. So let's add new
perf_evsel__mmap()/perf_evsel_munmap() functions to mmap/munmap an
evsel. This allows implementing userspace access as a fastpath for
perf_evsel__read().

The mmapped address is returned by perf_evsel__mmap_base() which
primarily for users/tests to check if userspace access is enabled.

Signed-off-by: Rob Herring <robh@kernel.org>
---
v8:
 - Add perf_evsel__munmap()
 - munmap in event of an error in perf_evsel__mmap()
 - Ensure repeated calls to perf_evsel__mmap return error
v7:
 - Add NULL fd check to perf_evsel__mmap
v6:
 - split mmap struct into it's own xyarray
v5:
 - Create an mmap for every underlying event opened. Due to this, we
   need a different way to get the mmap ptr, so perf_evsel__mmap_base()
   is introduced.
v4:
 - Change perf_evsel__mmap size to pages instead of bytes
v3:
 - New patch split out from user access patch
---
 tools/lib/perf/Documentation/libperf.txt |  3 +
 tools/lib/perf/evsel.c                   | 76 ++++++++++++++++++++++++
 tools/lib/perf/include/internal/evsel.h  |  1 +
 tools/lib/perf/include/perf/evsel.h      |  3 +
 tools/lib/perf/libperf.map               |  3 +
 5 files changed, 86 insertions(+)

diff --git a/tools/lib/perf/Documentation/libperf.txt b/tools/lib/perf/Documentation/libperf.txt
index 0c74c30ed23a..63ae5e0195ce 100644
--- a/tools/lib/perf/Documentation/libperf.txt
+++ b/tools/lib/perf/Documentation/libperf.txt
@@ -136,6 +136,9 @@ SYNOPSIS
                        struct perf_thread_map *threads);
   void perf_evsel__close(struct perf_evsel *evsel);
   void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
+  int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
+  void perf_evsel__munmap(struct perf_evsel *evsel);
+  void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
   int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
                        struct perf_counts_values *count);
   int perf_evsel__enable(struct perf_evsel *evsel);
diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index 4dc06289f4c7..cd203998e875 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -11,10 +11,12 @@
 #include <stdlib.h>
 #include <internal/xyarray.h>
 #include <internal/cpumap.h>
+#include <internal/mmap.h>
 #include <internal/threadmap.h>
 #include <internal/lib.h>
 #include <linux/string.h>
 #include <sys/ioctl.h>
+#include <sys/mman.h>

 void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr)
 {
@@ -38,6 +40,7 @@ void perf_evsel__delete(struct perf_evsel *evsel)
 }

 #define FD(e, x, y) (*(int *) xyarray__entry(e->fd, x, y))
+#define MMAP(e, x, y) (e->mmap ? ((struct perf_mmap *) xyarray__entry(e->mmap, x, y)) : NULL)

 int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
 {
@@ -55,6 +58,13 @@ int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
 	return evsel->fd != NULL ? 0 : -ENOMEM;
 }

+static int perf_evsel__alloc_mmap(struct perf_evsel *evsel, int ncpus, int nthreads)
+{
+	evsel->mmap = xyarray__new(ncpus, nthreads, sizeof(struct perf_mmap));
+
+	return evsel->mmap != NULL ? 0 : -ENOMEM;
+}
+
 static int
 sys_perf_event_open(struct perf_event_attr *attr,
 		    pid_t pid, int cpu, int group_fd,
@@ -156,6 +166,72 @@ void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu)
 	perf_evsel__close_fd_cpu(evsel, cpu);
 }

+void perf_evsel__munmap(struct perf_evsel *evsel)
+{
+	int cpu, thread;
+
+	if (evsel->fd == NULL || evsel->mmap == NULL)
+		return;
+
+	for (cpu = 0; cpu < xyarray__max_x(evsel->fd); cpu++) {
+		for (thread = 0; thread < xyarray__max_y(evsel->fd); thread++) {
+			int fd = FD(evsel, cpu, thread);
+			struct perf_mmap *map = MMAP(evsel, cpu, thread);
+
+			if (fd < 0)
+				continue;
+
+			perf_mmap__munmap(map);
+		}
+	}
+
+	xyarray__delete(evsel->mmap);
+	evsel->mmap = NULL;
+}
+
+int perf_evsel__mmap(struct perf_evsel *evsel, int pages)
+{
+	int ret, cpu, thread;
+	struct perf_mmap_param mp = {
+		.prot = PROT_READ | PROT_WRITE,
+		.mask = (pages * page_size) - 1,
+	};
+
+	if (evsel->fd == NULL || evsel->mmap)
+		return -EINVAL;
+
+	if (perf_evsel__alloc_mmap(evsel, xyarray__max_x(evsel->fd), xyarray__max_y(evsel->fd)) < 0)
+		return -ENOMEM;
+
+	for (cpu = 0; cpu < xyarray__max_x(evsel->fd); cpu++) {
+		for (thread = 0; thread < xyarray__max_y(evsel->fd); thread++) {
+			int fd = FD(evsel, cpu, thread);
+			struct perf_mmap *map = MMAP(evsel, cpu, thread);
+
+			if (fd < 0)
+				continue;
+
+			perf_mmap__init(map, NULL, false, NULL);
+
+			ret = perf_mmap__mmap(map, &mp, fd, cpu);
+			if (ret) {
+				perf_evsel__munmap(evsel);
+				return ret;
+			}
+		}
+	}
+
+	return 0;
+}
+
+void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
+{
+	if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
+		return NULL;
+
+	return MMAP(evsel, cpu, thread)->base;
+}
+
 int perf_evsel__read_size(struct perf_evsel *evsel)
 {
 	u64 read_format = evsel->attr.read_format;
diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
index 1ffd083b235e..1c067d088bc6 100644
--- a/tools/lib/perf/include/internal/evsel.h
+++ b/tools/lib/perf/include/internal/evsel.h
@@ -41,6 +41,7 @@ struct perf_evsel {
 	struct perf_cpu_map	*own_cpus;
 	struct perf_thread_map	*threads;
 	struct xyarray		*fd;
+	struct xyarray		*mmap;
 	struct xyarray		*sample_id;
 	u64			*id;
 	u32			 ids;
diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
index c82ec39a4ad0..60eae25076d3 100644
--- a/tools/lib/perf/include/perf/evsel.h
+++ b/tools/lib/perf/include/perf/evsel.h
@@ -27,6 +27,9 @@ LIBPERF_API int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *
 				 struct perf_thread_map *threads);
 LIBPERF_API void perf_evsel__close(struct perf_evsel *evsel);
 LIBPERF_API void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
+LIBPERF_API int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
+LIBPERF_API void perf_evsel__munmap(struct perf_evsel *evsel);
+LIBPERF_API void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
 LIBPERF_API int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
 				 struct perf_counts_values *count);
 LIBPERF_API int perf_evsel__enable(struct perf_evsel *evsel);
diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
index 7be1af8a546c..c0c7ceb11060 100644
--- a/tools/lib/perf/libperf.map
+++ b/tools/lib/perf/libperf.map
@@ -23,6 +23,9 @@ LIBPERF_0.0.1 {
 		perf_evsel__disable;
 		perf_evsel__open;
 		perf_evsel__close;
+		perf_evsel__mmap;
+		perf_evsel__munmap;
+		perf_evsel__mmap_base;
 		perf_evsel__read;
 		perf_evsel__cpus;
 		perf_evsel__threads;
--
2.27.0

[PATCH v8 3/4] libperf: tests: Add support for verbose printing

[Rob Herring]

Add __T_VERBOSE() so tests can add verbose output. The verbose output is
enabled with the '-v' command line option. Running 'make tests V=1' will
enable the '-v' option when running the tests.

Signed-off-by: Rob Herring <robh@kernel.org>
---
v8:
 - Update commit msg with running 'make tests V=1'.
v5:
 - Pass verbose flag to static tests
 - Fix getopt loop with unsigned char (arm64)
v3:
 - New patch
---
 tools/lib/perf/include/internal/tests.h | 32 +++++++++++++++++++++++++
 tools/lib/perf/tests/Makefile           |  6 +++--
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/tools/lib/perf/include/internal/tests.h b/tools/lib/perf/include/internal/tests.h
index 2093e8868a67..29425c2dabe1 100644
--- a/tools/lib/perf/include/internal/tests.h
+++ b/tools/lib/perf/include/internal/tests.h
@@ -3,11 +3,32 @@
 #define __LIBPERF_INTERNAL_TESTS_H

 #include <stdio.h>
+#include <unistd.h>

 int tests_failed;
+int tests_verbose;
+
+static inline int get_verbose(char **argv, int argc)
+{
+	int c;
+	int verbose = 0;
+
+	while ((c = getopt(argc, argv, "v")) != -1) {
+		switch (c)
+		{
+		case 'v':
+			verbose = 1;
+			break;
+		default:
+			break;
+		}
+	}
+	return verbose;
+}

 #define __T_START					\
 do {							\
+	tests_verbose = get_verbose(argv, argc);	\
 	fprintf(stdout, "- running %s...", __FILE__);	\
 	fflush(NULL);					\
 	tests_failed = 0;				\
@@ -30,4 +51,15 @@ do {
 	}                                                                        \
 } while (0)

+#define __T_VERBOSE(...)						\
+do {									\
+	if (tests_verbose) {						\
+		if (tests_verbose == 1) {				\
+			fputc('\n', stderr);				\
+			tests_verbose++;				\
+		}							\
+		fprintf(stderr, ##__VA_ARGS__);				\
+	}								\
+} while (0)
+
 #endif /* __LIBPERF_INTERNAL_TESTS_H */
diff --git a/tools/lib/perf/tests/Makefile b/tools/lib/perf/tests/Makefile
index 96841775feaf..b536cc9a26dd 100644
--- a/tools/lib/perf/tests/Makefile
+++ b/tools/lib/perf/tests/Makefile
@@ -5,6 +5,8 @@ TESTS = test-cpumap test-threadmap test-evlist test-evsel
 TESTS_SO := $(addsuffix -so,$(TESTS))
 TESTS_A  := $(addsuffix -a,$(TESTS))

+TEST_ARGS := $(if $(V),-v)
+
 # Set compile option CFLAGS
 ifdef EXTRA_CFLAGS
   CFLAGS := $(EXTRA_CFLAGS)
@@ -28,9 +30,9 @@ all: $(TESTS_A) $(TESTS_SO)

 run:
 	@echo "running static:"
-	@for i in $(TESTS_A); do ./$$i; done
+	@for i in $(TESTS_A); do ./$$i $(TEST_ARGS); done
 	@echo "running dynamic:"
-	@for i in $(TESTS_SO); do LD_LIBRARY_PATH=../ ./$$i; done
+	@for i in $(TESTS_SO); do LD_LIBRARY_PATH=../ ./$$i $(TEST_ARGS); done

 clean:
 	$(call QUIET_CLEAN, tests)$(RM) $(TESTS_A) $(TESTS_SO)
--
2.27.0

[PATCH v8 4/4] libperf: Add support for user space counter access

[Rob Herring]

x86 and arm64 can both support direct access of event counters in
userspace. The access sequence is less than trivial and currently exists
in perf test code (tools/perf/arch/x86/tests/rdpmc.c) with copies in
projects such as PAPI and libpfm4.

In order to support usersapce access, an event must be mmapped first
with perf_evsel__mmap(). Then subsequent calls to perf_evsel__read()
will use the fast path (assuming the arch supports it).

Signed-off-by: Rob Herring <robh@kernel.org>
---
v8:
 - Add call to perf_evsel_munmap() in evsel test
v6:
 - Adapt to mmap changes adding MMAP NULL check
v5:
 - Make raw count s64 instead of u64 so that counter width shifting
   works
 - Adapt to mmap changes
v4:
 - Update perf_evsel__mmap size to pages
v3:
 - Split out perf_evsel__mmap() to separate patch
---
 tools/lib/perf/evsel.c                 |  4 ++
 tools/lib/perf/include/internal/mmap.h |  3 +
 tools/lib/perf/mmap.c                  | 88 ++++++++++++++++++++++++++
 tools/lib/perf/tests/test-evsel.c      | 66 +++++++++++++++++++
 4 files changed, 161 insertions(+)

diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index cd203998e875..bd8c2f19ef74 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -267,6 +267,10 @@ int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
 	if (FD(evsel, cpu, thread) < 0)
 		return -EINVAL;

+	if (MMAP(evsel, cpu, thread) &&
+	    !perf_mmap__read_self(MMAP(evsel, cpu, thread), count))
+		return 0;
+
 	if (readn(FD(evsel, cpu, thread), count->values, size) <= 0)
 		return -errno;

diff --git a/tools/lib/perf/include/internal/mmap.h b/tools/lib/perf/include/internal/mmap.h
index be7556e0a2b2..5e3422f40ed5 100644
--- a/tools/lib/perf/include/internal/mmap.h
+++ b/tools/lib/perf/include/internal/mmap.h
@@ -11,6 +11,7 @@
 #define PERF_SAMPLE_MAX_SIZE (1 << 16)

 struct perf_mmap;
+struct perf_counts_values;

 typedef void (*libperf_unmap_cb_t)(struct perf_mmap *map);

@@ -52,4 +53,6 @@ void perf_mmap__put(struct perf_mmap *map);

 u64 perf_mmap__read_head(struct perf_mmap *map);

+int perf_mmap__read_self(struct perf_mmap *map, struct perf_counts_values *count);
+
 #endif /* __LIBPERF_INTERNAL_MMAP_H */
diff --git a/tools/lib/perf/mmap.c b/tools/lib/perf/mmap.c
index 79d5ed6c38cc..915469f00cf4 100644
--- a/tools/lib/perf/mmap.c
+++ b/tools/lib/perf/mmap.c
@@ -8,9 +8,11 @@
 #include <linux/perf_event.h>
 #include <perf/mmap.h>
 #include <perf/event.h>
+#include <perf/evsel.h>
 #include <internal/mmap.h>
 #include <internal/lib.h>
 #include <linux/kernel.h>
+#include <linux/math64.h>
 #include "internal.h"

 void perf_mmap__init(struct perf_mmap *map, struct perf_mmap *prev,
@@ -273,3 +275,89 @@ union perf_event *perf_mmap__read_event(struct perf_mmap *map)

 	return event;
 }
+
+#if defined(__i386__) || defined(__x86_64__)
+static u64 read_perf_counter(unsigned int counter)
+{
+	unsigned int low, high;
+
+	asm volatile("rdpmc" : "=a" (low), "=d" (high) : "c" (counter));
+
+	return low | ((u64)high) << 32;
+}
+
+static u64 read_timestamp(void)
+{
+	unsigned int low, high;
+
+	asm volatile("rdtsc" : "=a" (low), "=d" (high));
+
+	return low | ((u64)high) << 32;
+}
+#else
+static u64 read_perf_counter(unsigned int counter) { return 0; }
+static u64 read_timestamp(void) { return 0; }
+#endif
+
+int perf_mmap__read_self(struct perf_mmap *map, struct perf_counts_values *count)
+{
+	struct perf_event_mmap_page *pc = map->base;
+	u32 seq, idx, time_mult = 0, time_shift = 0;
+	u64 cnt, cyc = 0, time_offset = 0, time_cycles = 0, time_mask = ~0ULL;
+
+	if (!pc || !pc->cap_user_rdpmc)
+		return -1;
+
+	do {
+		seq = READ_ONCE(pc->lock);
+		barrier();
+
+		count->ena = READ_ONCE(pc->time_enabled);
+		count->run = READ_ONCE(pc->time_running);
+
+		if (pc->cap_user_time && count->ena != count->run) {
+			cyc = read_timestamp();
+			time_mult = READ_ONCE(pc->time_mult);
+			time_shift = READ_ONCE(pc->time_shift);
+			time_offset = READ_ONCE(pc->time_offset);
+
+			if (pc->cap_user_time_short) {
+				time_cycles = READ_ONCE(pc->time_cycles);
+				time_mask = READ_ONCE(pc->time_mask);
+			}
+		}
+
+		idx = READ_ONCE(pc->index);
+		cnt = READ_ONCE(pc->offset);
+		if (pc->cap_user_rdpmc && idx) {
+			s64 evcnt = read_perf_counter(idx - 1);
+			u16 width = READ_ONCE(pc->pmc_width);
+
+			evcnt <<= 64 - width;
+			evcnt >>= 64 - width;
+			cnt += evcnt;
+		} else
+			return -1;
+
+		barrier();
+	} while (READ_ONCE(pc->lock) != seq);
+
+	if (count->ena != count->run) {
+		u64 delta;
+
+		/* Adjust for cap_usr_time_short, a nop if not */
+		cyc = time_cycles + ((cyc - time_cycles) & time_mask);
+
+		delta = time_offset + mul_u64_u32_shr(cyc, time_mult, time_shift);
+
+		count->ena += delta;
+		if (idx)
+			count->run += delta;
+
+		cnt = mul_u64_u64_div64(cnt, count->ena, count->run);
+	}
+
+	count->val = cnt;
+
+	return 0;
+}
diff --git a/tools/lib/perf/tests/test-evsel.c b/tools/lib/perf/tests/test-evsel.c
index 0ad82d7a2a51..288b5feaefe2 100644
--- a/tools/lib/perf/tests/test-evsel.c
+++ b/tools/lib/perf/tests/test-evsel.c
@@ -120,6 +120,70 @@ static int test_stat_thread_enable(void)
 	return 0;
 }

+static int test_stat_user_read(int event)
+{
+	struct perf_counts_values counts = { .val = 0 };
+	struct perf_thread_map *threads;
+	struct perf_evsel *evsel;
+	struct perf_event_mmap_page *pc;
+	struct perf_event_attr attr = {
+		.type	= PERF_TYPE_HARDWARE,
+		.config	= event,
+	};
+	int err, i;
+
+	threads = perf_thread_map__new_dummy();
+	__T("failed to create threads", threads);
+
+	perf_thread_map__set_pid(threads, 0, 0);
+
+	evsel = perf_evsel__new(&attr);
+	__T("failed to create evsel", evsel);
+
+	err = perf_evsel__open(evsel, NULL, threads);
+	__T("failed to open evsel", err == 0);
+
+	err = perf_evsel__mmap(evsel, 0);
+	__T("failed to mmap evsel", err == 0);
+
+	pc = perf_evsel__mmap_base(evsel, 0, 0);
+
+#if defined(__i386__) || defined(__x86_64__)
+	__T("userspace counter access not supported", pc->cap_user_rdpmc);
+	__T("userspace counter access not enabled", pc->index);
+	__T("userspace counter width not set", pc->pmc_width >= 32);
+#endif
+
+	perf_evsel__read(evsel, 0, 0, &counts);
+	__T("failed to read value for evsel", counts.val != 0);
+
+	for (i = 0; i < 5; i++) {
+		volatile int count = 0x10000 << i;
+		__u64 start, end, last = 0;
+
+		__T_VERBOSE("\tloop = %u, ", count);
+
+		perf_evsel__read(evsel, 0, 0, &counts);
+		start = counts.val;
+
+		while (count--) ;
+
+		perf_evsel__read(evsel, 0, 0, &counts);
+		end = counts.val;
+
+		__T("invalid counter data", (end - start) > last);
+		last = end - start;
+		__T_VERBOSE("count = %llu\n", end - start);
+	}
+
+	perf_evsel__munmap(evsel);
+	perf_evsel__close(evsel);
+	perf_evsel__delete(evsel);
+
+	perf_thread_map__put(threads);
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	__T_START;
@@ -129,6 +193,8 @@ int main(int argc, char **argv)
 	test_stat_cpu();
 	test_stat_thread();
 	test_stat_thread_enable();
+	test_stat_user_read(PERF_COUNT_HW_INSTRUCTIONS);
+	test_stat_user_read(PERF_COUNT_HW_CPU_CYCLES);

 	__T_END;
 	return tests_failed == 0 ? 0 : -1;
--
2.27.0

Re: [PATCH v8 2/4] libperf: Add evsel mmap support

[Namhyung Kim]

Hello,

On Thu, Apr 15, 2021 at 1:07 AM Rob Herring <robh@kernel.org> wrote:
> +void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
> +{
> +       if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
> +               return NULL;

I think you should check the cpu and the thread is in
a valid range.  Currently xyarray__entry() simply accesses
the content without checking the boundaries.

Thanks,
Namhyung


> +
> +       return MMAP(evsel, cpu, thread)->base;
> +}
> +
>  int perf_evsel__read_size(struct perf_evsel *evsel)
>  {
>         u64 read_format = evsel->attr.read_format;
> diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
> index 1ffd083b235e..1c067d088bc6 100644
> --- a/tools/lib/perf/include/internal/evsel.h
> +++ b/tools/lib/perf/include/internal/evsel.h
> @@ -41,6 +41,7 @@ struct perf_evsel {
>         struct perf_cpu_map     *own_cpus;
>         struct perf_thread_map  *threads;
>         struct xyarray          *fd;
> +       struct xyarray          *mmap;
>         struct xyarray          *sample_id;
>         u64                     *id;
>         u32                      ids;
> diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
> index c82ec39a4ad0..60eae25076d3 100644
> --- a/tools/lib/perf/include/perf/evsel.h
> +++ b/tools/lib/perf/include/perf/evsel.h
> @@ -27,6 +27,9 @@ LIBPERF_API int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *
>                                  struct perf_thread_map *threads);
>  LIBPERF_API void perf_evsel__close(struct perf_evsel *evsel);
>  LIBPERF_API void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
> +LIBPERF_API int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
> +LIBPERF_API void perf_evsel__munmap(struct perf_evsel *evsel);
> +LIBPERF_API void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
>  LIBPERF_API int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
>                                  struct perf_counts_values *count);
>  LIBPERF_API int perf_evsel__enable(struct perf_evsel *evsel);
> diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
> index 7be1af8a546c..c0c7ceb11060 100644
> --- a/tools/lib/perf/libperf.map
> +++ b/tools/lib/perf/libperf.map
> @@ -23,6 +23,9 @@ LIBPERF_0.0.1 {
>                 perf_evsel__disable;
>                 perf_evsel__open;
>                 perf_evsel__close;
> +               perf_evsel__mmap;
> +               perf_evsel__munmap;
> +               perf_evsel__mmap_base;
>                 perf_evsel__read;
>                 perf_evsel__cpus;
>                 perf_evsel__threads;
> --
> 2.27.0

Re: [PATCH v8 2/4] libperf: Add evsel mmap support

[Rob Herring]

On Wed, Apr 14, 2021 at 11:41 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hello,
>
> On Thu, Apr 15, 2021 at 1:07 AM Rob Herring <robh@kernel.org> wrote:
> > +void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
> > +{
> > +       if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
> > +               return NULL;
>
> I think you should check the cpu and the thread is in
> a valid range.  Currently xyarray__entry() simply accesses
> the content without checking the boundaries.

Happy to add a patch to do that if desired, but I think that's
separate from this series. That would be something to add to
xyarray__entry().

Rob

Re: [PATCH v8 2/4] libperf: Add evsel mmap support

[Namhyung Kim]

On Thu, Apr 15, 2021 at 1:53 AM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Apr 14, 2021 at 11:41 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hello,
> >
> > On Thu, Apr 15, 2021 at 1:07 AM Rob Herring <robh@kernel.org> wrote:
> > > +void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
> > > +{
> > > +       if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
> > > +               return NULL;
> >
> > I think you should check the cpu and the thread is in
> > a valid range.  Currently xyarray__entry() simply accesses
> > the content without checking the boundaries.
>
> Happy to add a patch to do that if desired, but I think that's
> separate from this series. That would be something to add to
> xyarray__entry().

Sure, we can do that separately.

Thanks,
Namhyung

Re: [PATCH v8 2/4] libperf: Add evsel mmap support

[Arnaldo Carvalho de Melo]

Em Thu, Apr 15, 2021 at 01:41:35AM +0900, Namhyung Kim escreveu:
> Hello,
> 
> On Thu, Apr 15, 2021 at 1:07 AM Rob Herring <robh@kernel.org> wrote:
> > +void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
> > +{
> > +       if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
> > +               return NULL;
> 
> I think you should check the cpu and the thread is in
> a valid range.  Currently xyarray__entry() simply accesses
> the content without checking the boundaries.

So, since xyarray has the bounds, it should check it, i.e. we need to
have a __xyarray__entry() that is what xyarray__entry() does, i.e.
assume the values have been bounds checked, then a new method,
xyarray__entry() that does bounds check, if it fails, return NULL,
otherwise calls __xyarray__entry().

I see this is frustrating and I should've chimed in earlier, but at
least now this is getting traction, and the end result will be better
not just for the feature you've been dilligently working on,

Thank you for your persistence,

- Arnaldo
 
> Thanks,
> Namhyung
> 
> 
> > +
> > +       return MMAP(evsel, cpu, thread)->base;
> > +}
> > +
> >  int perf_evsel__read_size(struct perf_evsel *evsel)
> >  {
> >         u64 read_format = evsel->attr.read_format;
> > diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
> > index 1ffd083b235e..1c067d088bc6 100644
> > --- a/tools/lib/perf/include/internal/evsel.h
> > +++ b/tools/lib/perf/include/internal/evsel.h
> > @@ -41,6 +41,7 @@ struct perf_evsel {
> >         struct perf_cpu_map     *own_cpus;
> >         struct perf_thread_map  *threads;
> >         struct xyarray          *fd;
> > +       struct xyarray          *mmap;
> >         struct xyarray          *sample_id;
> >         u64                     *id;
> >         u32                      ids;
> > diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
> > index c82ec39a4ad0..60eae25076d3 100644
> > --- a/tools/lib/perf/include/perf/evsel.h
> > +++ b/tools/lib/perf/include/perf/evsel.h
> > @@ -27,6 +27,9 @@ LIBPERF_API int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *
> >                                  struct perf_thread_map *threads);
> >  LIBPERF_API void perf_evsel__close(struct perf_evsel *evsel);
> >  LIBPERF_API void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
> > +LIBPERF_API int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
> > +LIBPERF_API void perf_evsel__munmap(struct perf_evsel *evsel);
> > +LIBPERF_API void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
> >  LIBPERF_API int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
> >                                  struct perf_counts_values *count);
> >  LIBPERF_API int perf_evsel__enable(struct perf_evsel *evsel);
> > diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
> > index 7be1af8a546c..c0c7ceb11060 100644
> > --- a/tools/lib/perf/libperf.map
> > +++ b/tools/lib/perf/libperf.map
> > @@ -23,6 +23,9 @@ LIBPERF_0.0.1 {
> >                 perf_evsel__disable;
> >                 perf_evsel__open;
> >                 perf_evsel__close;
> > +               perf_evsel__mmap;
> > +               perf_evsel__munmap;
> > +               perf_evsel__mmap_base;
> >                 perf_evsel__read;
> >                 perf_evsel__cpus;
> >                 perf_evsel__threads;
> > --
> > 2.27.0

-- 

- Arnaldo

Re: [PATCH v8 2/4] libperf: Add evsel mmap support

[Arnaldo Carvalho de Melo]

Em Wed, Apr 14, 2021 at 03:02:08PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Apr 15, 2021 at 01:41:35AM +0900, Namhyung Kim escreveu:
> > Hello,
> > 
> > On Thu, Apr 15, 2021 at 1:07 AM Rob Herring <robh@kernel.org> wrote:
> > > +void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
> > > +{
> > > +       if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
> > > +               return NULL;
> > 
> > I think you should check the cpu and the thread is in
> > a valid range.  Currently xyarray__entry() simply accesses
> > the content without checking the boundaries.
> 
> So, since xyarray has the bounds, it should check it, i.e. we need to
> have a __xyarray__entry() that is what xyarray__entry() does, i.e.
> assume the values have been bounds checked, then a new method,
> xyarray__entry() that does bounds check, if it fails, return NULL,
> otherwise calls __xyarray__entry().
> 
> I see this is frustrating and I should've chimed in earlier, but at
> least now this is getting traction, and the end result will be better
> not just for the feature you've been dilligently working on,
> 
> Thank you for your persistence,

Re-reading, yeah, this can be done in a separate patch, Namhyung, can I
have your Reviewed-by? That or an Acked-by?

- Arnaldo
 
> - Arnaldo
>  
> > Thanks,
> > Namhyung
> > 
> > 
> > > +
> > > +       return MMAP(evsel, cpu, thread)->base;
> > > +}
> > > +
> > >  int perf_evsel__read_size(struct perf_evsel *evsel)
> > >  {
> > >         u64 read_format = evsel->attr.read_format;
> > > diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
> > > index 1ffd083b235e..1c067d088bc6 100644
> > > --- a/tools/lib/perf/include/internal/evsel.h
> > > +++ b/tools/lib/perf/include/internal/evsel.h
> > > @@ -41,6 +41,7 @@ struct perf_evsel {
> > >         struct perf_cpu_map     *own_cpus;
> > >         struct perf_thread_map  *threads;
> > >         struct xyarray          *fd;
> > > +       struct xyarray          *mmap;
> > >         struct xyarray          *sample_id;
> > >         u64                     *id;
> > >         u32                      ids;
> > > diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
> > > index c82ec39a4ad0..60eae25076d3 100644
> > > --- a/tools/lib/perf/include/perf/evsel.h
> > > +++ b/tools/lib/perf/include/perf/evsel.h
> > > @@ -27,6 +27,9 @@ LIBPERF_API int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *
> > >                                  struct perf_thread_map *threads);
> > >  LIBPERF_API void perf_evsel__close(struct perf_evsel *evsel);
> > >  LIBPERF_API void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
> > > +LIBPERF_API int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
> > > +LIBPERF_API void perf_evsel__munmap(struct perf_evsel *evsel);
> > > +LIBPERF_API void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
> > >  LIBPERF_API int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
> > >                                  struct perf_counts_values *count);
> > >  LIBPERF_API int perf_evsel__enable(struct perf_evsel *evsel);
> > > diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
> > > index 7be1af8a546c..c0c7ceb11060 100644
> > > --- a/tools/lib/perf/libperf.map
> > > +++ b/tools/lib/perf/libperf.map
> > > @@ -23,6 +23,9 @@ LIBPERF_0.0.1 {
> > >                 perf_evsel__disable;
> > >                 perf_evsel__open;
> > >                 perf_evsel__close;
> > > +               perf_evsel__mmap;
> > > +               perf_evsel__munmap;
> > > +               perf_evsel__mmap_base;
> > >                 perf_evsel__read;
> > >                 perf_evsel__cpus;
> > >                 perf_evsel__threads;
> > > --
> > > 2.27.0
> 
> -- 
> 
> - Arnaldo

-- 

- Arnaldo

Re: [PATCH v8 2/4] libperf: Add evsel mmap support

[Namhyung Kim]

On Thu, Apr 15, 2021 at 3:23 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Wed, Apr 14, 2021 at 03:02:08PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Apr 15, 2021 at 01:41:35AM +0900, Namhyung Kim escreveu:
> > > Hello,
> > >
> > > On Thu, Apr 15, 2021 at 1:07 AM Rob Herring <robh@kernel.org> wrote:
> > > > +void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
> > > > +{
> > > > +       if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
> > > > +               return NULL;
> > >
> > > I think you should check the cpu and the thread is in
> > > a valid range.  Currently xyarray__entry() simply accesses
> > > the content without checking the boundaries.
> >
> > So, since xyarray has the bounds, it should check it, i.e. we need to
> > have a __xyarray__entry() that is what xyarray__entry() does, i.e.
> > assume the values have been bounds checked, then a new method,
> > xyarray__entry() that does bounds check, if it fails, return NULL,
> > otherwise calls __xyarray__entry().
> >
> > I see this is frustrating and I should've chimed in earlier, but at
> > least now this is getting traction, and the end result will be better
> > not just for the feature you've been dilligently working on,
> >
> > Thank you for your persistence,
>
> Re-reading, yeah, this can be done in a separate patch, Namhyung, can I
> have your Reviewed-by? That or an Acked-by?

Sure, for the series:

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

Re: [PATCH v8 0/4] libperf userspace counter access

[Jiri Olsa]

On Wed, Apr 14, 2021 at 10:54:08AM -0500, Rob Herring wrote:
> I'm resending just the libperf userspace counter access without the Arm 
> bits so hopefully it can be picked up for 5.13. The Arm bits seem to be 
> a never ending review filled with long periods of silence. :(
> 
> Prior versions are here[1][2][3][4][5][6][7][8].
> 
> Rob
> 
> [1] https://lore.kernel.org/r/20190822144220.27860-1-raphael.gault@arm.com/
> [2] https://lore.kernel.org/r/20200707205333.624938-1-robh@kernel.org/
> [3] https://lore.kernel.org/r/20200828205614.3391252-1-robh@kernel.org/
> [4] https://lore.kernel.org/r/20200911215118.2887710-1-robh@kernel.org/
> [5] https://lore.kernel.org/r/20201001140116.651970-1-robh@kernel.org/
> [6] https://lore.kernel.org/r/20210114020605.3943992-1-robh@kernel.org/
> [7] https://lore.kernel.org/r/20210311000837.3630499-1-robh@kernel.org/
> [8] https://lore.kernel.org/r/20210413171606.1825808-1-robh@kernel.org/
> 
> Rob Herring (4):
>   tools/include: Add an initial math64.h
>   libperf: Add evsel mmap support
>   libperf: tests: Add support for verbose printing
>   libperf: Add support for user space counter access

Acked-by: Jiri Olsa <jolsa@redhat.com>

thanks,
jirka

> 
>  tools/include/linux/math64.h             | 75 ++++++++++++++++++++
>  tools/lib/perf/Documentation/libperf.txt |  3 +
>  tools/lib/perf/evsel.c                   | 80 +++++++++++++++++++++
>  tools/lib/perf/include/internal/evsel.h  |  1 +
>  tools/lib/perf/include/internal/mmap.h   |  3 +
>  tools/lib/perf/include/internal/tests.h  | 32 +++++++++
>  tools/lib/perf/include/perf/evsel.h      |  3 +
>  tools/lib/perf/libperf.map               |  3 +
>  tools/lib/perf/mmap.c                    | 88 ++++++++++++++++++++++++
>  tools/lib/perf/tests/Makefile            |  6 +-
>  tools/lib/perf/tests/test-evsel.c        | 66 ++++++++++++++++++
>  11 files changed, 358 insertions(+), 2 deletions(-)
>  create mode 100644 tools/include/linux/math64.h
> 
> -- 
> 2.27.0
> 

Re: [PATCH v8 2/4] libperf: Add evsel mmap support

[Arnaldo Carvalho de Melo]

Em Thu, Apr 15, 2021 at 04:14:31AM +0900, Namhyung Kim escreveu:
> On Thu, Apr 15, 2021 at 3:23 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Wed, Apr 14, 2021 at 03:02:08PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Thu, Apr 15, 2021 at 01:41:35AM +0900, Namhyung Kim escreveu:
> > > > Hello,
> > > >
> > > > On Thu, Apr 15, 2021 at 1:07 AM Rob Herring <robh@kernel.org> wrote:
> > > > > +void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
> > > > > +{
> > > > > +       if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
> > > > > +               return NULL;
> > > >
> > > > I think you should check the cpu and the thread is in
> > > > a valid range.  Currently xyarray__entry() simply accesses
> > > > the content without checking the boundaries.
> > >
> > > So, since xyarray has the bounds, it should check it, i.e. we need to
> > > have a __xyarray__entry() that is what xyarray__entry() does, i.e.
> > > assume the values have been bounds checked, then a new method,
> > > xyarray__entry() that does bounds check, if it fails, return NULL,
> > > otherwise calls __xyarray__entry().
> > >
> > > I see this is frustrating and I should've chimed in earlier, but at
> > > least now this is getting traction, and the end result will be better
> > > not just for the feature you've been dilligently working on,
> > >
> > > Thank you for your persistence,
> >
> > Re-reading, yeah, this can be done in a separate patch, Namhyung, can I
> > have your Reviewed-by? That or an Acked-by?
> 
> Sure, for the series:
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Ok, b4 failed on it, probably some missing Reply to, so I'll apply it by
hand:

[acme@five perf]$ b4 am -t -s -l --cc-trailers 20210414155412.3697605-1-robh@kernel.org
Looking up https://lore.kernel.org/r/20210414155412.3697605-1-robh%40kernel.org
Grabbing thread from lore.kernel.org/lkml
Analyzing 11 messages in the thread
---
Thread incomplete, attempting to backfill
---
Writing ./v8_20210414_robh_libperf_userspace_counter_access.mbx
  [PATCH v8 1/4] tools/include: Add an initial math64.h
    + Acked-by: Namhyung Kim <namhyung@kernel.org>
    + Acked-by: Jiri Olsa <jolsa@redhat.com> (✓ DKIM/redhat.com)
    + Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
    + Link: https://lore.kernel.org/r/20210414155412.3697605-2-robh@kernel.org
    + Cc: Catalin Marinas <catalin.marinas@arm.com>
    + Cc: Mark Rutland <mark.rutland@arm.com>
    + Cc: Itaru Kitayama <itaru.kitayama@gmail.com>
    + Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
    + Cc: Will Deacon <will@kernel.org>
    + Cc: Ingo Molnar <mingo@redhat.com>
    + Cc: linux-kernel@vger.kernel.org
  ERROR: missing [2/4]!
  [PATCH v8 3/4] libperf: tests: Add support for verbose printing
    + Acked-by: Jiri Olsa <jolsa@redhat.com> (✓ DKIM/redhat.com)
    + Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
    + Link: https://lore.kernel.org/r/20210414155412.3697605-3-robh@kernel.org
    + Cc: Catalin Marinas <catalin.marinas@arm.com>
    + Cc: Mark Rutland <mark.rutland@arm.com>
    + Cc: Itaru Kitayama <itaru.kitayama@gmail.com>
    + Cc: Peter Zijlstra <peterz@infradead.org>
    + Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
    + Cc: Namhyung Kim <namhyung@kernel.org>
    + Cc: Will Deacon <will@kernel.org>
    + Cc: Ingo Molnar <mingo@redhat.com>
    + Cc: linux-kernel@vger.kernel.org
  [PATCH v8 4/4] libperf: Add support for user space counter access
    + Acked-by: Jiri Olsa <jolsa@redhat.com> (✓ DKIM/redhat.com)
    + Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
    + Link: https://lore.kernel.org/r/20210414155412.3697605-4-robh@kernel.org
    + Cc: Catalin Marinas <catalin.marinas@arm.com>
    + Cc: Mark Rutland <mark.rutland@arm.com>
    + Cc: Itaru Kitayama <itaru.kitayama@gmail.com>
    + Cc: Peter Zijlstra <peterz@infradead.org>
    + Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
    + Cc: Namhyung Kim <namhyung@kernel.org>
    + Cc: Will Deacon <will@kernel.org>
    + Cc: Ingo Molnar <mingo@redhat.com>
    + Cc: linux-kernel@vger.kernel.org
---
Total patches: 3
---
WARNING: Thread incomplete!
Cover: ./v8_20210414_robh_libperf_userspace_counter_access.cover
 Link: https://lore.kernel.org/r/20210414155412.3697605-1-robh@kernel.org
 Base: not found
       git am ./v8_20210414_robh_libperf_userspace_counter_access.mbx
[acme@five perf]$

Re: [PATCH v8 2/4] libperf: Add evsel mmap support

[Rob Herring]

On Thu, Apr 15, 2021 at 2:37 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Thu, Apr 15, 2021 at 04:14:31AM +0900, Namhyung Kim escreveu:
> > On Thu, Apr 15, 2021 at 3:23 AM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> > >
> > > Em Wed, Apr 14, 2021 at 03:02:08PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > Em Thu, Apr 15, 2021 at 01:41:35AM +0900, Namhyung Kim escreveu:
> > > > > Hello,
> > > > >
> > > > > On Thu, Apr 15, 2021 at 1:07 AM Rob Herring <robh@kernel.org> wrote:
> > > > > > +void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
> > > > > > +{
> > > > > > +       if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
> > > > > > +               return NULL;
> > > > >
> > > > > I think you should check the cpu and the thread is in
> > > > > a valid range.  Currently xyarray__entry() simply accesses
> > > > > the content without checking the boundaries.
> > > >
> > > > So, since xyarray has the bounds, it should check it, i.e. we need to
> > > > have a __xyarray__entry() that is what xyarray__entry() does, i.e.
> > > > assume the values have been bounds checked, then a new method,
> > > > xyarray__entry() that does bounds check, if it fails, return NULL,
> > > > otherwise calls __xyarray__entry().
> > > >
> > > > I see this is frustrating and I should've chimed in earlier, but at
> > > > least now this is getting traction, and the end result will be better
> > > > not just for the feature you've been dilligently working on,
> > > >
> > > > Thank you for your persistence,
> > >
> > > Re-reading, yeah, this can be done in a separate patch, Namhyung, can I
> > > have your Reviewed-by? That or an Acked-by?
> >
> > Sure, for the series:
> >
> > Acked-by: Namhyung Kim <namhyung@kernel.org>
>
> Ok, b4 failed on it, probably some missing Reply to, so I'll apply it by
> hand:

That's my fault. A duplicate message-id is the issue. git-send-email
died after patch 1/4 (can't say I've ever had that happen). So in my
attempt to manually resend 2-4, I was off by 1 in the message-id and
duplicated patch 1's message-id. I should have just resent the whole
thing.

Rob

Re: [PATCH v8 2/4] libperf: Add evsel mmap support

[Arnaldo Carvalho de Melo]

Em Thu, Apr 15, 2021 at 03:09:28PM -0500, Rob Herring escreveu:
> On Thu, Apr 15, 2021 at 2:37 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > Ok, b4 failed on it, probably some missing Reply to, so I'll apply it by
> > hand:
> 
> That's my fault. A duplicate message-id is the issue. git-send-email
> died after patch 1/4 (can't say I've ever had that happen). So in my
> attempt to manually resend 2-4, I was off by 1 in the message-id and
> duplicated patch 1's message-id. I should have just resent the whole
> thing.

No problem, it is already in, just letting you know to fix your scripts
:-)

- Arnaldo

Re: [PATCH v8 4/4] libperf: Add support for user space counter access

[Arnaldo Carvalho de Melo]

Em Wed, Apr 14, 2021 at 11:07:39AM -0500, Rob Herring escreveu:
> x86 and arm64 can both support direct access of event counters in
> userspace. The access sequence is less than trivial and currently exists
> in perf test code (tools/perf/arch/x86/tests/rdpmc.c) with copies in
> projects such as PAPI and libpfm4.
> 
> In order to support usersapce access, an event must be mmapped first
> with perf_evsel__mmap(). Then subsequent calls to perf_evsel__read()
> will use the fast path (assuming the arch supports it).

Had to apply this to fix the build on the other arches:
 
> +#if defined(__i386__) || defined(__x86_64__)
> +static u64 read_perf_counter(unsigned int counter)
> +{
> +	unsigned int low, high;
> +
> +	asm volatile("rdpmc" : "=a" (low), "=d" (high) : "c" (counter));
> +
> +	return low | ((u64)high) << 32;
> +}
> +
> +static u64 read_timestamp(void)
> +{
> +	unsigned int low, high;
> +
> +	asm volatile("rdtsc" : "=a" (low), "=d" (high));
> +
> +	return low | ((u64)high) << 32;
> +}
> +#else
> +static u64 read_perf_counter(unsigned int counter) { return 0; }
> +static u64 read_timestamp(void) { return 0; }
> +#endif

diff --git a/tools/lib/perf/mmap.c b/tools/lib/perf/mmap.c
index 915469f00cf4c3fb..c89dfa5f67b3a408 100644
--- a/tools/lib/perf/mmap.c
+++ b/tools/lib/perf/mmap.c
@@ -295,7 +295,7 @@ static u64 read_timestamp(void)
 	return low | ((u64)high) << 32;
 }
 #else
-static u64 read_perf_counter(unsigned int counter) { return 0; }
+static u64 read_perf_counter(unsigned int counter __maybe_unused) { return 0; }
 static u64 read_timestamp(void) { return 0; }
 #endif