2020-09-11 21:52:31

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH v3 0/10] libperf and arm64 userspace counter access support

This is resurrecting Raphael's series[1] to enable userspace counter
access on arm64. My previous versions are here[2][3].

Changes in v3:
- Dropped removing x86 rdpmc test until libperf tests can run via 'perf test'
- Added verbose prints for tests
- Split adding perf_evsel__mmap() to separate patch


The following changes to the arm64 support have been made compared to
Raphael's last version:

The major change is support for heterogeneous systems with some
restrictions. Specifically, userspace must pin itself to like CPUs, open
a specific PMU by type, and use h/w specific events. The tests have been
reworked to demonstrate this.

Chained events are not supported. The problem with supporting chained
events was there's no way to distinguish between a chained event and a
native 64-bit counter. We could add some flag, but do self monitoring
processes really need that? Native 64-bit counters are supported if the
PMU h/w has support. As there's already an explicit ABI to request 64-bit
counters, userspace can request 64-bit counters and if user
access is not enabled, then it must retry with 32-bit counters.

Prior versions broke the build on arm32 (surprisingly never caught by
0-day). As a result, event_mapped and event_unmapped implementations have
been moved into the arm64 code.

There was a bug in that pmc_width was not set in the user page. The tests
now check for this.

The documentation has been converted to rST. I've added sections on
chained events and heterogeneous.

The tests have been expanded to test the cycle counter access.

Rob

[1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
[2] https://lore.kernel.org/linux-arm-kernel/[email protected]/
[3] https://lore.kernel.org/linux-arm-kernel/[email protected]/

Raphael Gault (4):
arm64: pmu: Add hook to handle pmu-related undefined instructions
arm64: pmu: Add function implementation to update event index in
userpage
arm64: perf: Enable pmu counter direct access for perf event on armv8
Documentation: arm64: Document PMU counters access from userspace

Rob Herring (6):
tools/include: Add an initial math64.h
libperf: Add libperf_evsel__mmap()
libperf: tests: Add support for verbose printing
libperf: Add support for user space counter access
libperf: Add arm64 support to perf_mmap__read_self()
perf: arm64: Add test for userspace counter access on heterogeneous
systems

Documentation/arm64/index.rst | 1 +
.../arm64/perf_counter_user_access.rst | 56 ++++++
arch/arm64/include/asm/mmu.h | 5 +
arch/arm64/include/asm/mmu_context.h | 2 +
arch/arm64/include/asm/perf_event.h | 14 ++
arch/arm64/kernel/cpufeature.c | 4 +-
arch/arm64/kernel/perf_event.c | 116 +++++++++++
include/linux/perf/arm_pmu.h | 2 +
tools/include/linux/math64.h | 75 +++++++
tools/lib/perf/Documentation/libperf.txt | 1 +
tools/lib/perf/evsel.c | 34 ++++
tools/lib/perf/include/internal/evsel.h | 2 +
tools/lib/perf/include/internal/mmap.h | 3 +
tools/lib/perf/include/internal/tests.h | 32 +++
tools/lib/perf/include/perf/evsel.h | 2 +
tools/lib/perf/libperf.map | 1 +
tools/lib/perf/mmap.c | 188 ++++++++++++++++++
tools/lib/perf/tests/Makefile | 4 +-
tools/lib/perf/tests/test-evsel.c | 63 ++++++
tools/perf/arch/arm64/include/arch-tests.h | 7 +
tools/perf/arch/arm64/tests/Build | 1 +
tools/perf/arch/arm64/tests/arch-tests.c | 4 +
tools/perf/arch/arm64/tests/user-events.c | 170 ++++++++++++++++
23 files changed, 784 insertions(+), 3 deletions(-)
create mode 100644 Documentation/arm64/perf_counter_user_access.rst
create mode 100644 tools/include/linux/math64.h
create mode 100644 tools/perf/arch/arm64/tests/user-events.c

--
2.25.1


2020-09-11 21:52:48

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH v3 01/10] arm64: pmu: Add hook to handle pmu-related undefined instructions

From: Raphael Gault <[email protected]>

This patch introduces a protection for the userspace processes which are
trying to access the registers from the pmu registers on a big.LITTLE
environment. It introduces a hook to handle undefined instructions.

The goal here is to prevent the process to be interrupted by a signal
when the error is caused by the task being scheduled while accessing
a counter, causing the counter access to be invalid. As we are not able
to know efficiently the number of counters available physically on both
pmu in that context we consider that any faulting access to a counter
which is architecturally correct should not cause a SIGILL signal if
the permissions are set accordingly.

This commit also modifies the mask of the mrs_hook declared in
arch/arm64/kernel/cpufeatures.c which emulates only feature register
access. This is necessary because this hook's mask was too large and
thus masking any mrs instruction, even if not related to the emulated
registers which made the pmu emulation inefficient.

Signed-off-by: Raphael Gault <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
---
v2:
- Fix warning for set but unused sys_reg
---
arch/arm64/kernel/cpufeature.c | 4 +--
arch/arm64/kernel/perf_event.c | 54 ++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index a389b999482e..00bf53ffd9b0 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2811,8 +2811,8 @@ static int emulate_mrs(struct pt_regs *regs, u32 insn)
}

static struct undef_hook mrs_hook = {
- .instr_mask = 0xfff00000,
- .instr_val = 0xd5300000,
+ .instr_mask = 0xffff0000,
+ .instr_val = 0xd5380000,
.pstate_mask = PSR_AA32_MODE_MASK,
.pstate_val = PSR_MODE_EL0t,
.fn = emulate_mrs,
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 462f9a9cc44b..70538ae684da 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -8,9 +8,11 @@
* This code is based heavily on the ARMv7 perf event code.
*/

+#include <asm/cpu.h>
#include <asm/irq_regs.h>
#include <asm/perf_event.h>
#include <asm/sysreg.h>
+#include <asm/traps.h>
#include <asm/virt.h>

#include <clocksource/arm_arch_timer.h>
@@ -1016,6 +1018,58 @@ static int armv8pmu_probe_pmu(struct arm_pmu *cpu_pmu)
return probe.present ? 0 : -ENODEV;
}

+static int emulate_pmu(struct pt_regs *regs, u32 insn)
+{
+ u32 rt;
+ u32 pmuserenr;
+
+ rt = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RT, insn);
+ pmuserenr = read_sysreg(pmuserenr_el0);
+
+ if ((pmuserenr & (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR)) !=
+ (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR))
+ return -EINVAL;
+
+
+ /*
+ * Userspace is expected to only use this in the context of the scheme
+ * described in the struct perf_event_mmap_page comments.
+ *
+ * Given that context, we can only get here if we got migrated between
+ * getting the register index and doing the MSR read. This in turn
+ * implies we'll fail the sequence and retry, so any value returned is
+ * 'good', all we need is to be non-fatal.
+ *
+ * The choice of the value 0 is comming from the fact that when
+ * accessing a register which is not counting events but is accessible,
+ * we get 0.
+ */
+ pt_regs_write_reg(regs, rt, 0);
+
+ arm64_skip_faulting_instruction(regs, 4);
+ return 0;
+}
+
+/*
+ * This hook will only be triggered by mrs
+ * instructions on PMU registers. This is mandatory
+ * in order to have a consistent behaviour even on
+ * big.LITTLE systems.
+ */
+static struct undef_hook pmu_hook = {
+ .instr_mask = 0xffff8800,
+ .instr_val = 0xd53b8800,
+ .fn = emulate_pmu,
+};
+
+static int __init enable_pmu_emulation(void)
+{
+ register_undef_hook(&pmu_hook);
+ return 0;
+}
+
+core_initcall(enable_pmu_emulation);
+
static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
int (*map_event)(struct perf_event *event),
const struct attribute_group *events,
--
2.25.1

2020-09-11 21:53:04

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH v3 02/10] arm64: pmu: Add function implementation to update event index in userpage

From: Raphael Gault <[email protected]>

In order to be able to access the counter directly for userspace,
we need to provide the index of the counter using the userpage.
We thus need to override the event_idx function to retrieve and
convert the perf_event index to armv8 hardware index.

Since the arm_pmu driver can be used by any implementation, even
if not armv8, two components play a role into making sure the
behaviour is correct and consistent with the PMU capabilities:

* the ARMPMU_EL0_RD_CNTR flag which denotes the capability to access
counter from userspace.
* the event_idx call back, which is implemented and initialized by
the PMU implementation: if no callback is provided, the default
behaviour applies, returning 0 as index value.

Signed-off-by: Raphael Gault <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
---
arch/arm64/kernel/perf_event.c | 21 +++++++++++++++++++++
include/linux/perf/arm_pmu.h | 2 ++
2 files changed, 23 insertions(+)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 70538ae684da..2727d126cecd 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -820,6 +820,22 @@ static void armv8pmu_clear_event_idx(struct pmu_hw_events *cpuc,
clear_bit(idx - 1, cpuc->used_mask);
}

+static int armv8pmu_access_event_idx(struct perf_event *event)
+{
+ if (!(event->hw.flags & ARMPMU_EL0_RD_CNTR))
+ return 0;
+
+ /*
+ * We remap the cycle counter index to 32 to
+ * match the offset applied to the rest of
+ * the counter indices.
+ */
+ if (event->hw.idx == ARMV8_IDX_CYCLE_COUNTER)
+ return 32;
+
+ return event->hw.idx;
+}
+
/*
* Add an event filter to a given event.
*/
@@ -916,6 +932,9 @@ static int __armv8_pmuv3_map_event(struct perf_event *event,
if (armv8pmu_event_is_64bit(event))
event->hw.flags |= ARMPMU_EVT_64BIT;

+ if (!armv8pmu_event_is_chained(event))
+ event->hw.flags |= ARMPMU_EL0_RD_CNTR;
+
/* Only expose micro/arch events supported by this PMU */
if ((hw_event_id > 0) && (hw_event_id < ARMV8_PMUV3_MAX_COMMON_EVENTS)
&& test_bit(hw_event_id, armpmu->pmceid_bitmap)) {
@@ -1092,6 +1111,8 @@ static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
cpu_pmu->set_event_filter = armv8pmu_set_event_filter;
cpu_pmu->filter_match = armv8pmu_filter_match;

+ cpu_pmu->pmu.event_idx = armv8pmu_access_event_idx;
+
cpu_pmu->name = name;
cpu_pmu->map_event = map_event;
cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_EVENTS] = events ?
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 5b616dde9a4c..74fbbbd29dc7 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -26,6 +26,8 @@
*/
/* Event uses a 64bit counter */
#define ARMPMU_EVT_64BIT 1
+/* Allow access to hardware counter from userspace */
+#define ARMPMU_EL0_RD_CNTR 2

#define HW_OP_UNSUPPORTED 0xFFFF
#define C(_x) PERF_COUNT_HW_CACHE_##_x
--
2.25.1

2020-09-11 21:53:24

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH v3 04/10] tools/include: Add an initial math64.h

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/[email protected]/

Cc: Peter Zijlstra <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
---
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.25.1

2020-09-11 21:53:34

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH v3 05/10] libperf: Add libperf_evsel__mmap()

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 a new
perf_evsel__mmap() function to mmap an evsel. This allows implementing
userspace access as a fastpath for perf_evsel__read().

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

Signed-off-by: Rob Herring <[email protected]>
---
v3:
- New patch split out from user access patch
---
tools/lib/perf/Documentation/libperf.txt | 1 +
tools/lib/perf/evsel.c | 31 ++++++++++++++++++++++++
tools/lib/perf/include/internal/evsel.h | 2 ++
tools/lib/perf/include/perf/evsel.h | 2 ++
tools/lib/perf/libperf.map | 1 +
5 files changed, 37 insertions(+)

diff --git a/tools/lib/perf/Documentation/libperf.txt b/tools/lib/perf/Documentation/libperf.txt
index 0c74c30ed23a..0b4694ce42b9 100644
--- a/tools/lib/perf/Documentation/libperf.txt
+++ b/tools/lib/perf/Documentation/libperf.txt
@@ -136,6 +136,7 @@ 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);
+ void *perf_evsel__mmap(struct perf_evsel *evsel, size_t length);
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..99fa53dc0887 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)
{
@@ -156,6 +158,35 @@ void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu)
perf_evsel__close_fd_cpu(evsel, cpu);
}

+void *perf_evsel__mmap(struct perf_evsel *evsel, size_t length)
+{
+ int ret;
+ struct perf_mmap *map;
+ struct perf_mmap_param mp = {
+ .prot = PROT_READ | PROT_WRITE,
+ };
+
+ if (FD(evsel, 0, 0) < 0)
+ return NULL;
+
+ mp.mask = length - page_size - 1;
+
+ map = zalloc(sizeof(*map));
+ if (!map)
+ return NULL;
+
+ perf_mmap__init(map, NULL, false, NULL);
+
+ ret = perf_mmap__mmap(map, &mp, FD(evsel, 0, 0), 0);
+ if (ret) {
+ free(map);
+ return NULL;
+ }
+
+ evsel->mmap = map;
+ return map->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..a7985dbb68ff 100644
--- a/tools/lib/perf/include/internal/evsel.h
+++ b/tools/lib/perf/include/internal/evsel.h
@@ -9,6 +9,7 @@

struct perf_cpu_map;
struct perf_thread_map;
+struct perf_mmap;
struct xyarray;

/*
@@ -40,6 +41,7 @@ struct perf_evsel {
struct perf_cpu_map *cpus;
struct perf_cpu_map *own_cpus;
struct perf_thread_map *threads;
+ struct perf_mmap *mmap;
struct xyarray *fd;
struct xyarray *sample_id;
u64 *id;
diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
index c82ec39a4ad0..28f1354e52df 100644
--- a/tools/lib/perf/include/perf/evsel.h
+++ b/tools/lib/perf/include/perf/evsel.h
@@ -3,6 +3,7 @@
#define __LIBPERF_EVSEL_H

#include <stdint.h>
+#include <stddef.h>
#include <perf/core.h>

struct perf_evsel;
@@ -27,6 +28,7 @@ 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 void *perf_evsel__mmap(struct perf_evsel *evsel, size_t length);
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..733a0647be8b 100644
--- a/tools/lib/perf/libperf.map
+++ b/tools/lib/perf/libperf.map
@@ -23,6 +23,7 @@ LIBPERF_0.0.1 {
perf_evsel__disable;
perf_evsel__open;
perf_evsel__close;
+ perf_evsel__mmap;
perf_evsel__read;
perf_evsel__cpus;
perf_evsel__threads;
--
2.25.1

2020-09-11 21:53:43

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH v3 09/10] perf: arm64: Add test for userspace counter access on heterogeneous systems

Userspace counter access only works on heterogeneous systems with some
restrictions. The userspace process must be pinned to a homogeneous
subset of CPUs and must open the corresponding PMU for those CPUs. This
commit adds a test implementing these requirements.

Signed-off-by: Rob Herring <[email protected]>
---
v2:
- Drop all but heterogeneous test as others covered by libperf tests
- Rework to use libperf
---
tools/perf/arch/arm64/include/arch-tests.h | 7 +
tools/perf/arch/arm64/tests/Build | 1 +
tools/perf/arch/arm64/tests/arch-tests.c | 4 +
tools/perf/arch/arm64/tests/user-events.c | 170 +++++++++++++++++++++
4 files changed, 182 insertions(+)
create mode 100644 tools/perf/arch/arm64/tests/user-events.c

diff --git a/tools/perf/arch/arm64/include/arch-tests.h b/tools/perf/arch/arm64/include/arch-tests.h
index 90ec4c8cb880..380ad34a3f09 100644
--- a/tools/perf/arch/arm64/include/arch-tests.h
+++ b/tools/perf/arch/arm64/include/arch-tests.h
@@ -2,11 +2,18 @@
#ifndef ARCH_TESTS_H
#define ARCH_TESTS_H

+#include <linux/compiler.h>
+
#ifdef HAVE_DWARF_UNWIND_SUPPORT
struct thread;
struct perf_sample;
+int test__arch_unwind_sample(struct perf_sample *sample,
+ struct thread *thread);
#endif

extern struct test arch_tests[];
+int test__rd_pinned(struct test __maybe_unused *test,
+ int __maybe_unused subtest);
+

#endif
diff --git a/tools/perf/arch/arm64/tests/Build b/tools/perf/arch/arm64/tests/Build
index a61c06bdb757..3f9a20c17fc6 100644
--- a/tools/perf/arch/arm64/tests/Build
+++ b/tools/perf/arch/arm64/tests/Build
@@ -1,4 +1,5 @@
perf-y += regs_load.o
perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o

+perf-y += user-events.o
perf-y += arch-tests.o
diff --git a/tools/perf/arch/arm64/tests/arch-tests.c b/tools/perf/arch/arm64/tests/arch-tests.c
index 5b1543c98022..80ce7bd3c16d 100644
--- a/tools/perf/arch/arm64/tests/arch-tests.c
+++ b/tools/perf/arch/arm64/tests/arch-tests.c
@@ -10,6 +10,10 @@ struct test arch_tests[] = {
.func = test__dwarf_unwind,
},
#endif
+ {
+ .desc = "Pinned CPU user counter access",
+ .func = test__rd_pinned,
+ },
{
.func = NULL,
},
diff --git a/tools/perf/arch/arm64/tests/user-events.c b/tools/perf/arch/arm64/tests/user-events.c
new file mode 100644
index 000000000000..9cf30adf39d9
--- /dev/null
+++ b/tools/perf/arch/arm64/tests/user-events.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <unistd.h>
+#include <sched.h>
+#include <cpumap.h>
+
+#include <perf/core.h>
+#include <perf/threadmap.h>
+#include <perf/evsel.h>
+
+#include "pmu.h"
+#include "debug.h"
+#include "tests/tests.h"
+#include "arch-tests.h"
+
+static int run_test(struct perf_evsel *evsel)
+{
+ int n;
+ volatile int tmp = 0;
+ u64 delta, i, loops = 1000;
+ struct perf_counts_values counts = { .val = 0 };
+
+ for (n = 0; n < 6; n++) {
+ u64 stamp, now;
+
+ perf_evsel__read(evsel, 0, 0, &counts);
+ stamp = counts.val;
+
+ for (i = 0; i < loops; i++)
+ tmp++;
+
+ perf_evsel__read(evsel, 0, 0, &counts);
+ now = counts.val;
+ loops *= 10;
+
+ delta = now - stamp;
+ pr_debug("%14d: %14llu\n", n, (long long)delta);
+
+ if (!delta)
+ break;
+ }
+ return delta ? 0 : -1;
+}
+
+static struct perf_pmu *pmu_for_cpu(int cpu)
+{
+ int acpu, idx;
+ struct perf_pmu *pmu = NULL;
+
+ while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+ if (pmu->is_uncore)
+ continue;
+ perf_cpu_map__for_each_cpu(acpu, idx, pmu->cpus)
+ if (acpu == cpu)
+ return pmu;
+ }
+ return NULL;
+}
+
+static bool pmu_is_homogeneous(void)
+{
+ int core_cnt = 0;
+ struct perf_pmu *pmu = NULL;
+
+ while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+ if (!pmu->is_uncore && !perf_cpu_map__empty(pmu->cpus))
+ core_cnt++;
+ }
+ return core_cnt == 1;
+}
+
+static int libperf_print(enum libperf_print_level level,
+ const char *fmt, va_list ap)
+{
+ (void)level;
+ return vfprintf(stderr, fmt, ap);
+}
+
+static struct perf_evsel *perf_init(struct perf_event_attr *attr)
+{
+ int err;
+ struct perf_thread_map *threads;
+ struct perf_evsel *evsel;
+
+ libperf_init(libperf_print);
+
+ threads = perf_thread_map__new_dummy();
+ if (!threads) {
+ pr_err("failed to create threads\n");
+ return NULL;
+ }
+
+ perf_thread_map__set_pid(threads, 0, 0);
+
+ evsel = perf_evsel__new(attr);
+ if (!evsel) {
+ pr_err("failed to create evsel\n");
+ goto out_thread;
+ }
+
+ err = perf_evsel__open(evsel, NULL, threads);
+ if (err) {
+ pr_err("failed to open evsel\n");
+ goto out_open;
+ }
+
+ if (!perf_evsel__mmap(evsel)) {
+ pr_err("failed to mmap evsel\n");
+ goto out_mmap;
+ }
+
+ return evsel;
+
+out_mmap:
+ perf_evsel__close(evsel);
+out_open:
+ perf_evsel__delete(evsel);
+out_thread:
+ perf_thread_map__put(threads);
+ return NULL;
+}
+
+int test__rd_pinned(struct test __maybe_unused *test,
+ int __maybe_unused subtest)
+{
+ int cpu, cputmp, ret = -1;
+ struct perf_evsel *evsel;
+ struct perf_event_attr attr = {
+ .config = 0x8, /* Instruction count */
+ .config1 = 0, /* 32-bit counter */
+ .exclude_kernel = 1,
+ };
+ cpu_set_t cpu_set;
+ struct perf_pmu *pmu;
+
+ if (pmu_is_homogeneous())
+ return TEST_SKIP;
+
+ cpu = sched_getcpu();
+ pmu = pmu_for_cpu(cpu);
+ if (!pmu)
+ return -1;
+ attr.type = pmu->type;
+
+ CPU_ZERO(&cpu_set);
+ perf_cpu_map__for_each_cpu(cpu, cputmp, pmu->cpus)
+ CPU_SET(cpu, &cpu_set);
+ if (sched_setaffinity(0, sizeof(cpu_set), &cpu_set) < 0)
+ pr_err("Could not set affinity\n");
+
+ evsel = perf_init(&attr);
+ if (!evsel)
+ return -1;
+
+ perf_cpu_map__for_each_cpu(cpu, cputmp, pmu->cpus) {
+ CPU_ZERO(&cpu_set);
+ CPU_SET(cpu, &cpu_set);
+ if (sched_setaffinity(0, sizeof(cpu_set), &cpu_set) < 0)
+ pr_err("Could not set affinity\n");
+
+ pr_debug("Running on CPU %d\n", cpu);
+
+ ret = run_test(evsel);
+ if (ret)
+ break;
+ }
+
+ perf_evsel__close(evsel);
+ perf_evsel__delete(evsel);
+ return ret;
+}
--
2.25.1

2020-09-11 21:53:49

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH v3 06/10] libperf: tests: Add support for verbose printing

Add __T_VERBOSE() so tests can add verbose output. The verbose output is
enabled with the '-v' command line option.

Signed-off-by: Rob Herring <[email protected]>
---
v3:
- New patch
---
tools/lib/perf/include/internal/tests.h | 32 +++++++++++++++++++++++++
tools/lib/perf/tests/Makefile | 4 +++-
2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/tools/lib/perf/include/internal/tests.h b/tools/lib/perf/include/internal/tests.h
index 2093e8868a67..27b6e64299e2 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)
+{
+ char 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..9438b385d489 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)
@@ -30,7 +32,7 @@ run:
@echo "running static:"
@for i in $(TESTS_A); do ./$$i; 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.25.1

2020-09-11 21:54:38

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH v3 10/10] Documentation: arm64: Document PMU counters access from userspace

From: Raphael Gault <[email protected]>

Add a documentation file to describe the access to the pmu hardware
counters from userspace

Signed-off-by: Raphael Gault <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
---
v2:
- Update links to test examples

Changes from Raphael's v4:
- Convert to rSt
- Update chained event status
- Add section for heterogeneous systems
---
Documentation/arm64/index.rst | 1 +
.../arm64/perf_counter_user_access.rst | 56 +++++++++++++++++++
2 files changed, 57 insertions(+)
create mode 100644 Documentation/arm64/perf_counter_user_access.rst

diff --git a/Documentation/arm64/index.rst b/Documentation/arm64/index.rst
index d9665d83c53a..c712a08e7627 100644
--- a/Documentation/arm64/index.rst
+++ b/Documentation/arm64/index.rst
@@ -15,6 +15,7 @@ ARM64 Architecture
legacy_instructions
memory
perf
+ perf_counter_user_access
pointer-authentication
silicon-errata
sve
diff --git a/Documentation/arm64/perf_counter_user_access.rst b/Documentation/arm64/perf_counter_user_access.rst
new file mode 100644
index 000000000000..e49e141f10cc
--- /dev/null
+++ b/Documentation/arm64/perf_counter_user_access.rst
@@ -0,0 +1,56 @@
+=============================================
+Access to PMU hardware counter from userspace
+=============================================
+
+Overview
+--------
+The perf userspace tool relies on the PMU to monitor events. It offers an
+abstraction layer over the hardware counters since the underlying
+implementation is cpu-dependent.
+Arm64 allows userspace tools to have access to the registers storing the
+hardware counters' values directly.
+
+This targets specifically self-monitoring tasks in order to reduce the overhead
+by directly accessing the registers without having to go through the kernel.
+
+How-to
+------
+The focus is set on the armv8 pmuv3 which makes sure that the access to the pmu
+registers is enabled and that the userspace has access to the relevant
+information in order to use them.
+
+In order to have access to the hardware counter it is necessary to open the event
+using the perf tool interface: the sys_perf_event_open syscall returns a fd which
+can subsequently be used with the mmap syscall in order to retrieve a page of
+memory containing information about the event.
+The PMU driver uses this page to expose to the user the hardware counter's
+index and other necessary data. Using this index enables the user to access the
+PMU registers using the `mrs` instruction.
+
+The userspace access is supported in libperf using the perf_evsel__mmap()
+and perf_evsel__read() functions. See `tools/lib/perf/tests/test-evsel.c`_ for
+an example.
+
+About heterogeneous systems
+---------------------------
+On heterogeneous systems such as big.LITTLE, userspace PMU counter access can
+only be enabled when the tasks are pinned to a homogeneous subset of cores and
+the corresponding PMU instance is opened by specifying the 'type' attribute.
+The use of generic event types is not supported in this case.
+
+Have a look at `tools/perf/arch/arm64/tests/user-events.c`_ for an example. It
+can be run using the perf tool to check that the access to the registers works
+correctly from userspace:
+
+.. code-block:: sh
+
+ perf test -v user
+
+About chained events
+--------------------
+Chained events are not supported in userspace. If a 64-bit counter is requested,
+userspace access will only be enabled if the underlying counter is 64-bit.
+
+.. Links
+.. _tools/perf/arch/arm64/tests/user-events.c:
+ https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/tools/perf/arch/arm64/tests/user-events.c
--
2.25.1

2020-09-11 21:54:44

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH v3 07/10] libperf: Add support for user space counter access

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 <[email protected]>
---
v3:
- Split out perf_evsel__mmap() to separate patch
---
tools/lib/perf/evsel.c | 3 +
tools/lib/perf/include/internal/mmap.h | 3 +
tools/lib/perf/mmap.c | 90 ++++++++++++++++++++++++++
tools/lib/perf/tests/test-evsel.c | 63 ++++++++++++++++++
4 files changed, 159 insertions(+)

diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index 99fa53dc0887..4164e6524453 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -222,6 +222,9 @@ int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
if (FD(evsel, cpu, thread) < 0)
return -EINVAL;

+ if (evsel->mmap && !perf_mmap__read_self(evsel->mmap, 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..cb07969cfdbf 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,91 @@ 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;
+
+ BUG_ON(!pc);
+
+ if (!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) {
+ u64 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 135722ac965b..a88c12a95e92 100644
--- a/tools/lib/perf/tests/test-evsel.c
+++ b/tools/lib/perf/tests/test-evsel.c
@@ -120,6 +120,67 @@ 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);
+
+ pc = perf_evsel__mmap(evsel, 0x1000);
+ __T("failed to mmap evsel", pc);
+
+#if defined(__i386__) || defined(__x86_64__) || defined(__aarch64__)
+ __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__close(evsel);
+ perf_evsel__delete(evsel);
+
+ perf_thread_map__put(threads);
+ return 0;
+}
+
int main(int argc, char **argv)
{
__T_START;
@@ -129,6 +190,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 0;
--
2.25.1

2020-09-11 21:54:45

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH v3 03/10] arm64: perf: Enable pmu counter direct access for perf event on armv8

From: Raphael Gault <[email protected]>

Keep track of event opened with direct access to the hardware counters
and modify permissions while they are open.

The strategy used here is the same which x86 uses: everytime an event
is mapped, the permissions are set if required. The atomic field added
in the mm_context helps keep track of the different event opened and
de-activate the permissions when all are unmapped.
We also need to update the permissions in the context switch code so
that tasks keep the right permissions.

Signed-off-by: Raphael Gault <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
---
v2:
- Move mapped/unmapped into arm64 code. Fixes arm32.
- Rebase on cap_user_time_short changes

Changes from Raphael's v4:
- Drop homogeneous check
- Disable access for chained counters
- Set pmc_width in user page
---
arch/arm64/include/asm/mmu.h | 5 ++++
arch/arm64/include/asm/mmu_context.h | 2 ++
arch/arm64/include/asm/perf_event.h | 14 ++++++++++
arch/arm64/kernel/perf_event.c | 41 ++++++++++++++++++++++++++++
4 files changed, 62 insertions(+)

diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index a7a5ecaa2e83..52cfdb676f06 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -19,6 +19,11 @@

typedef struct {
atomic64_t id;
+ /*
+ * non-zero if userspace have access to hardware
+ * counters directly.
+ */
+ atomic_t pmu_direct_access;
#ifdef CONFIG_COMPAT
void *sigpage;
#endif
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index f2d7537d6f83..d24589ecb07a 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -21,6 +21,7 @@
#include <asm/proc-fns.h>
#include <asm-generic/mm_hooks.h>
#include <asm/cputype.h>
+#include <asm/perf_event.h>
#include <asm/sysreg.h>
#include <asm/tlbflush.h>

@@ -224,6 +225,7 @@ static inline void __switch_mm(struct mm_struct *next)
}

check_and_switch_context(next);
+ perf_switch_user_access(next);
}

static inline void
diff --git a/arch/arm64/include/asm/perf_event.h b/arch/arm64/include/asm/perf_event.h
index 2c2d7dbe8a02..a025d9595d51 100644
--- a/arch/arm64/include/asm/perf_event.h
+++ b/arch/arm64/include/asm/perf_event.h
@@ -8,6 +8,7 @@

#include <asm/stack_pointer.h>
#include <asm/ptrace.h>
+#include <linux/mm_types.h>

#define ARMV8_PMU_MAX_COUNTERS 32
#define ARMV8_PMU_COUNTER_MASK (ARMV8_PMU_MAX_COUNTERS - 1)
@@ -251,4 +252,17 @@ extern unsigned long perf_misc_flags(struct pt_regs *regs);
(regs)->pstate = PSR_MODE_EL1h; \
}

+static inline void perf_switch_user_access(struct mm_struct *mm)
+{
+ if (!IS_ENABLED(CONFIG_PERF_EVENTS))
+ return;
+
+ if (atomic_read(&mm->context.pmu_direct_access)) {
+ write_sysreg(ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR,
+ pmuserenr_el0);
+ } else {
+ write_sysreg(0, pmuserenr_el0);
+ }
+}
+
#endif
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 2727d126cecd..cf44591f5be1 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -836,6 +836,41 @@ static int armv8pmu_access_event_idx(struct perf_event *event)
return event->hw.idx;
}

+static void refresh_pmuserenr(void *mm)
+{
+ perf_switch_user_access(mm);
+}
+
+static void armv8pmu_event_mapped(struct perf_event *event, struct mm_struct *mm)
+{
+ if (!(event->hw.flags & ARMPMU_EL0_RD_CNTR))
+ return;
+
+ /*
+ * This function relies on not being called concurrently in two
+ * tasks in the same mm. Otherwise one task could observe
+ * pmu_direct_access > 1 and return all the way back to
+ * userspace with user access disabled while another task is still
+ * doing on_each_cpu_mask() to enable user access.
+ *
+ * For now, this can't happen because all callers hold mmap_lock
+ * for write. If this changes, we'll need a different solution.
+ */
+ lockdep_assert_held_write(&mm->mmap_lock);
+
+ if (atomic_inc_return(&mm->context.pmu_direct_access) == 1)
+ on_each_cpu(refresh_pmuserenr, mm, 1);
+}
+
+static void armv8pmu_event_unmapped(struct perf_event *event, struct mm_struct *mm)
+{
+ if (!(event->hw.flags & ARMPMU_EL0_RD_CNTR))
+ return;
+
+ if (atomic_dec_and_test(&mm->context.pmu_direct_access))
+ on_each_cpu_mask(mm_cpumask(mm), refresh_pmuserenr, NULL, 1);
+}
+
/*
* Add an event filter to a given event.
*/
@@ -1112,6 +1147,8 @@ static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
cpu_pmu->filter_match = armv8pmu_filter_match;

cpu_pmu->pmu.event_idx = armv8pmu_access_event_idx;
+ cpu_pmu->pmu.event_mapped = armv8pmu_event_mapped;
+ cpu_pmu->pmu.event_unmapped = armv8pmu_event_unmapped;

cpu_pmu->name = name;
cpu_pmu->map_event = map_event;
@@ -1272,6 +1309,10 @@ void arch_perf_update_userpage(struct perf_event *event,
userpg->cap_user_time = 0;
userpg->cap_user_time_zero = 0;
userpg->cap_user_time_short = 0;
+ userpg->cap_user_rdpmc = !!(event->hw.flags & ARMPMU_EL0_RD_CNTR);
+
+ if (userpg->cap_user_rdpmc)
+ userpg->pmc_width = armv8pmu_event_is_64bit(event) ? 64 : 32;

do {
rd = sched_clock_read_begin(&seq);
--
2.25.1

2020-09-11 21:56:40

by Rob Herring (Arm)

[permalink] [raw]
Subject: [PATCH v3 08/10] libperf: Add arm64 support to perf_mmap__read_self()

Add the arm64 variants for read_perf_counter() and read_timestamp().
Unfortunately the counter number is encoded into the instruction, so the
code is a bit verbose to enumerate all possible counters.

Signed-off-by: Rob Herring <[email protected]>
---
tools/lib/perf/mmap.c | 98 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 98 insertions(+)

diff --git a/tools/lib/perf/mmap.c b/tools/lib/perf/mmap.c
index cb07969cfdbf..6cf939aae6b6 100644
--- a/tools/lib/perf/mmap.c
+++ b/tools/lib/perf/mmap.c
@@ -13,6 +13,7 @@
#include <internal/lib.h>
#include <linux/kernel.h>
#include <linux/math64.h>
+#include <linux/stringify.h>
#include "internal.h"

void perf_mmap__init(struct perf_mmap *map, struct perf_mmap *prev,
@@ -294,6 +295,103 @@ static u64 read_timestamp(void)

return low | ((u64)high) << 32;
}
+#elif defined(__aarch64__)
+#define read_sysreg(r) ({ \
+ u64 __val; \
+ asm volatile("mrs %0, " __stringify(r) : "=r" (__val)); \
+ __val; \
+})
+
+static u64 read_pmccntr(void)
+{
+ return read_sysreg(pmccntr_el0);
+}
+
+#define PMEVCNTR_READ(idx) \
+ static u64 read_pmevcntr_##idx(void) { \
+ return read_sysreg(pmevcntr##idx##_el0); \
+ }
+
+PMEVCNTR_READ(0);
+PMEVCNTR_READ(1);
+PMEVCNTR_READ(2);
+PMEVCNTR_READ(3);
+PMEVCNTR_READ(4);
+PMEVCNTR_READ(5);
+PMEVCNTR_READ(6);
+PMEVCNTR_READ(7);
+PMEVCNTR_READ(8);
+PMEVCNTR_READ(9);
+PMEVCNTR_READ(10);
+PMEVCNTR_READ(11);
+PMEVCNTR_READ(12);
+PMEVCNTR_READ(13);
+PMEVCNTR_READ(14);
+PMEVCNTR_READ(15);
+PMEVCNTR_READ(16);
+PMEVCNTR_READ(17);
+PMEVCNTR_READ(18);
+PMEVCNTR_READ(19);
+PMEVCNTR_READ(20);
+PMEVCNTR_READ(21);
+PMEVCNTR_READ(22);
+PMEVCNTR_READ(23);
+PMEVCNTR_READ(24);
+PMEVCNTR_READ(25);
+PMEVCNTR_READ(26);
+PMEVCNTR_READ(27);
+PMEVCNTR_READ(28);
+PMEVCNTR_READ(29);
+PMEVCNTR_READ(30);
+
+/*
+ * Read a value direct from PMEVCNTR<idx>
+ */
+static u64 read_perf_counter(unsigned int counter)
+{
+ static u64 (* const read_f[])(void) = {
+ read_pmevcntr_0,
+ read_pmevcntr_1,
+ read_pmevcntr_2,
+ read_pmevcntr_3,
+ read_pmevcntr_4,
+ read_pmevcntr_5,
+ read_pmevcntr_6,
+ read_pmevcntr_7,
+ read_pmevcntr_8,
+ read_pmevcntr_9,
+ read_pmevcntr_10,
+ read_pmevcntr_11,
+ read_pmevcntr_13,
+ read_pmevcntr_12,
+ read_pmevcntr_14,
+ read_pmevcntr_15,
+ read_pmevcntr_16,
+ read_pmevcntr_17,
+ read_pmevcntr_18,
+ read_pmevcntr_19,
+ read_pmevcntr_20,
+ read_pmevcntr_21,
+ read_pmevcntr_22,
+ read_pmevcntr_23,
+ read_pmevcntr_24,
+ read_pmevcntr_25,
+ read_pmevcntr_26,
+ read_pmevcntr_27,
+ read_pmevcntr_28,
+ read_pmevcntr_29,
+ read_pmevcntr_30,
+ read_pmccntr
+ };
+
+ if (counter < ARRAY_SIZE(read_f))
+ return (read_f[counter])();
+
+ return 0;
+}
+
+static u64 read_timestamp(void) { return read_sysreg(cntvct_el0); }
+
#else
static u64 read_perf_counter(unsigned int counter) { return 0; }
static u64 read_timestamp(void) { return 0; }
--
2.25.1

2020-09-12 20:54:28

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH v3 0/10] libperf and arm64 userspace counter access support

On Fri, Sep 11, 2020 at 03:51:08PM -0600, Rob Herring wrote:
> This is resurrecting Raphael's series[1] to enable userspace counter
> access on arm64. My previous versions are here[2][3].
>
> Changes in v3:
> - Dropped removing x86 rdpmc test until libperf tests can run via 'perf test'
> - Added verbose prints for tests
> - Split adding perf_evsel__mmap() to separate patch

hi,
which commit/branch did you base on? I can't apply arm bits:

Applying: arm64: pmu: Add hook to handle pmu-related undefined instructions
Applying: arm64: perf: Enable pmu counter direct access for perf event on armv8
error: patch failed: arch/arm64/kernel/perf_event.c:836
error: arch/arm64/kernel/perf_event.c: patch does not apply
Patch failed at 0002 arm64: perf: Enable pmu counter direct access for perf event on armv8

thanks,
jirka

2020-09-14 14:23:47

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 0/10] libperf and arm64 userspace counter access support

On Sat, Sep 12, 2020 at 2:53 PM Jiri Olsa <[email protected]> wrote:
>
> On Fri, Sep 11, 2020 at 03:51:08PM -0600, Rob Herring wrote:
> > This is resurrecting Raphael's series[1] to enable userspace counter
> > access on arm64. My previous versions are here[2][3].
> >
> > Changes in v3:
> > - Dropped removing x86 rdpmc test until libperf tests can run via 'perf test'
> > - Added verbose prints for tests
> > - Split adding perf_evsel__mmap() to separate patch
>
> hi,
> which commit/branch did you base on? I can't apply arm bits:
>
> Applying: arm64: pmu: Add hook to handle pmu-related undefined instructions
> Applying: arm64: perf: Enable pmu counter direct access for perf event on armv8
> error: patch failed: arch/arm64/kernel/perf_event.c:836
> error: arch/arm64/kernel/perf_event.c: patch does not apply
> Patch failed at 0002 arm64: perf: Enable pmu counter direct access for perf event on armv8

It's on v5.9-rc2.

Rob

2020-09-16 02:52:31

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 0/10] libperf and arm64 userspace counter access support

On Mon, Sep 14, 2020 at 5:14 PM Itaru Kitayama <[email protected]> wrote:
>
> Could you point us to your repo too?

Sure, pushed a branch here:

git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git user-perf-event-v3

Rob

2020-09-18 14:35:28

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH v3 05/10] libperf: Add libperf_evsel__mmap()

On Fri, Sep 11, 2020 at 03:51:13PM -0600, Rob Herring wrote:
> 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 a new
> perf_evsel__mmap() function to mmap an evsel. This allows implementing
> userspace access as a fastpath for perf_evsel__read().
>
> The mmapped address is returned by perf_evsel__mmap() primarily for
> users/tests to check if userspace access is enabled.
>
> Signed-off-by: Rob Herring <[email protected]>
> ---
> v3:
> - New patch split out from user access patch
> ---
> tools/lib/perf/Documentation/libperf.txt | 1 +
> tools/lib/perf/evsel.c | 31 ++++++++++++++++++++++++
> tools/lib/perf/include/internal/evsel.h | 2 ++
> tools/lib/perf/include/perf/evsel.h | 2 ++
> tools/lib/perf/libperf.map | 1 +
> 5 files changed, 37 insertions(+)
>
> diff --git a/tools/lib/perf/Documentation/libperf.txt b/tools/lib/perf/Documentation/libperf.txt
> index 0c74c30ed23a..0b4694ce42b9 100644
> --- a/tools/lib/perf/Documentation/libperf.txt
> +++ b/tools/lib/perf/Documentation/libperf.txt
> @@ -136,6 +136,7 @@ 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);
> + void *perf_evsel__mmap(struct perf_evsel *evsel, size_t length);
> 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..99fa53dc0887 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)
> {
> @@ -156,6 +158,35 @@ void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu)
> perf_evsel__close_fd_cpu(evsel, cpu);
> }
>
> +void *perf_evsel__mmap(struct perf_evsel *evsel, size_t length)
> +{

could you rather use 'int pages' insted of length as we
do in perf_evlist__mmap, so we keep the same interface

len = (pages + 1) * page_size

thanks,
jirka

2020-09-19 07:25:03

by Itaru Kitayama

[permalink] [raw]
Subject: Re: [PATCH v3 0/10] libperf and arm64 userspace counter access support

Rob,
V3 fails to build on ThunderX2:

$ make
BUILD: Doing 'make -j224' parallel build

Auto-detecting system features:
... dwarf: [ on ]
... dwarf_getlocations: [ on ]
... glibc: [ on ]
... gtk2: [ OFF ]
... libbfd: [ on ]
... libcap: [ on ]
... libelf: [ on ]
... libnuma: [ on ]
... numa_num_possible_cpus: [ on ]
... libperl: [ on ]
... libpython: [ on ]
... libcrypto: [ on ]
... libunwind: [ OFF ]
... libdw-dwarf-unwind: [ on ]
... zlib: [ on ]
... lzma: [ on ]
... get_cpuid: [ OFF ]
... bpf: [ on ]
... libaio: [ on ]
... libzstd: [ on ]
... disassembler-four-args: [ on ]

Makefile.config:604: No libunwind found. Please install
libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR
Makefile.config:730: GTK2 not found, disables GTK2 support. Please
install gtk2-devel or libgtk2.0-dev
Makefile.config:965: No libbabeltrace found, disables 'perf data' CTF
format support, please install
libbabeltrace-dev[el]/libbabeltrace-ctf-dev
Makefile.config:1009: No openjdk development package found, please
install JDK package, e.g. openjdk-8-jdk, java-1.8.0-openjdk-devel
CC arch/arm64/tests/user-events.o
CC arch/arm64/tests/arch-tests.o
In file included from arch/arm64/tests/arch-tests.c:4:
/home/users/itaru.kitayama/projects/linux/tools/perf/arch/arm64/include/arch-tests.h:10:5:
error: redundant redeclaration of ‘test__arch_unwind_sample’
[-Werror=redundant-decls]
int test__arch_unwind_sample(struct perf_sample *sample,
^~~~~~~~~~~~~~~~~~~~~~~~
In file included from arch/arm64/tests/arch-tests.c:3:
/home/users/itaru.kitayama/projects/linux/tools/perf/tests/tests.h:134:5:
note: previous declaration of ‘test__arch_unwind_sample’ was here
int test__arch_unwind_sample(struct perf_sample *sample,
^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/home/users/itaru.kitayama/projects/linux/tools/build/Makefile.build:97:
arch/arm64/tests/arch-tests.o] Error 1
make[6]: *** Waiting for unfinished jobs....
In file included from arch/arm64/tests/user-events.c:13:
/home/users/itaru.kitayama/projects/linux/tools/perf/arch/arm64/include/arch-tests.h:10:5:
error: redundant redeclaration of ‘test__arch_unwind_sample’
[-Werror=redundant-decls]
int test__arch_unwind_sample(struct perf_sample *sample,
^~~~~~~~~~~~~~~~~~~~~~~~
In file included from arch/arm64/tests/user-events.c:12:
/home/users/itaru.kitayama/projects/linux/tools/perf/tests/tests.h:134:5:
note: previous declaration of ‘test__arch_unwind_sample’ was here
int test__arch_unwind_sample(struct perf_sample *sample,
^~~~~~~~~~~~~~~~~~~~~~~~
arch/arm64/tests/user-events.c: In function ‘perf_init’:
arch/arm64/tests/user-events.c:106:7: error: too few arguments to
function ‘perf_evsel__mmap’
if (!perf_evsel__mmap(evsel)) {
^~~~~~~~~~~~~~~~
In file included from arch/arm64/tests/user-events.c:8:
/home/users/itaru.kitayama/projects/linux/tools/lib/perf/include/perf/evsel.h:31:19:
note: declared here
LIBPERF_API void *perf_evsel__mmap(struct perf_evsel *evsel, size_t length);
^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[6]: *** [/home/users/itaru.kitayama/projects/linux/tools/build/Makefile.build:97:
arch/arm64/tests/user-events.o] Error 1
make[5]: *** [/home/users/itaru.kitayama/projects/linux/tools/build/Makefile.build:139:
tests] Error 2
make[4]: *** [/home/users/itaru.kitayama/projects/linux/tools/build/Makefile.build:139:
arm64] Error 2
make[3]: *** [/home/users/itaru.kitayama/projects/linux/tools/build/Makefile.build:139:
arch] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [Makefile.perf:634: perf-in.o] Error 2
make[1]: *** [Makefile.perf:233: sub-make] Error 2
make: *** [Makefile:70: all] Error 2

On Wed, Sep 16, 2020 at 11:50 AM Rob Herring <[email protected]> wrote:
>
> On Mon, Sep 14, 2020 at 5:14 PM Itaru Kitayama <[email protected]> wrote:
> >
> > Could you point us to your repo too?
>
> Sure, pushed a branch here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git user-perf-event-v3
>
> Rob

2020-09-22 15:26:20

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 0/10] libperf and arm64 userspace counter access support

On Sat, Sep 19, 2020 at 1:22 AM Itaru Kitayama <[email protected]> wrote:
>
> Rob,
> V3 fails to build on ThunderX2:
>
> $ make
> BUILD: Doing 'make -j224' parallel build
>
> Auto-detecting system features:
> ... dwarf: [ on ]
> ... dwarf_getlocations: [ on ]
> ... glibc: [ on ]
> ... gtk2: [ OFF ]
> ... libbfd: [ on ]
> ... libcap: [ on ]
> ... libelf: [ on ]
> ... libnuma: [ on ]
> ... numa_num_possible_cpus: [ on ]
> ... libperl: [ on ]
> ... libpython: [ on ]
> ... libcrypto: [ on ]
> ... libunwind: [ OFF ]
> ... libdw-dwarf-unwind: [ on ]
> ... zlib: [ on ]
> ... lzma: [ on ]
> ... get_cpuid: [ OFF ]
> ... bpf: [ on ]
> ... libaio: [ on ]
> ... libzstd: [ on ]
> ... disassembler-four-args: [ on ]
>
> Makefile.config:604: No libunwind found. Please install

I believe this is an existing problem with your setup?

> libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR
> Makefile.config:730: GTK2 not found, disables GTK2 support. Please
> install gtk2-devel or libgtk2.0-dev
> Makefile.config:965: No libbabeltrace found, disables 'perf data' CTF
> format support, please install
> libbabeltrace-dev[el]/libbabeltrace-ctf-dev
> Makefile.config:1009: No openjdk development package found, please
> install JDK package, e.g. openjdk-8-jdk, java-1.8.0-openjdk-devel
> CC arch/arm64/tests/user-events.o
> CC arch/arm64/tests/arch-tests.o
> In file included from arch/arm64/tests/arch-tests.c:4:
> /home/users/itaru.kitayama/projects/linux/tools/perf/arch/arm64/include/arch-tests.h:10:5:
> error: redundant redeclaration of ‘test__arch_unwind_sample’
> [-Werror=redundant-decls]
> int test__arch_unwind_sample(struct perf_sample *sample,
> ^~~~~~~~~~~~~~~~~~~~~~~~
> In file included from arch/arm64/tests/arch-tests.c:3:
> /home/users/itaru.kitayama/projects/linux/tools/perf/tests/tests.h:134:5:
> note: previous declaration of ‘test__arch_unwind_sample’ was here
> int test__arch_unwind_sample(struct perf_sample *sample,
> ^~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> make[6]: *** [/home/users/itaru.kitayama/projects/linux/tools/build/Makefile.build:97:
> arch/arm64/tests/arch-tests.o] Error 1
> make[6]: *** Waiting for unfinished jobs....
> In file included from arch/arm64/tests/user-events.c:13:
> /home/users/itaru.kitayama/projects/linux/tools/perf/arch/arm64/include/arch-tests.h:10:5:
> error: redundant redeclaration of ‘test__arch_unwind_sample’
> [-Werror=redundant-decls]
> int test__arch_unwind_sample(struct perf_sample *sample,
> ^~~~~~~~~~~~~~~~~~~~~~~~
> In file included from arch/arm64/tests/user-events.c:12:
> /home/users/itaru.kitayama/projects/linux/tools/perf/tests/tests.h:134:5:
> note: previous declaration of ‘test__arch_unwind_sample’ was here
> int test__arch_unwind_sample(struct perf_sample *sample,
> ^~~~~~~~~~~~~~~~~~~~~~~~

These are due to the above issue.

> arch/arm64/tests/user-events.c: In function ‘perf_init’:
> arch/arm64/tests/user-events.c:106:7: error: too few arguments to
> function ‘perf_evsel__mmap’
> if (!perf_evsel__mmap(evsel)) {

I forgot to update this test. Should be 'perf_evsel__mmap(evsel,
SZ_4K)' here, but that's going to change to 'perf_evsel__mmap(evsel,
0)' with it changing from bytes to pages (minus 1).

Rob

2020-09-22 15:32:37

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 05/10] libperf: Add libperf_evsel__mmap()

On Fri, Sep 18, 2020 at 8:33 AM Jiri Olsa <[email protected]> wrote:
>
> On Fri, Sep 11, 2020 at 03:51:13PM -0600, Rob Herring wrote:
> > 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 a new
> > perf_evsel__mmap() function to mmap an evsel. This allows implementing
> > userspace access as a fastpath for perf_evsel__read().
> >
> > The mmapped address is returned by perf_evsel__mmap() primarily for
> > users/tests to check if userspace access is enabled.
> >
> > Signed-off-by: Rob Herring <[email protected]>
> > ---
> > v3:
> > - New patch split out from user access patch
> > ---
> > tools/lib/perf/Documentation/libperf.txt | 1 +
> > tools/lib/perf/evsel.c | 31 ++++++++++++++++++++++++
> > tools/lib/perf/include/internal/evsel.h | 2 ++
> > tools/lib/perf/include/perf/evsel.h | 2 ++
> > tools/lib/perf/libperf.map | 1 +
> > 5 files changed, 37 insertions(+)
> >
> > diff --git a/tools/lib/perf/Documentation/libperf.txt b/tools/lib/perf/Documentation/libperf.txt
> > index 0c74c30ed23a..0b4694ce42b9 100644
> > --- a/tools/lib/perf/Documentation/libperf.txt
> > +++ b/tools/lib/perf/Documentation/libperf.txt
> > @@ -136,6 +136,7 @@ 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);
> > + void *perf_evsel__mmap(struct perf_evsel *evsel, size_t length);
> > 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..99fa53dc0887 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)
> > {
> > @@ -156,6 +158,35 @@ void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu)
> > perf_evsel__close_fd_cpu(evsel, cpu);
> > }
> >
> > +void *perf_evsel__mmap(struct perf_evsel *evsel, size_t length)
> > +{
>
> could you rather use 'int pages' insted of length as we
> do in perf_evlist__mmap, so we keep the same interface
>
> len = (pages + 1) * page_size

Sure, but what's the reason it is pages minus 1? It seems a bit odd to
pass in 0 here to get 1 page which would be the typical usage.

Rob

2020-09-22 18:35:23

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH v3 05/10] libperf: Add libperf_evsel__mmap()

On Tue, Sep 22, 2020 at 09:28:44AM -0600, Rob Herring wrote:
> On Fri, Sep 18, 2020 at 8:33 AM Jiri Olsa <[email protected]> wrote:
> >
> > On Fri, Sep 11, 2020 at 03:51:13PM -0600, Rob Herring wrote:
> > > 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 a new
> > > perf_evsel__mmap() function to mmap an evsel. This allows implementing
> > > userspace access as a fastpath for perf_evsel__read().
> > >
> > > The mmapped address is returned by perf_evsel__mmap() primarily for
> > > users/tests to check if userspace access is enabled.
> > >
> > > Signed-off-by: Rob Herring <[email protected]>
> > > ---
> > > v3:
> > > - New patch split out from user access patch
> > > ---
> > > tools/lib/perf/Documentation/libperf.txt | 1 +
> > > tools/lib/perf/evsel.c | 31 ++++++++++++++++++++++++
> > > tools/lib/perf/include/internal/evsel.h | 2 ++
> > > tools/lib/perf/include/perf/evsel.h | 2 ++
> > > tools/lib/perf/libperf.map | 1 +
> > > 5 files changed, 37 insertions(+)
> > >
> > > diff --git a/tools/lib/perf/Documentation/libperf.txt b/tools/lib/perf/Documentation/libperf.txt
> > > index 0c74c30ed23a..0b4694ce42b9 100644
> > > --- a/tools/lib/perf/Documentation/libperf.txt
> > > +++ b/tools/lib/perf/Documentation/libperf.txt
> > > @@ -136,6 +136,7 @@ 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);
> > > + void *perf_evsel__mmap(struct perf_evsel *evsel, size_t length);
> > > 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..99fa53dc0887 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)
> > > {
> > > @@ -156,6 +158,35 @@ void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu)
> > > perf_evsel__close_fd_cpu(evsel, cpu);
> > > }
> > >
> > > +void *perf_evsel__mmap(struct perf_evsel *evsel, size_t length)
> > > +{
> >
> > could you rather use 'int pages' insted of length as we
> > do in perf_evlist__mmap, so we keep the same interface
> >
> > len = (pages + 1) * page_size
>
> Sure, but what's the reason it is pages minus 1? It seems a bit odd to
> pass in 0 here to get 1 page which would be the typical usage.

the user mmap page is extra, so when you map 2 pages you actualy map 3,
but we want the size of what's available for storing events in the API

jirka

2020-09-28 18:30:04

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v3 01/10] arm64: pmu: Add hook to handle pmu-related undefined instructions

On Fri, Sep 11, 2020 at 03:51:09PM -0600, Rob Herring wrote:
> From: Raphael Gault <[email protected]>
>
> This patch introduces a protection for the userspace processes which are
> trying to access the registers from the pmu registers on a big.LITTLE
> environment. It introduces a hook to handle undefined instructions.
>
> The goal here is to prevent the process to be interrupted by a signal
> when the error is caused by the task being scheduled while accessing
> a counter, causing the counter access to be invalid. As we are not able
> to know efficiently the number of counters available physically on both
> pmu in that context we consider that any faulting access to a counter
> which is architecturally correct should not cause a SIGILL signal if
> the permissions are set accordingly.
>
> This commit also modifies the mask of the mrs_hook declared in
> arch/arm64/kernel/cpufeatures.c which emulates only feature register
> access. This is necessary because this hook's mask was too large and
> thus masking any mrs instruction, even if not related to the emulated
> registers which made the pmu emulation inefficient.
>
> Signed-off-by: Raphael Gault <[email protected]>
> Signed-off-by: Rob Herring <[email protected]>
> ---
> v2:
> - Fix warning for set but unused sys_reg
> ---
> arch/arm64/kernel/cpufeature.c | 4 +--
> arch/arm64/kernel/perf_event.c | 54 ++++++++++++++++++++++++++++++++++
> 2 files changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index a389b999482e..00bf53ffd9b0 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2811,8 +2811,8 @@ static int emulate_mrs(struct pt_regs *regs, u32 insn)
> }
>
> static struct undef_hook mrs_hook = {
> - .instr_mask = 0xfff00000,
> - .instr_val = 0xd5300000,
> + .instr_mask = 0xffff0000,
> + .instr_val = 0xd5380000,
> .pstate_mask = PSR_AA32_MODE_MASK,
> .pstate_val = PSR_MODE_EL0t,
> .fn = emulate_mrs,
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 462f9a9cc44b..70538ae684da 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -8,9 +8,11 @@
> * This code is based heavily on the ARMv7 perf event code.
> */
>
> +#include <asm/cpu.h>
> #include <asm/irq_regs.h>
> #include <asm/perf_event.h>
> #include <asm/sysreg.h>
> +#include <asm/traps.h>
> #include <asm/virt.h>
>
> #include <clocksource/arm_arch_timer.h>
> @@ -1016,6 +1018,58 @@ static int armv8pmu_probe_pmu(struct arm_pmu *cpu_pmu)
> return probe.present ? 0 : -ENODEV;
> }
>
> +static int emulate_pmu(struct pt_regs *regs, u32 insn)
> +{
> + u32 rt;
> + u32 pmuserenr;
> +
> + rt = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RT, insn);
> + pmuserenr = read_sysreg(pmuserenr_el0);
> +
> + if ((pmuserenr & (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR)) !=
> + (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR))
> + return -EINVAL;
> +
> +
> + /*
> + * Userspace is expected to only use this in the context of the scheme
> + * described in the struct perf_event_mmap_page comments.
> + *
> + * Given that context, we can only get here if we got migrated between
> + * getting the register index and doing the MSR read. This in turn
> + * implies we'll fail the sequence and retry, so any value returned is
> + * 'good', all we need is to be non-fatal.
> + *
> + * The choice of the value 0 is comming from the fact that when
> + * accessing a register which is not counting events but is accessible,
> + * we get 0.
> + */
> + pt_regs_write_reg(regs, rt, 0);

Hmm... this feels pretty fragile since, although we may expect userspace only
to trigger this in the context of the specific perf use-case, we don't have
a way to detect that, so the ABI we're exposing is that EL0 accesses to
non-existent counters will return 0. I don't really think that's something
we want to commit to.

When restartable sequences were added to the kernel, one of the proposed
use-cases was to allow PMU access on big/little systems, because the
sequence will abort on preemption. Taking that approach removes the need
for this emulation hook entirely. Is that something we can rely on instead
of this emulation hook?

Cheers,

Will

2020-09-29 13:48:43

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 01/10] arm64: pmu: Add hook to handle pmu-related undefined instructions

On Mon, Sep 28, 2020 at 1:26 PM Will Deacon <[email protected]> wrote:
>
> On Fri, Sep 11, 2020 at 03:51:09PM -0600, Rob Herring wrote:
> > From: Raphael Gault <[email protected]>
> >
> > This patch introduces a protection for the userspace processes which are
> > trying to access the registers from the pmu registers on a big.LITTLE
> > environment. It introduces a hook to handle undefined instructions.
> >
> > The goal here is to prevent the process to be interrupted by a signal
> > when the error is caused by the task being scheduled while accessing
> > a counter, causing the counter access to be invalid. As we are not able
> > to know efficiently the number of counters available physically on both
> > pmu in that context we consider that any faulting access to a counter
> > which is architecturally correct should not cause a SIGILL signal if
> > the permissions are set accordingly.
> >
> > This commit also modifies the mask of the mrs_hook declared in
> > arch/arm64/kernel/cpufeatures.c which emulates only feature register
> > access. This is necessary because this hook's mask was too large and
> > thus masking any mrs instruction, even if not related to the emulated
> > registers which made the pmu emulation inefficient.
> >
> > Signed-off-by: Raphael Gault <[email protected]>
> > Signed-off-by: Rob Herring <[email protected]>
> > ---
> > v2:
> > - Fix warning for set but unused sys_reg
> > ---
> > arch/arm64/kernel/cpufeature.c | 4 +--
> > arch/arm64/kernel/perf_event.c | 54 ++++++++++++++++++++++++++++++++++
> > 2 files changed, 56 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > index a389b999482e..00bf53ffd9b0 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -2811,8 +2811,8 @@ static int emulate_mrs(struct pt_regs *regs, u32 insn)
> > }
> >
> > static struct undef_hook mrs_hook = {
> > - .instr_mask = 0xfff00000,
> > - .instr_val = 0xd5300000,
> > + .instr_mask = 0xffff0000,
> > + .instr_val = 0xd5380000,
> > .pstate_mask = PSR_AA32_MODE_MASK,
> > .pstate_val = PSR_MODE_EL0t,
> > .fn = emulate_mrs,
> > diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> > index 462f9a9cc44b..70538ae684da 100644
> > --- a/arch/arm64/kernel/perf_event.c
> > +++ b/arch/arm64/kernel/perf_event.c
> > @@ -8,9 +8,11 @@
> > * This code is based heavily on the ARMv7 perf event code.
> > */
> >
> > +#include <asm/cpu.h>
> > #include <asm/irq_regs.h>
> > #include <asm/perf_event.h>
> > #include <asm/sysreg.h>
> > +#include <asm/traps.h>
> > #include <asm/virt.h>
> >
> > #include <clocksource/arm_arch_timer.h>
> > @@ -1016,6 +1018,58 @@ static int armv8pmu_probe_pmu(struct arm_pmu *cpu_pmu)
> > return probe.present ? 0 : -ENODEV;
> > }
> >
> > +static int emulate_pmu(struct pt_regs *regs, u32 insn)
> > +{
> > + u32 rt;
> > + u32 pmuserenr;
> > +
> > + rt = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RT, insn);
> > + pmuserenr = read_sysreg(pmuserenr_el0);
> > +
> > + if ((pmuserenr & (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR)) !=
> > + (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR))
> > + return -EINVAL;
> > +
> > +
> > + /*
> > + * Userspace is expected to only use this in the context of the scheme
> > + * described in the struct perf_event_mmap_page comments.
> > + *
> > + * Given that context, we can only get here if we got migrated between
> > + * getting the register index and doing the MSR read. This in turn
> > + * implies we'll fail the sequence and retry, so any value returned is
> > + * 'good', all we need is to be non-fatal.
> > + *
> > + * The choice of the value 0 is comming from the fact that when
> > + * accessing a register which is not counting events but is accessible,
> > + * we get 0.
> > + */
> > + pt_regs_write_reg(regs, rt, 0);
>
> Hmm... this feels pretty fragile since, although we may expect userspace only
> to trigger this in the context of the specific perf use-case, we don't have
> a way to detect that, so the ABI we're exposing is that EL0 accesses to
> non-existent counters will return 0. I don't really think that's something
> we want to commit to.
>
> When restartable sequences were added to the kernel, one of the proposed
> use-cases was to allow PMU access on big/little systems, because the
> sequence will abort on preemption. Taking that approach removes the need
> for this emulation hook entirely. Is that something we can rely on instead
> of this emulation hook?

So back to the RFC version[1]!? That would mean pulling librseq into
the kernel based on the prior discussion. It doesn't look like that
has happened yet.

Why not just drop the undef hook? For heterogeneous systems, we
require userspace to pin itself to cores for a specific PMU. See patch
9. If userspace fails to do that, then it gets to keep the pieces.

Rob

[1] https://lore.kernel.org/linux-arm-kernel/20190625134117.r3gysn7mvzzdrytj@willie-the-truck/

2020-09-29 17:51:17

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v3 01/10] arm64: pmu: Add hook to handle pmu-related undefined instructions

On Tue, Sep 29, 2020 at 08:46:46AM -0500, Rob Herring wrote:
> On Mon, Sep 28, 2020 at 1:26 PM Will Deacon <[email protected]> wrote:
> > On Fri, Sep 11, 2020 at 03:51:09PM -0600, Rob Herring wrote:
> > > +static int emulate_pmu(struct pt_regs *regs, u32 insn)
> > > +{
> > > + u32 rt;
> > > + u32 pmuserenr;
> > > +
> > > + rt = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RT, insn);
> > > + pmuserenr = read_sysreg(pmuserenr_el0);
> > > +
> > > + if ((pmuserenr & (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR)) !=
> > > + (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR))
> > > + return -EINVAL;
> > > +
> > > +
> > > + /*
> > > + * Userspace is expected to only use this in the context of the scheme
> > > + * described in the struct perf_event_mmap_page comments.
> > > + *
> > > + * Given that context, we can only get here if we got migrated between
> > > + * getting the register index and doing the MSR read. This in turn
> > > + * implies we'll fail the sequence and retry, so any value returned is
> > > + * 'good', all we need is to be non-fatal.
> > > + *
> > > + * The choice of the value 0 is comming from the fact that when
> > > + * accessing a register which is not counting events but is accessible,
> > > + * we get 0.
> > > + */
> > > + pt_regs_write_reg(regs, rt, 0);
> >
> > Hmm... this feels pretty fragile since, although we may expect userspace only
> > to trigger this in the context of the specific perf use-case, we don't have
> > a way to detect that, so the ABI we're exposing is that EL0 accesses to
> > non-existent counters will return 0. I don't really think that's something
> > we want to commit to.
> >
> > When restartable sequences were added to the kernel, one of the proposed
> > use-cases was to allow PMU access on big/little systems, because the
> > sequence will abort on preemption. Taking that approach removes the need
> > for this emulation hook entirely. Is that something we can rely on instead
> > of this emulation hook?
>
> So back to the RFC version[1]!? That would mean pulling librseq into
> the kernel based on the prior discussion. It doesn't look like that
> has happened yet.

Yeah, or just don't bother supporting heterogeneous systems with this
for now.

> Why not just drop the undef hook? For heterogeneous systems, we
> require userspace to pin itself to cores for a specific PMU. See patch
> 9. If userspace fails to do that, then it gets to keep the pieces.

Dropping it works too!

Will

2020-09-29 20:49:02

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 01/10] arm64: pmu: Add hook to handle pmu-related undefined instructions

On Tue, Sep 29, 2020 at 12:49 PM Will Deacon <[email protected]> wrote:
>
> On Tue, Sep 29, 2020 at 08:46:46AM -0500, Rob Herring wrote:
> > On Mon, Sep 28, 2020 at 1:26 PM Will Deacon <[email protected]> wrote:
> > > On Fri, Sep 11, 2020 at 03:51:09PM -0600, Rob Herring wrote:
> > > > +static int emulate_pmu(struct pt_regs *regs, u32 insn)
> > > > +{
> > > > + u32 rt;
> > > > + u32 pmuserenr;
> > > > +
> > > > + rt = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RT, insn);
> > > > + pmuserenr = read_sysreg(pmuserenr_el0);
> > > > +
> > > > + if ((pmuserenr & (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR)) !=
> > > > + (ARMV8_PMU_USERENR_ER|ARMV8_PMU_USERENR_CR))
> > > > + return -EINVAL;
> > > > +
> > > > +
> > > > + /*
> > > > + * Userspace is expected to only use this in the context of the scheme
> > > > + * described in the struct perf_event_mmap_page comments.
> > > > + *
> > > > + * Given that context, we can only get here if we got migrated between
> > > > + * getting the register index and doing the MSR read. This in turn
> > > > + * implies we'll fail the sequence and retry, so any value returned is
> > > > + * 'good', all we need is to be non-fatal.
> > > > + *
> > > > + * The choice of the value 0 is comming from the fact that when
> > > > + * accessing a register which is not counting events but is accessible,
> > > > + * we get 0.
> > > > + */
> > > > + pt_regs_write_reg(regs, rt, 0);
> > >
> > > Hmm... this feels pretty fragile since, although we may expect userspace only
> > > to trigger this in the context of the specific perf use-case, we don't have
> > > a way to detect that, so the ABI we're exposing is that EL0 accesses to
> > > non-existent counters will return 0. I don't really think that's something
> > > we want to commit to.
> > >
> > > When restartable sequences were added to the kernel, one of the proposed
> > > use-cases was to allow PMU access on big/little systems, because the
> > > sequence will abort on preemption. Taking that approach removes the need
> > > for this emulation hook entirely. Is that something we can rely on instead
> > > of this emulation hook?
> >
> > So back to the RFC version[1]!? That would mean pulling librseq into
> > the kernel based on the prior discussion. It doesn't look like that
> > has happened yet.
>
> Yeah, or just don't bother supporting heterogeneous systems with this
> for now.

But the people are asking for it. :)

> > Why not just drop the undef hook? For heterogeneous systems, we
> > require userspace to pin itself to cores for a specific PMU. See patch
> > 9. If userspace fails to do that, then it gets to keep the pieces.
>
> Dropping it works too!

Great. I asked Mark R to comment in case I'm forgetting some other reason.

Rob