]> rtime.felk.cvut.cz Git - zynq/linux.git/commitdiff
perf/x86/intel: Fix unwind errors from PEBS entries (mk-II)
authorPeter Zijlstra <peterz@infradead.org>
Thu, 10 May 2018 13:48:41 +0000 (15:48 +0200)
committerIngo Molnar <mingo@kernel.org>
Wed, 25 Jul 2018 09:46:21 +0000 (11:46 +0200)
Vince reported the perf_fuzzer giving various unwinder warnings and
Josh reported:

> Deja vu.  Most of these are related to perf PEBS, similar to the
> following issue:
>
>   b8000586c90b ("perf/x86/intel: Cure bogus unwind from PEBS entries")
>
> This is basically the ORC version of that.  setup_pebs_sample_data() is
> assembling a franken-pt_regs which ORC isn't happy about.  RIP is
> inconsistent with some of the other registers (like RSP and RBP).

And where the previous unwinder only needed BP,SP ORC also requires
IP. But we cannot spoof IP because then the sample will get displaced,
entirely negating the point of PEBS.

So cure the whole thing differently by doing the unwind early; this
does however require a means to communicate we did the unwind early.
We (ab)use an unused sample_type bit for this, which we set on events
that fill out the data->callchain before the normal
perf_prepare_sample().

Debugged-by: Josh Poimboeuf <jpoimboe@redhat.com>
Reported-by: Vince Weaver <vincent.weaver@maine.edu>
Tested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Tested-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
arch/x86/events/intel/core.c
arch/x86/events/intel/ds.c
include/linux/perf_event.h
include/uapi/linux/perf_event.h
kernel/events/core.c

index 707b2a96e516b5579c9321cbf0822a627bc8a8d8..86f0c15dcc2db2fbc63a3ee6d81e349cd5351e48 100644 (file)
@@ -2997,6 +2997,9 @@ static int intel_pmu_hw_config(struct perf_event *event)
                }
                if (x86_pmu.pebs_aliases)
                        x86_pmu.pebs_aliases(event);
+
+               if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN)
+                       event->attr.sample_type |= __PERF_SAMPLE_CALLCHAIN_EARLY;
        }
 
        if (needs_branch_stack(event)) {
index 8cf03f1019380bf0ba0c201067c2c781eb1a543c..8dbba77e05184263daaeca280a1fe79853617ca0 100644 (file)
@@ -1185,17 +1185,21 @@ static void setup_pebs_sample_data(struct perf_event *event,
                data->data_src.val = val;
        }
 
+       /*
+        * We must however always use iregs for the unwinder to stay sane; the
+        * record BP,SP,IP can point into thin air when the record is from a
+        * previous PMI context or an (I)RET happend between the record and
+        * PMI.
+        */
+       if (sample_type & PERF_SAMPLE_CALLCHAIN)
+               data->callchain = perf_callchain(event, iregs);
+
        /*
         * We use the interrupt regs as a base because the PEBS record does not
         * contain a full regs set, specifically it seems to lack segment
         * descriptors, which get used by things like user_mode().
         *
         * In the simple case fix up only the IP for PERF_SAMPLE_IP.
-        *
-        * We must however always use BP,SP from iregs for the unwinder to stay
-        * sane; the record BP,SP can point into thin air when the record is
-        * from a previous PMI context or an (I)RET happend between the record
-        * and PMI.
         */
        *regs = *iregs;
 
@@ -1214,15 +1218,8 @@ static void setup_pebs_sample_data(struct perf_event *event,
                regs->si = pebs->si;
                regs->di = pebs->di;
 
-               /*
-                * Per the above; only set BP,SP if we don't need callchains.
-                *
-                * XXX: does this make sense?
-                */
-               if (!(sample_type & PERF_SAMPLE_CALLCHAIN)) {
-                       regs->bp = pebs->bp;
-                       regs->sp = pebs->sp;
-               }
+               regs->bp = pebs->bp;
+               regs->sp = pebs->sp;
 
 #ifndef CONFIG_X86_32
                regs->r8 = pebs->r8;
index 1fa12887ec020568eaca8ba8b0e334f4471c20f3..87f6db437e4af55c4b27cdcafb040e5d5e46d832 100644 (file)
@@ -1130,6 +1130,7 @@ extern void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct
 extern struct perf_callchain_entry *
 get_perf_callchain(struct pt_regs *regs, u32 init_nr, bool kernel, bool user,
                   u32 max_stack, bool crosstask, bool add_mark);
+extern struct perf_callchain_entry *perf_callchain(struct perf_event *event, struct pt_regs *regs);
 extern int get_callchain_buffers(int max_stack);
 extern void put_callchain_buffers(void);
 
index b8e288a1f7409012d50e464e7993b96d4c404610..eeb787b1c53c72771c8d684154b7a87dc029a45b 100644 (file)
@@ -143,6 +143,8 @@ enum perf_event_sample_format {
        PERF_SAMPLE_PHYS_ADDR                   = 1U << 19,
 
        PERF_SAMPLE_MAX = 1U << 20,             /* non-ABI */
+
+       __PERF_SAMPLE_CALLCHAIN_EARLY           = 1ULL << 63,
 };
 
 /*
index 8f0434a9951af00bce3f009c21ddbd3c5cd61e01..cdb32cf8e33cb8b6bf04081022a27b5dd382b538 100644 (file)
@@ -6343,7 +6343,7 @@ static u64 perf_virt_to_phys(u64 virt)
 
 static struct perf_callchain_entry __empty_callchain = { .nr = 0, };
 
-static struct perf_callchain_entry *
+struct perf_callchain_entry *
 perf_callchain(struct perf_event *event, struct pt_regs *regs)
 {
        bool kernel = !event->attr.exclude_callchain_kernel;
@@ -6382,7 +6382,9 @@ void perf_prepare_sample(struct perf_event_header *header,
        if (sample_type & PERF_SAMPLE_CALLCHAIN) {
                int size = 1;
 
-               data->callchain = perf_callchain(event, regs);
+               if (!(sample_type & __PERF_SAMPLE_CALLCHAIN_EARLY))
+                       data->callchain = perf_callchain(event, regs);
+
                size += data->callchain->nr;
 
                header->size += size * sizeof(u64);