linux-kernel - Re: [PATCH -tip v5 2/5] perf: set correct value to perf_event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1331055135.11248.325.camel@twins>
Date:	Tue, 06 Mar 2012 18:32:15 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Akihiro Nagai <akihiro.nagai.hw@...achi.com>
Cc:	Arnaldo Carvalho de Melo <acme@...radead.org>,
	Ingo Molnar <mingo@...e.hu>,
	Frederic Weisbecker <fweisbec@...il.com>,
	David Ahern <dsahern@...il.com>, linux-kernel@...r.kernel.org,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	yrl.pp-manager.tt@...achi.com, Paul Mackerras <paulus@...ba.org>
Subject: Re: [PATCH -tip v5 2/5] perf: set correct value to
 perf_event_header.misc for BTS

On Tue, 2012-02-21 at 14:39 +0900, Akihiro Nagai wrote:
> (2012/01/30 18:35), Peter Zijlstra wrote:
> > On Mon, 2012-01-30 at 13:43 +0900, Akihiro Nagai wrote:
> >> @@ -330,6 +342,14 @@ int intel_pmu_drain_bts_buffer(void)
> >>                  return 1;
> >>
> >>          for (; at<  top; at++) {
> >> +               /*
> >> +                * To resolve user space symbols and DSOs correctly, set
> >> +                * PERF_RECORD_MISC_USER if from_addr or to_addr is user space.
> >> +                */
> >> +               if (!kernel_ip(data.ip) || !kernel_ip(data.addr)) {
> >> +                       header.misc&= ~PERF_RECORD_MISC_CPUMODE_MASK;
> >> +                       header.misc |= PERF_RECORD_MISC_USER;
> >> +               }
> >>                  data.ip         = at->from;
> >>                  data.addr       = at->to;
> >
> > Why not key off of the from? If its a jump from userspace its a user
> > event, its a jump from kernel space its a kernel event?
> 
> Of course, originally, perf does that. 

I don't think it does.

> And, in those cases,
> BTS records the both addresses of kernel and user in one
> perf_sample on branches from kernel to user. 

Sorry, I don't get this.

> Current perf sets PERF_RECORD_MISC_KERNEL to all BTS events,

It doesn't, it does something far more stupid.. it sets the state to
wherever we were when the BTS overflow interrupt happens. If that was in
kernel space, we mark all of them as in-kernel, if that was in
user-space we mark all of then in-user.

>  and
> perf-script doesn't resolve symbols and DSOs about the
> user-space address, because it is a kernel event. 

Well, that's a perf-script problem, not something you should change the
kernel for.

So how about something like this?

---
 arch/x86/kernel/cpu/perf_event_intel_ds.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c
b/arch/x86/kernel/cpu/perf_event_intel_ds.c
index d6bd49f..81e788c 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
@@ -333,6 +333,13 @@ int intel_pmu_drain_bts_buffer(void)
 		data.ip		= at->from;
 		data.addr	= at->to;
 
+		/* XXX doesn't do virt muck properly */
+		header.misc &= ~PERF_RECORD_MISC_CPUMODE_MASK;
+		if (kernel_ip(at->from))
+			header.misc |= PERF_RECORD_MISC_KERNEL;
+		else
+			header.misc |= PERF_RECORD_MISC_USER;
+
 		perf_output_sample(&handle, &header, &data, event);
 	}
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/