2017-12-01 14:19:38

by Hendrik Brueckner

[permalink] [raw]
Subject: [PATCH 0/5] bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type

Perf tool bpf selftests revealed a broken uapi for s390 and arm64.
With the BPF_PROG_TYPE_PERF_EVENT program type the bpf_perf_event
structure exports the pt_regs structure for all architectures.

This fails for s390 and arm64 because pt_regs are not part of the
user api and kept in-kernel only. To mitigate the broken uapi,
introduce a wrapper that exports pt_regs in an asm-generic way.
For arm64, export the exising user_pt_regs structure. For s390,
introduce a user_pt_regs structure that exports the beginning of
pt_regs.

Note that user_pt_regs must export from the beginning of pt_regs
as BPF_PROG_TYPE_PERF_EVENT program type is not the only type for
running BPF programs.

Some more background:
For the bpf_perf_event, there is a uapi definition that is
passed to the BPF program. For other "probe" points like
trace points, kprobes, and uprobes, there is no uapi and the
BPF program is always passed pt_regs (which is OK as the BPF
program runs in the kernel context). The perf tool can attach
BPF programs to all of these "probe" points and, optionally,
can create a BPF prologue to access particular arguments
(passed as registers). For this, it uses DWARF/CFI
information to obtain the register and calls a perf-arch
backend function, regs_query_register_offset(). This function
returns the index into (user_)pt_regs for a particular
register. Then, perf creates a BPF prologue that accesses
this register based on the passed stucture from the "probe"
point.

Part of this series, are also updates to the testing and bpf selftest
to deal with asm-specifics. To complete the bpf support in perf, the
the regs_query_register_offset function is added for s390 to support
BPF prologue creation.

Hendrik Brueckner (5):
bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type
s390/bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program
type
arm64/bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program
type
selftests/bpf: sync kernel headers and introduce arch support in
Makefile
perf s390: add regs_query_register_offset()

arch/arm64/include/asm/perf_event.h | 2 +
arch/arm64/include/uapi/asm/bpf_perf_event.h | 9 +
arch/s390/include/asm/perf_event.h | 1 +
arch/s390/include/asm/ptrace.h | 11 +-
arch/s390/include/uapi/asm/bpf_perf_event.h | 9 +
arch/s390/include/uapi/asm/ptrace.h | 11 +
include/linux/perf_event.h | 6 +-
include/uapi/asm-generic/bpf_perf_event.h | 9 +
include/uapi/linux/bpf_perf_event.h | 5 +-
kernel/events/core.c | 2 +-
tools/arch/arm64/include/uapi/asm/bpf_perf_event.h | 9 +
tools/arch/s390/include/uapi/asm/bpf_perf_event.h | 9 +
tools/arch/s390/include/uapi/asm/ptrace.h | 471 +++++++++++++++++++++
tools/include/uapi/asm-generic/bpf_perf_event.h | 9 +
tools/include/uapi/linux/bpf_perf_event.h | 6 +-
tools/perf/arch/s390/Makefile | 1 +
tools/perf/arch/s390/util/dwarf-regs.c | 32 +-
tools/perf/check-headers.sh | 1 +
tools/testing/selftests/bpf/Makefile | 14 +-
19 files changed, 602 insertions(+), 15 deletions(-)
create mode 100644 arch/arm64/include/uapi/asm/bpf_perf_event.h
create mode 100644 arch/s390/include/uapi/asm/bpf_perf_event.h
create mode 100644 include/uapi/asm-generic/bpf_perf_event.h
create mode 100644 tools/arch/arm64/include/uapi/asm/bpf_perf_event.h
create mode 100644 tools/arch/s390/include/uapi/asm/bpf_perf_event.h
create mode 100644 tools/arch/s390/include/uapi/asm/ptrace.h
create mode 100644 tools/include/uapi/asm-generic/bpf_perf_event.h

--
1.8.3.1


2017-12-01 14:19:45

by Hendrik Brueckner

[permalink] [raw]
Subject: [PATCH 3/5] arm64/bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type

Correct the broken uapi for the BPF_PROG_TYPE_PERF_EVENT program type
by exporting the user_pt_regs structure instead of the pt_regs structure
that is in-kernel only.

Signed-off-by: Hendrik Brueckner <[email protected]>
Reviewed-by: Thomas Richter <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---
arch/arm64/include/asm/perf_event.h | 2 ++
arch/arm64/include/uapi/asm/bpf_perf_event.h | 9 +++++++++
2 files changed, 11 insertions(+)
create mode 100644 arch/arm64/include/uapi/asm/bpf_perf_event.h

diff --git a/arch/arm64/include/asm/perf_event.h b/arch/arm64/include/asm/perf_event.h
index 8d5cbec..f9ccc36 100644
--- a/arch/arm64/include/asm/perf_event.h
+++ b/arch/arm64/include/asm/perf_event.h
@@ -18,6 +18,7 @@
#define __ASM_PERF_EVENT_H

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

#define ARMV8_PMU_MAX_COUNTERS 32
#define ARMV8_PMU_COUNTER_MASK (ARMV8_PMU_MAX_COUNTERS - 1)
@@ -79,6 +80,7 @@
extern unsigned long perf_instruction_pointer(struct pt_regs *regs);
extern unsigned long perf_misc_flags(struct pt_regs *regs);
#define perf_misc_flags(regs) perf_misc_flags(regs)
+#define perf_arch_bpf_user_pt_regs(regs) &regs->user_regs
#endif

#define perf_arch_fetch_caller_regs(regs, __ip) { \
diff --git a/arch/arm64/include/uapi/asm/bpf_perf_event.h b/arch/arm64/include/uapi/asm/bpf_perf_event.h
new file mode 100644
index 0000000..b551b74
--- /dev/null
+++ b/arch/arm64/include/uapi/asm/bpf_perf_event.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _UAPI__ASM_BPF_PERF_EVENT_H__
+#define _UAPI__ASM_BPF_PERF_EVENT_H__
+
+#include <asm/ptrace.h>
+
+typedef struct user_pt_regs bpf_user_pt_regs_t;
+
+#endif /* _UAPI__ASM_BPF_PERF_EVENT_H__ */
--
1.8.3.1

2017-12-01 14:19:40

by Hendrik Brueckner

[permalink] [raw]
Subject: [PATCH 1/5] bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type

Commit 0515e5999a466dfe ("bpf: introduce BPF_PROG_TYPE_PERF_EVENT
program type") introduced the bpf_perf_event_data structure which
exports the pt_regs structure. This is OK for multiple architectures
but fail for s390 and arm64 which do not export pt_regs. Programs
using them, for example, the bpf selftest fail to compile on these
architectures.

For s390, exporting the pt_regs is not an option because s390 wants
to allow changes to it. For arm64, there is a user_pt_regs structure
that covers parts of the pt_regs structure for use by user space.

To solve the broken uapi for s390 and arm64, introduce an abstract
type for pt_regs and add an asm/bpf_perf_event.h file that concretes
the type. An asm-generic header file covers the architectures that
export pt_regs today.

The arch-specific enablement for s390 and arm64 follows in separate
commits.

Reported-by: Thomas Richter <[email protected]>
Fixes: 0515e5999a466dfe ("bpf: introduce BPF_PROG_TYPE_PERF_EVENT program type")
Signed-off-by: Hendrik Brueckner <[email protected]>
Reviewed-and-tested-by: Thomas Richter <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
Cc: Daniel Borkmann <[email protected]>
---
include/linux/perf_event.h | 6 +++++-
include/uapi/asm-generic/bpf_perf_event.h | 9 +++++++++
include/uapi/linux/bpf_perf_event.h | 5 ++---
kernel/events/core.c | 2 +-
4 files changed, 17 insertions(+), 5 deletions(-)
create mode 100644 include/uapi/asm-generic/bpf_perf_event.h

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 2c9c87d..7546822 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -15,6 +15,7 @@
#define _LINUX_PERF_EVENT_H

#include <uapi/linux/perf_event.h>
+#include <uapi/linux/bpf_perf_event.h>

/*
* Kernel-internal data types and definitions:
@@ -787,7 +788,7 @@ struct perf_output_handle {
};

struct bpf_perf_event_data_kern {
- struct pt_regs *regs;
+ bpf_user_pt_regs_t *regs;
struct perf_sample_data *data;
struct perf_event *event;
};
@@ -1177,6 +1178,9 @@ extern void perf_tp_event(u16 event_type, u64 count, void *record,
(user_mode(regs) ? PERF_RECORD_MISC_USER : PERF_RECORD_MISC_KERNEL)
# define perf_instruction_pointer(regs) instruction_pointer(regs)
#endif
+#ifndef perf_arch_bpf_user_pt_regs
+# define perf_arch_bpf_user_pt_regs(regs) regs
+#endif

static inline bool has_branch_stack(struct perf_event *event)
{
diff --git a/include/uapi/asm-generic/bpf_perf_event.h b/include/uapi/asm-generic/bpf_perf_event.h
new file mode 100644
index 0000000..53815d2
--- /dev/null
+++ b/include/uapi/asm-generic/bpf_perf_event.h
@@ -0,0 +1,9 @@
+#ifndef _UAPI__ASM_GENERIC_BPF_PERF_EVENT_H__
+#define _UAPI__ASM_GENERIC_BPF_PERF_EVENT_H__
+
+#include <linux/ptrace.h>
+
+/* Export kernel pt_regs structure */
+typedef struct pt_regs bpf_user_pt_regs_t;
+
+#endif /* _UAPI__ASM_GENERIC_BPF_PERF_EVENT_H__ */
diff --git a/include/uapi/linux/bpf_perf_event.h b/include/uapi/linux/bpf_perf_event.h
index af549d4..8f95303 100644
--- a/include/uapi/linux/bpf_perf_event.h
+++ b/include/uapi/linux/bpf_perf_event.h
@@ -8,11 +8,10 @@
#ifndef _UAPI__LINUX_BPF_PERF_EVENT_H__
#define _UAPI__LINUX_BPF_PERF_EVENT_H__

-#include <linux/types.h>
-#include <linux/ptrace.h>
+#include <asm/bpf_perf_event.h>

struct bpf_perf_event_data {
- struct pt_regs regs;
+ bpf_user_pt_regs_t regs;
__u64 sample_period;
};

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 16beab4..ba957b9 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7987,11 +7987,11 @@ static void bpf_overflow_handler(struct perf_event *event,
{
struct bpf_perf_event_data_kern ctx = {
.data = data,
- .regs = regs,
.event = event,
};
int ret = 0;

+ ctx.regs = perf_arch_bpf_user_pt_regs(regs);
preempt_disable();
if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1))
goto out;
--
1.8.3.1

2017-12-01 14:20:22

by Hendrik Brueckner

[permalink] [raw]
Subject: [PATCH 5/5] perf s390: add regs_query_register_offset()

The regs_query_register_offset() helper function converts
register name like "%r0" to an offset of a register in user_pt_regs
It is required by the BPF prologue generator.

The user_pt_regs structure was recently added to "asm/ptrace.h".
Hence, update tools/perf/check-headers.sh to keep the header file
in sync with kernel changes.

Suggested-by: Thomas Richter <[email protected]>
Signed-off-by: Hendrik Brueckner <[email protected]>
Reviewed-and-tested-by: Thomas Richter <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Heiko Carstens <[email protected]>
---
tools/perf/arch/s390/Makefile | 1 +
tools/perf/arch/s390/util/dwarf-regs.c | 32 +++++++++++++++++++++++++++++---
tools/perf/check-headers.sh | 1 +
3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/s390/Makefile b/tools/perf/arch/s390/Makefile
index 21322e0..09ba923 100644
--- a/tools/perf/arch/s390/Makefile
+++ b/tools/perf/arch/s390/Makefile
@@ -2,3 +2,4 @@ ifndef NO_DWARF
PERF_HAVE_DWARF_REGS := 1
endif
HAVE_KVM_STAT_SUPPORT := 1
+PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
diff --git a/tools/perf/arch/s390/util/dwarf-regs.c b/tools/perf/arch/s390/util/dwarf-regs.c
index f47576c..a8ace5c 100644
--- a/tools/perf/arch/s390/util/dwarf-regs.c
+++ b/tools/perf/arch/s390/util/dwarf-regs.c
@@ -2,17 +2,43 @@
/*
* Mapping of DWARF debug register numbers into register names.
*
- * Copyright IBM Corp. 2010
- * Author(s): Heiko Carstens <[email protected]>,
+ * Copyright IBM Corp. 2010, 2017
+ * Author(s): Heiko Carstens <[email protected]>,
+ * Hendrik Brueckner <[email protected]>
*
*/

+#include <errno.h>
#include <stddef.h>
-#include <dwarf-regs.h>
+#include <stdlib.h>
#include <linux/kernel.h>
+#include <asm/ptrace.h>
+#include <string.h>
+#include <dwarf-regs.h>
#include "dwarf-regs-table.h"

const char *get_arch_regstr(unsigned int n)
{
return (n >= ARRAY_SIZE(s390_dwarf_regs)) ? NULL : s390_dwarf_regs[n];
}
+
+/*
+ * Convert the register name into an offset to struct pt_regs (kernel).
+ * This is required by the BPF prologue generator. The BPF
+ * program is called in the BPF overflow handler in the perf
+ * core.
+ */
+int regs_query_register_offset(const char *name)
+{
+ unsigned long gpr;
+
+ if (!name || strncmp(name, "%r", 2))
+ return -EINVAL;
+
+ errno = 0;
+ gpr = strtoul(name + 2, NULL, 10);
+ if (errno || gpr >= 16)
+ return -EINVAL;
+
+ return offsetof(user_pt_regs, gprs) + 8 * gpr;
+}
diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index 77406d2..6db9d80 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -30,6 +30,7 @@ arch/x86/include/uapi/asm/vmx.h
arch/powerpc/include/uapi/asm/kvm.h
arch/s390/include/uapi/asm/kvm.h
arch/s390/include/uapi/asm/kvm_perf.h
+arch/s390/include/uapi/asm/ptrace.h
arch/s390/include/uapi/asm/sie.h
arch/arm/include/uapi/asm/kvm.h
arch/arm64/include/uapi/asm/kvm.h
--
1.8.3.1

2017-12-01 14:20:20

by Hendrik Brueckner

[permalink] [raw]
Subject: [PATCH 4/5] selftests/bpf: sync kernel headers and introduce arch support in Makefile

Synchronize the uapi kernel header files which solves the broken
uapi export of pt_regs. Because of arch-specific uapi headers,
extended the include path in the Makefile.

With this change, the test_verifier program compiles and runs successfully
on s390.

Signed-off-by: Hendrik Brueckner <[email protected]>
Reviewed-and-tested-by: Thomas Richter <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
Cc: Daniel Borkmann <[email protected]>
Cc: Shuah Khan <[email protected]>
---
tools/arch/arm64/include/uapi/asm/bpf_perf_event.h | 9 +
tools/arch/s390/include/uapi/asm/bpf_perf_event.h | 9 +
tools/arch/s390/include/uapi/asm/ptrace.h | 471 +++++++++++++++++++++
tools/include/uapi/asm-generic/bpf_perf_event.h | 9 +
tools/include/uapi/linux/bpf_perf_event.h | 6 +-
tools/testing/selftests/bpf/Makefile | 14 +-
6 files changed, 514 insertions(+), 4 deletions(-)
create mode 100644 tools/arch/arm64/include/uapi/asm/bpf_perf_event.h
create mode 100644 tools/arch/s390/include/uapi/asm/bpf_perf_event.h
create mode 100644 tools/arch/s390/include/uapi/asm/ptrace.h
create mode 100644 tools/include/uapi/asm-generic/bpf_perf_event.h

diff --git a/tools/arch/arm64/include/uapi/asm/bpf_perf_event.h b/tools/arch/arm64/include/uapi/asm/bpf_perf_event.h
new file mode 100644
index 0000000..b551b74
--- /dev/null
+++ b/tools/arch/arm64/include/uapi/asm/bpf_perf_event.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _UAPI__ASM_BPF_PERF_EVENT_H__
+#define _UAPI__ASM_BPF_PERF_EVENT_H__
+
+#include <asm/ptrace.h>
+
+typedef struct user_pt_regs bpf_user_pt_regs_t;
+
+#endif /* _UAPI__ASM_BPF_PERF_EVENT_H__ */
diff --git a/tools/arch/s390/include/uapi/asm/bpf_perf_event.h b/tools/arch/s390/include/uapi/asm/bpf_perf_event.h
new file mode 100644
index 0000000..cefe7c7
--- /dev/null
+++ b/tools/arch/s390/include/uapi/asm/bpf_perf_event.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _UAPI__ASM_BPF_PERF_EVENT_H__
+#define _UAPI__ASM_BPF_PERF_EVENT_H__
+
+#include <asm/ptrace.h>
+
+typedef user_pt_regs bpf_user_pt_regs_t;
+
+#endif /* _UAPI__ASM_BPF_PERF_EVENT_H__ */
diff --git a/tools/arch/s390/include/uapi/asm/ptrace.h b/tools/arch/s390/include/uapi/asm/ptrace.h
new file mode 100644
index 0000000..70f7cb2
--- /dev/null
+++ b/tools/arch/s390/include/uapi/asm/ptrace.h
@@ -0,0 +1,471 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * S390 version
+ * Copyright IBM Corp. 1999, 2000
+ * Author(s): Denis Joseph Barrow ([email protected],[email protected])
+ */
+
+#ifndef _UAPI_S390_PTRACE_H
+#define _UAPI_S390_PTRACE_H
+
+/*
+ * Offsets in the user_regs_struct. They are used for the ptrace
+ * system call and in entry.S
+ */
+#ifndef __s390x__
+
+#define PT_PSWMASK 0x00
+#define PT_PSWADDR 0x04
+#define PT_GPR0 0x08
+#define PT_GPR1 0x0C
+#define PT_GPR2 0x10
+#define PT_GPR3 0x14
+#define PT_GPR4 0x18
+#define PT_GPR5 0x1C
+#define PT_GPR6 0x20
+#define PT_GPR7 0x24
+#define PT_GPR8 0x28
+#define PT_GPR9 0x2C
+#define PT_GPR10 0x30
+#define PT_GPR11 0x34
+#define PT_GPR12 0x38
+#define PT_GPR13 0x3C
+#define PT_GPR14 0x40
+#define PT_GPR15 0x44
+#define PT_ACR0 0x48
+#define PT_ACR1 0x4C
+#define PT_ACR2 0x50
+#define PT_ACR3 0x54
+#define PT_ACR4 0x58
+#define PT_ACR5 0x5C
+#define PT_ACR6 0x60
+#define PT_ACR7 0x64
+#define PT_ACR8 0x68
+#define PT_ACR9 0x6C
+#define PT_ACR10 0x70
+#define PT_ACR11 0x74
+#define PT_ACR12 0x78
+#define PT_ACR13 0x7C
+#define PT_ACR14 0x80
+#define PT_ACR15 0x84
+#define PT_ORIGGPR2 0x88
+#define PT_FPC 0x90
+/*
+ * A nasty fact of life that the ptrace api
+ * only supports passing of longs.
+ */
+#define PT_FPR0_HI 0x98
+#define PT_FPR0_LO 0x9C
+#define PT_FPR1_HI 0xA0
+#define PT_FPR1_LO 0xA4
+#define PT_FPR2_HI 0xA8
+#define PT_FPR2_LO 0xAC
+#define PT_FPR3_HI 0xB0
+#define PT_FPR3_LO 0xB4
+#define PT_FPR4_HI 0xB8
+#define PT_FPR4_LO 0xBC
+#define PT_FPR5_HI 0xC0
+#define PT_FPR5_LO 0xC4
+#define PT_FPR6_HI 0xC8
+#define PT_FPR6_LO 0xCC
+#define PT_FPR7_HI 0xD0
+#define PT_FPR7_LO 0xD4
+#define PT_FPR8_HI 0xD8
+#define PT_FPR8_LO 0XDC
+#define PT_FPR9_HI 0xE0
+#define PT_FPR9_LO 0xE4
+#define PT_FPR10_HI 0xE8
+#define PT_FPR10_LO 0xEC
+#define PT_FPR11_HI 0xF0
+#define PT_FPR11_LO 0xF4
+#define PT_FPR12_HI 0xF8
+#define PT_FPR12_LO 0xFC
+#define PT_FPR13_HI 0x100
+#define PT_FPR13_LO 0x104
+#define PT_FPR14_HI 0x108
+#define PT_FPR14_LO 0x10C
+#define PT_FPR15_HI 0x110
+#define PT_FPR15_LO 0x114
+#define PT_CR_9 0x118
+#define PT_CR_10 0x11C
+#define PT_CR_11 0x120
+#define PT_IEEE_IP 0x13C
+#define PT_LASTOFF PT_IEEE_IP
+#define PT_ENDREGS 0x140-1
+
+#define GPR_SIZE 4
+#define CR_SIZE 4
+
+#define STACK_FRAME_OVERHEAD 96 /* size of minimum stack frame */
+
+#else /* __s390x__ */
+
+#define PT_PSWMASK 0x00
+#define PT_PSWADDR 0x08
+#define PT_GPR0 0x10
+#define PT_GPR1 0x18
+#define PT_GPR2 0x20
+#define PT_GPR3 0x28
+#define PT_GPR4 0x30
+#define PT_GPR5 0x38
+#define PT_GPR6 0x40
+#define PT_GPR7 0x48
+#define PT_GPR8 0x50
+#define PT_GPR9 0x58
+#define PT_GPR10 0x60
+#define PT_GPR11 0x68
+#define PT_GPR12 0x70
+#define PT_GPR13 0x78
+#define PT_GPR14 0x80
+#define PT_GPR15 0x88
+#define PT_ACR0 0x90
+#define PT_ACR1 0x94
+#define PT_ACR2 0x98
+#define PT_ACR3 0x9C
+#define PT_ACR4 0xA0
+#define PT_ACR5 0xA4
+#define PT_ACR6 0xA8
+#define PT_ACR7 0xAC
+#define PT_ACR8 0xB0
+#define PT_ACR9 0xB4
+#define PT_ACR10 0xB8
+#define PT_ACR11 0xBC
+#define PT_ACR12 0xC0
+#define PT_ACR13 0xC4
+#define PT_ACR14 0xC8
+#define PT_ACR15 0xCC
+#define PT_ORIGGPR2 0xD0
+#define PT_FPC 0xD8
+#define PT_FPR0 0xE0
+#define PT_FPR1 0xE8
+#define PT_FPR2 0xF0
+#define PT_FPR3 0xF8
+#define PT_FPR4 0x100
+#define PT_FPR5 0x108
+#define PT_FPR6 0x110
+#define PT_FPR7 0x118
+#define PT_FPR8 0x120
+#define PT_FPR9 0x128
+#define PT_FPR10 0x130
+#define PT_FPR11 0x138
+#define PT_FPR12 0x140
+#define PT_FPR13 0x148
+#define PT_FPR14 0x150
+#define PT_FPR15 0x158
+#define PT_CR_9 0x160
+#define PT_CR_10 0x168
+#define PT_CR_11 0x170
+#define PT_IEEE_IP 0x1A8
+#define PT_LASTOFF PT_IEEE_IP
+#define PT_ENDREGS 0x1B0-1
+
+#define GPR_SIZE 8
+#define CR_SIZE 8
+
+#define STACK_FRAME_OVERHEAD 160 /* size of minimum stack frame */
+
+#endif /* __s390x__ */
+
+#define NUM_GPRS 16
+#define NUM_FPRS 16
+#define NUM_CRS 16
+#define NUM_ACRS 16
+
+#define NUM_CR_WORDS 3
+
+#define FPR_SIZE 8
+#define FPC_SIZE 4
+#define FPC_PAD_SIZE 4 /* gcc insists on aligning the fpregs */
+#define ACR_SIZE 4
+
+
+#define PTRACE_OLDSETOPTIONS 21
+
+#ifndef __ASSEMBLY__
+#include <linux/stddef.h>
+#include <linux/types.h>
+
+typedef union
+{
+ float f;
+ double d;
+ __u64 ui;
+ struct
+ {
+ __u32 hi;
+ __u32 lo;
+ } fp;
+} freg_t;
+
+typedef struct
+{
+ __u32 fpc;
+ __u32 pad;
+ freg_t fprs[NUM_FPRS];
+} s390_fp_regs;
+
+#define FPC_EXCEPTION_MASK 0xF8000000
+#define FPC_FLAGS_MASK 0x00F80000
+#define FPC_DXC_MASK 0x0000FF00
+#define FPC_RM_MASK 0x00000003
+
+/* this typedef defines how a Program Status Word looks like */
+typedef struct
+{
+ unsigned long mask;
+ unsigned long addr;
+} __attribute__ ((aligned(8))) psw_t;
+
+#ifndef __s390x__
+
+#define PSW_MASK_PER 0x40000000UL
+#define PSW_MASK_DAT 0x04000000UL
+#define PSW_MASK_IO 0x02000000UL
+#define PSW_MASK_EXT 0x01000000UL
+#define PSW_MASK_KEY 0x00F00000UL
+#define PSW_MASK_BASE 0x00080000UL /* always one */
+#define PSW_MASK_MCHECK 0x00040000UL
+#define PSW_MASK_WAIT 0x00020000UL
+#define PSW_MASK_PSTATE 0x00010000UL
+#define PSW_MASK_ASC 0x0000C000UL
+#define PSW_MASK_CC 0x00003000UL
+#define PSW_MASK_PM 0x00000F00UL
+#define PSW_MASK_RI 0x00000000UL
+#define PSW_MASK_EA 0x00000000UL
+#define PSW_MASK_BA 0x00000000UL
+
+#define PSW_MASK_USER 0x0000FF00UL
+
+#define PSW_ADDR_AMODE 0x80000000UL
+#define PSW_ADDR_INSN 0x7FFFFFFFUL
+
+#define PSW_DEFAULT_KEY (((unsigned long) PAGE_DEFAULT_ACC) << 20)
+
+#define PSW_ASC_PRIMARY 0x00000000UL
+#define PSW_ASC_ACCREG 0x00004000UL
+#define PSW_ASC_SECONDARY 0x00008000UL
+#define PSW_ASC_HOME 0x0000C000UL
+
+#else /* __s390x__ */
+
+#define PSW_MASK_PER 0x4000000000000000UL
+#define PSW_MASK_DAT 0x0400000000000000UL
+#define PSW_MASK_IO 0x0200000000000000UL
+#define PSW_MASK_EXT 0x0100000000000000UL
+#define PSW_MASK_BASE 0x0000000000000000UL
+#define PSW_MASK_KEY 0x00F0000000000000UL
+#define PSW_MASK_MCHECK 0x0004000000000000UL
+#define PSW_MASK_WAIT 0x0002000000000000UL
+#define PSW_MASK_PSTATE 0x0001000000000000UL
+#define PSW_MASK_ASC 0x0000C00000000000UL
+#define PSW_MASK_CC 0x0000300000000000UL
+#define PSW_MASK_PM 0x00000F0000000000UL
+#define PSW_MASK_RI 0x0000008000000000UL
+#define PSW_MASK_EA 0x0000000100000000UL
+#define PSW_MASK_BA 0x0000000080000000UL
+
+#define PSW_MASK_USER 0x0000FF0180000000UL
+
+#define PSW_ADDR_AMODE 0x0000000000000000UL
+#define PSW_ADDR_INSN 0xFFFFFFFFFFFFFFFFUL
+
+#define PSW_DEFAULT_KEY (((unsigned long) PAGE_DEFAULT_ACC) << 52)
+
+#define PSW_ASC_PRIMARY 0x0000000000000000UL
+#define PSW_ASC_ACCREG 0x0000400000000000UL
+#define PSW_ASC_SECONDARY 0x0000800000000000UL
+#define PSW_ASC_HOME 0x0000C00000000000UL
+
+#endif /* __s390x__ */
+
+
+/*
+ * The s390_regs structure is used to define the elf_gregset_t.
+ */
+typedef struct
+{
+ psw_t psw;
+ unsigned long gprs[NUM_GPRS];
+ unsigned int acrs[NUM_ACRS];
+ unsigned long orig_gpr2;
+} s390_regs;
+
+/*
+ * The user_pt_regs structure exports the beginning of
+ * the in-kernel pt_regs structure to user space.
+ */
+typedef struct
+{
+ unsigned long args[1];
+ psw_t psw;
+ unsigned long gprs[NUM_GPRS];
+} user_pt_regs;
+
+/*
+ * Now for the user space program event recording (trace) definitions.
+ * The following structures are used only for the ptrace interface, don't
+ * touch or even look at it if you don't want to modify the user-space
+ * ptrace interface. In particular stay away from it for in-kernel PER.
+ */
+typedef struct
+{
+ unsigned long cr[NUM_CR_WORDS];
+} per_cr_words;
+
+#define PER_EM_MASK 0xE8000000UL
+
+typedef struct
+{
+#ifdef __s390x__
+ unsigned : 32;
+#endif /* __s390x__ */
+ unsigned em_branching : 1;
+ unsigned em_instruction_fetch : 1;
+ /*
+ * Switching on storage alteration automatically fixes
+ * the storage alteration event bit in the users std.
+ */
+ unsigned em_storage_alteration : 1;
+ unsigned em_gpr_alt_unused : 1;
+ unsigned em_store_real_address : 1;
+ unsigned : 3;
+ unsigned branch_addr_ctl : 1;
+ unsigned : 1;
+ unsigned storage_alt_space_ctl : 1;
+ unsigned : 21;
+ unsigned long starting_addr;
+ unsigned long ending_addr;
+} per_cr_bits;
+
+typedef struct
+{
+ unsigned short perc_atmid;
+ unsigned long address;
+ unsigned char access_id;
+} per_lowcore_words;
+
+typedef struct
+{
+ unsigned perc_branching : 1;
+ unsigned perc_instruction_fetch : 1;
+ unsigned perc_storage_alteration : 1;
+ unsigned perc_gpr_alt_unused : 1;
+ unsigned perc_store_real_address : 1;
+ unsigned : 3;
+ unsigned atmid_psw_bit_31 : 1;
+ unsigned atmid_validity_bit : 1;
+ unsigned atmid_psw_bit_32 : 1;
+ unsigned atmid_psw_bit_5 : 1;
+ unsigned atmid_psw_bit_16 : 1;
+ unsigned atmid_psw_bit_17 : 1;
+ unsigned si : 2;
+ unsigned long address;
+ unsigned : 4;
+ unsigned access_id : 4;
+} per_lowcore_bits;
+
+typedef struct
+{
+ union {
+ per_cr_words words;
+ per_cr_bits bits;
+ } control_regs;
+ /*
+ * The single_step and instruction_fetch bits are obsolete,
+ * the kernel always sets them to zero. To enable single
+ * stepping use ptrace(PTRACE_SINGLESTEP) instead.
+ */
+ unsigned single_step : 1;
+ unsigned instruction_fetch : 1;
+ unsigned : 30;
+ /*
+ * These addresses are copied into cr10 & cr11 if single
+ * stepping is switched off
+ */
+ unsigned long starting_addr;
+ unsigned long ending_addr;
+ union {
+ per_lowcore_words words;
+ per_lowcore_bits bits;
+ } lowcore;
+} per_struct;
+
+typedef struct
+{
+ unsigned int len;
+ unsigned long kernel_addr;
+ unsigned long process_addr;
+} ptrace_area;
+
+/*
+ * S/390 specific non posix ptrace requests. I chose unusual values so
+ * they are unlikely to clash with future ptrace definitions.
+ */
+#define PTRACE_PEEKUSR_AREA 0x5000
+#define PTRACE_POKEUSR_AREA 0x5001
+#define PTRACE_PEEKTEXT_AREA 0x5002
+#define PTRACE_PEEKDATA_AREA 0x5003
+#define PTRACE_POKETEXT_AREA 0x5004
+#define PTRACE_POKEDATA_AREA 0x5005
+#define PTRACE_GET_LAST_BREAK 0x5006
+#define PTRACE_PEEK_SYSTEM_CALL 0x5007
+#define PTRACE_POKE_SYSTEM_CALL 0x5008
+#define PTRACE_ENABLE_TE 0x5009
+#define PTRACE_DISABLE_TE 0x5010
+#define PTRACE_TE_ABORT_RAND 0x5011
+
+/*
+ * The numbers chosen here are somewhat arbitrary but absolutely MUST
+ * not overlap with any of the number assigned in <linux/ptrace.h>.
+ */
+#define PTRACE_SINGLEBLOCK 12 /* resume execution until next branch */
+
+/*
+ * PT_PROT definition is loosely based on hppa bsd definition in
+ * gdb/hppab-nat.c
+ */
+#define PTRACE_PROT 21
+
+typedef enum
+{
+ ptprot_set_access_watchpoint,
+ ptprot_set_write_watchpoint,
+ ptprot_disable_watchpoint
+} ptprot_flags;
+
+typedef struct
+{
+ unsigned long lowaddr;
+ unsigned long hiaddr;
+ ptprot_flags prot;
+} ptprot_area;
+
+/* Sequence of bytes for breakpoint illegal instruction. */
+#define S390_BREAKPOINT {0x0,0x1}
+#define S390_BREAKPOINT_U16 ((__u16)0x0001)
+#define S390_SYSCALL_OPCODE ((__u16)0x0a00)
+#define S390_SYSCALL_SIZE 2
+
+/*
+ * The user_regs_struct defines the way the user registers are
+ * store on the stack for signal handling.
+ */
+struct user_regs_struct
+{
+ psw_t psw;
+ unsigned long gprs[NUM_GPRS];
+ unsigned int acrs[NUM_ACRS];
+ unsigned long orig_gpr2;
+ s390_fp_regs fp_regs;
+ /*
+ * These per registers are in here so that gdb can modify them
+ * itself as there is no "official" ptrace interface for hardware
+ * watchpoints. This is the way intel does it.
+ */
+ per_struct per_info;
+ unsigned long ieee_instruction_pointer; /* obsolete, always 0 */
+};
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _UAPI_S390_PTRACE_H */
diff --git a/tools/include/uapi/asm-generic/bpf_perf_event.h b/tools/include/uapi/asm-generic/bpf_perf_event.h
new file mode 100644
index 0000000..53815d2
--- /dev/null
+++ b/tools/include/uapi/asm-generic/bpf_perf_event.h
@@ -0,0 +1,9 @@
+#ifndef _UAPI__ASM_GENERIC_BPF_PERF_EVENT_H__
+#define _UAPI__ASM_GENERIC_BPF_PERF_EVENT_H__
+
+#include <linux/ptrace.h>
+
+/* Export kernel pt_regs structure */
+typedef struct pt_regs bpf_user_pt_regs_t;
+
+#endif /* _UAPI__ASM_GENERIC_BPF_PERF_EVENT_H__ */
diff --git a/tools/include/uapi/linux/bpf_perf_event.h b/tools/include/uapi/linux/bpf_perf_event.h
index 0674272..8f95303 100644
--- a/tools/include/uapi/linux/bpf_perf_event.h
+++ b/tools/include/uapi/linux/bpf_perf_event.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
/* Copyright (c) 2016 Facebook
*
* This program is free software; you can redistribute it and/or
@@ -7,11 +8,10 @@
#ifndef _UAPI__LINUX_BPF_PERF_EVENT_H__
#define _UAPI__LINUX_BPF_PERF_EVENT_H__

-#include <linux/types.h>
-#include <linux/ptrace.h>
+#include <asm/bpf_perf_event.h>

struct bpf_perf_event_data {
- struct pt_regs regs;
+ bpf_user_pt_regs_t regs;
__u64 sample_period;
};

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 333a486..21a2d76 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,7 +1,19 @@
# SPDX-License-Identifier: GPL-2.0
+
+ifeq ($(srctree),)
+srctree := $(patsubst %/,%,$(dir $(CURDIR)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+endif
+include $(srctree)/tools/scripts/Makefile.arch
+
+$(call detected_var,SRCARCH)
+
LIBDIR := ../../../lib
BPFDIR := $(LIBDIR)/bpf
APIDIR := ../../../include/uapi
+ASMDIR:= ../../../arch/$(ARCH)/include/uapi
GENDIR := ../../../../include/generated
GENHDR := $(GENDIR)/autoconf.h

@@ -9,7 +21,7 @@ ifneq ($(wildcard $(GENHDR)),)
GENFLAGS := -DHAVE_GENHDR
endif

-CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
+CFLAGS += -Wall -O2 -I$(APIDIR) -I$(ASMDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
LDLIBS += -lcap -lelf

TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
--
1.8.3.1

2017-12-01 14:22:27

by Hendrik Brueckner

[permalink] [raw]
Subject: [PATCH 2/5] s390/bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type

To mitigate and correct the broken uapi for the BPF_PROG_TYPE_PERF_EVENT
program type, introduce a user_pt_regs structure (similar to arm64) that
exports parts from the beginnig of the pt_regs structure.

The export must start with the beginning of the pt_regs structure because
to correctly calculate BPF prologues for perf (regs_query_register_offset()).

For BPF_PROG_TYPE_PERF_EVENT program types, the BPF program is then passed
a user_pt_regs structure.

Note: Depending on future changes to the s390 pt_regs structure, consider
the user_pt_regs structure to be stable for a particular kernel version
only. (Of course, s390 tries to ensure keep it stable as much as possible.)

Signed-off-by: Hendrik Brueckner <[email protected]>
Reviewed-and-tested-by: Thomas Richter <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
Cc: Heiko Carstens <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
Cc: Daniel Borkmann <[email protected]>
---
arch/s390/include/asm/perf_event.h | 1 +
arch/s390/include/asm/ptrace.h | 11 ++++++++---
arch/s390/include/uapi/asm/bpf_perf_event.h | 9 +++++++++
arch/s390/include/uapi/asm/ptrace.h | 11 +++++++++++
4 files changed, 29 insertions(+), 3 deletions(-)
create mode 100644 arch/s390/include/uapi/asm/bpf_perf_event.h

diff --git a/arch/s390/include/asm/perf_event.h b/arch/s390/include/asm/perf_event.h
index d6c9d1e..b9c0e36 100644
--- a/arch/s390/include/asm/perf_event.h
+++ b/arch/s390/include/asm/perf_event.h
@@ -40,6 +40,7 @@ extern ssize_t cpumf_events_sysfs_show(struct device *dev,
extern unsigned long perf_instruction_pointer(struct pt_regs *regs);
extern unsigned long perf_misc_flags(struct pt_regs *regs);
#define perf_misc_flags(regs) perf_misc_flags(regs)
+#define perf_arch_bpf_user_pt_regs(regs) &regs->user_regs

/* Perf pt_regs extension for sample-data-entry indicators */
struct perf_sf_sde_regs {
diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
index a3788da..6f70d81 100644
--- a/arch/s390/include/asm/ptrace.h
+++ b/arch/s390/include/asm/ptrace.h
@@ -74,9 +74,14 @@ enum {
*/
struct pt_regs
{
- unsigned long args[1];
- psw_t psw;
- unsigned long gprs[NUM_GPRS];
+ union {
+ user_pt_regs user_regs;
+ struct {
+ unsigned long args[1];
+ psw_t psw;
+ unsigned long gprs[NUM_GPRS];
+ };
+ };
unsigned long orig_gpr2;
unsigned int int_code;
unsigned int int_parm;
diff --git a/arch/s390/include/uapi/asm/bpf_perf_event.h b/arch/s390/include/uapi/asm/bpf_perf_event.h
new file mode 100644
index 0000000..cefe7c7
--- /dev/null
+++ b/arch/s390/include/uapi/asm/bpf_perf_event.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _UAPI__ASM_BPF_PERF_EVENT_H__
+#define _UAPI__ASM_BPF_PERF_EVENT_H__
+
+#include <asm/ptrace.h>
+
+typedef user_pt_regs bpf_user_pt_regs_t;
+
+#endif /* _UAPI__ASM_BPF_PERF_EVENT_H__ */
diff --git a/arch/s390/include/uapi/asm/ptrace.h b/arch/s390/include/uapi/asm/ptrace.h
index 0d23c8f..70f7cb2 100644
--- a/arch/s390/include/uapi/asm/ptrace.h
+++ b/arch/s390/include/uapi/asm/ptrace.h
@@ -291,6 +291,17 @@
} s390_regs;

/*
+ * The user_pt_regs structure exports the beginning of
+ * the in-kernel pt_regs structure to user space.
+ */
+typedef struct
+{
+ unsigned long args[1];
+ psw_t psw;
+ unsigned long gprs[NUM_GPRS];
+} user_pt_regs;
+
+/*
* Now for the user space program event recording (trace) definitions.
* The following structures are used only for the ptrace interface, don't
* touch or even look at it if you don't want to modify the user-space
--
1.8.3.1

2017-12-03 17:51:17

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: [PATCH 1/5] bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type

On Fri, Dec 01, 2017 at 03:19:04PM +0100, Hendrik Brueckner wrote:
> Commit 0515e5999a466dfe ("bpf: introduce BPF_PROG_TYPE_PERF_EVENT
> program type") introduced the bpf_perf_event_data structure which
> exports the pt_regs structure. This is OK for multiple architectures
> but fail for s390 and arm64 which do not export pt_regs. Programs
> using them, for example, the bpf selftest fail to compile on these
> architectures.
>
> For s390, exporting the pt_regs is not an option because s390 wants
> to allow changes to it. For arm64, there is a user_pt_regs structure
> that covers parts of the pt_regs structure for use by user space.
>
> To solve the broken uapi for s390 and arm64, introduce an abstract
> type for pt_regs and add an asm/bpf_perf_event.h file that concretes
> the type. An asm-generic header file covers the architectures that
> export pt_regs today.
>
> The arch-specific enablement for s390 and arm64 follows in separate
> commits.
>
> Reported-by: Thomas Richter <[email protected]>
> Fixes: 0515e5999a466dfe ("bpf: introduce BPF_PROG_TYPE_PERF_EVENT program type")
> Signed-off-by: Hendrik Brueckner <[email protected]>
> Reviewed-and-tested-by: Thomas Richter <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Alexander Shishkin <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Alexei Starovoitov <[email protected]>
> Cc: Daniel Borkmann <[email protected]>
> ---
> include/linux/perf_event.h | 6 +++++-
> include/uapi/asm-generic/bpf_perf_event.h | 9 +++++++++
> include/uapi/linux/bpf_perf_event.h | 5 ++---
> kernel/events/core.c | 2 +-
> 4 files changed, 17 insertions(+), 5 deletions(-)
> create mode 100644 include/uapi/asm-generic/bpf_perf_event.h
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 2c9c87d..7546822 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -15,6 +15,7 @@
> #define _LINUX_PERF_EVENT_H
>
> #include <uapi/linux/perf_event.h>
> +#include <uapi/linux/bpf_perf_event.h>
>
> /*
> * Kernel-internal data types and definitions:
> @@ -787,7 +788,7 @@ struct perf_output_handle {
> };
>
> struct bpf_perf_event_data_kern {
> - struct pt_regs *regs;
> + bpf_user_pt_regs_t *regs;
> struct perf_sample_data *data;
> struct perf_event *event;
> };
> @@ -1177,6 +1178,9 @@ extern void perf_tp_event(u16 event_type, u64 count, void *record,
> (user_mode(regs) ? PERF_RECORD_MISC_USER : PERF_RECORD_MISC_KERNEL)
> # define perf_instruction_pointer(regs) instruction_pointer(regs)
> #endif
> +#ifndef perf_arch_bpf_user_pt_regs
> +# define perf_arch_bpf_user_pt_regs(regs) regs
> +#endif
>
> static inline bool has_branch_stack(struct perf_event *event)
> {
> diff --git a/include/uapi/asm-generic/bpf_perf_event.h b/include/uapi/asm-generic/bpf_perf_event.h
> new file mode 100644
> index 0000000..53815d2
> --- /dev/null
> +++ b/include/uapi/asm-generic/bpf_perf_event.h
> @@ -0,0 +1,9 @@
> +#ifndef _UAPI__ASM_GENERIC_BPF_PERF_EVENT_H__
> +#define _UAPI__ASM_GENERIC_BPF_PERF_EVENT_H__
> +
> +#include <linux/ptrace.h>
> +
> +/* Export kernel pt_regs structure */
> +typedef struct pt_regs bpf_user_pt_regs_t;
> +
> +#endif /* _UAPI__ASM_GENERIC_BPF_PERF_EVENT_H__ */
> diff --git a/include/uapi/linux/bpf_perf_event.h b/include/uapi/linux/bpf_perf_event.h
> index af549d4..8f95303 100644
> --- a/include/uapi/linux/bpf_perf_event.h
> +++ b/include/uapi/linux/bpf_perf_event.h
> @@ -8,11 +8,10 @@
> #ifndef _UAPI__LINUX_BPF_PERF_EVENT_H__
> #define _UAPI__LINUX_BPF_PERF_EVENT_H__
>
> -#include <linux/types.h>
> -#include <linux/ptrace.h>
> +#include <asm/bpf_perf_event.h>
>
> struct bpf_perf_event_data {
> - struct pt_regs regs;
> + bpf_user_pt_regs_t regs;
> __u64 sample_period;
> };

Thank you for working on this problem.
The fix looks great to me.
While applying it I noticed few nits:
Applying: selftests/bpf: sync kernel headers and introduce arch support in Makefile
/w/bpf/.git/rebase-apply/patch:253: trailing whitespace.
freg_t fprs[NUM_FPRS];
/w/bpf/.git/rebase-apply/patch:262: trailing whitespace.
typedef struct
/w/bpf/.git/rebase-apply/patch:439: trailing whitespace.
} lowcore;
/w/bpf/.git/rebase-apply/patch:490: trailing whitespace.
} ptprot_area;
warning: 4 lines add whitespace errors.

Could you please fix those and resubmit ?
With that fixed feel free to add my
Acked-by: Alexei Starovoitov <[email protected]>
to the patches.
I've tested it on arm64 and don't see any issues.

When resubmitting could you please reduce cc-list, since this set
went into spam folder for me and I noticed it only in patchworks.
Thanks

2017-12-04 04:01:35

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/5] bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type

Hi Hendrik,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.15-rc2]
[cannot apply to tip/perf/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Hendrik-Brueckner/bpf-correct-broken-uapi-for-BPF_PROG_TYPE_PERF_EVENT-program-type/20171204-092027
config: x86_64-randconfig-g0-12040613 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

All errors (new ones prefixed by >>):

In file included from include/linux/perf_event.h:18:0,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from include/linux/syscalls.h:82,
from net/socket.c:83:
>> include/uapi/linux/bpf_perf_event.h:11:32: fatal error: asm/bpf_perf_event.h: No such file or directory
#include <asm/bpf_perf_event.h>
^
compilation terminated.

vim +11 include/uapi/linux/bpf_perf_event.h

10
> 11 #include <asm/bpf_perf_event.h>
12

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (1.36 kB)
.config.gz (28.71 kB)
Download all attachments

2017-12-04 08:34:50

by Hendrik Brueckner

[permalink] [raw]
Subject: Re: [PATCH 1/5] bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type

Hi Alexei,

On Sun, Dec 03, 2017 at 09:51:10AM -0800, Alexei Starovoitov wrote:
> On Fri, Dec 01, 2017 at 03:19:04PM +0100, Hendrik Brueckner wrote:
> > --- a/include/uapi/linux/bpf_perf_event.h
> > +++ b/include/uapi/linux/bpf_perf_event.h
> > @@ -8,11 +8,10 @@
> > #ifndef _UAPI__LINUX_BPF_PERF_EVENT_H__
> > #define _UAPI__LINUX_BPF_PERF_EVENT_H__
> >
> > -#include <linux/types.h>
> > -#include <linux/ptrace.h>
> > +#include <asm/bpf_perf_event.h>
> >
> > struct bpf_perf_event_data {
> > - struct pt_regs regs;
> > + bpf_user_pt_regs_t regs;
> > __u64 sample_period;
> > };
>
> Thank you for working on this problem.
> The fix looks great to me.

Thank you.

> While applying it I noticed few nits:
> Applying: selftests/bpf: sync kernel headers and introduce arch support in Makefile
> /w/bpf/.git/rebase-apply/patch:253: trailing whitespace.
> freg_t fprs[NUM_FPRS];
> /w/bpf/.git/rebase-apply/patch:262: trailing whitespace.
> typedef struct
> /w/bpf/.git/rebase-apply/patch:439: trailing whitespace.
> } lowcore;
> /w/bpf/.git/rebase-apply/patch:490: trailing whitespace.
> } ptprot_area;
> warning: 4 lines add whitespace errors.
>
> Could you please fix those and resubmit ?

Sure will do. For that I have to add a clean-up patch for the s390 ptrace.h
uapi header.

> With that fixed feel free to add my
> Acked-by: Alexei Starovoitov <[email protected]>
> to the patches.

Thank you very much, will do!

> I've tested it on arm64 and don't see any issues.

I got a nice message from the build bot, I have some more work on that.
So will respin the series, add you Acked-by, and sent a v2 around.

> When resubmitting could you please reduce cc-list, since this set
> went into spam folder for me and I noticed it only in patchworks.

Will do... I just used what get_maintainers.pl told me :)