When decoding the perf_regs mask in regs_dump__printf(),
we loop through the mask using find_first_bit and find_next_bit functions.
And mask is of type "u64". But "u64" is send as a "unsigned long *" to
lib functions along with sizeof().
While the exisitng code works fine in most of the case, when using a
32bit perf on a 64bit kernel (Big Endian), we end up reading the wrong word
in the u64 mask. Patch to fix the mask in regs_dump__printf().
Suggested-by: Yury Norov <redacted>
Cc: Yury Norov <redacted>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Kan Liang <redacted>
Cc: Wang Nan <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Madhavan Srinivasan <redacted>
---
tools/perf/util/session.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
From: Jiri Olsa <hidden> Date: 2016-06-17 06:37:16
On Fri, Jun 17, 2016 at 10:52:38AM +0530, Madhavan Srinivasan wrote:
quoted hunk
When decoding the perf_regs mask in regs_dump__printf(),
we loop through the mask using find_first_bit and find_next_bit functions.
And mask is of type "u64". But "u64" is send as a "unsigned long *" to
lib functions along with sizeof().
While the exisitng code works fine in most of the case, when using a
32bit perf on a 64bit kernel (Big Endian), we end up reading the wrong word
in the u64 mask. Patch to fix the mask in regs_dump__printf().
Suggested-by: Yury Norov <redacted>
Cc: Yury Norov <redacted>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Kan Liang <redacted>
Cc: Wang Nan <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Madhavan Srinivasan <redacted>
---
tools/perf/util/session.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
I think we should do this earlier when reading the mask,
not at the moment we just print it, like we do for other
types.. maybe we could do this as an extra bit for
perf_event__swap_ops[PERF_RECORD_SAMPLE] function
also there's print_sample_iregs in builtin-script.c that's
most likely affected as well
thanks,
jirka
On Fri, Jun 17, 2016 at 10:52:38AM +0530, Madhavan Srinivasan wrote:
quoted
When decoding the perf_regs mask in regs_dump__printf(),
we loop through the mask using find_first_bit and find_next_bit functions.
And mask is of type "u64". But "u64" is send as a "unsigned long *" to
lib functions along with sizeof().
While the exisitng code works fine in most of the case, when using a
32bit perf on a 64bit kernel (Big Endian), we end up reading the wrong word
in the u64 mask. Patch to fix the mask in regs_dump__printf().
Suggested-by: Yury Norov <redacted>
Cc: Yury Norov <redacted>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Kan Liang <redacted>
Cc: Wang Nan <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Madhavan Srinivasan <redacted>
---
tools/perf/util/session.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
I think we should do this earlier when reading the mask,
not at the moment we just print it, like we do for other
types.. maybe we could do this as an extra bit for
Ok. I tried something like this. Currently
mask is read in perf_evsel__parse_sample(),
so added for both sample type (PERF_SAMPLE_REGS_USER
and PERF_SAMPLE_REGS_INTR)
if (data->user_regs.abi) {
- u64 mask = evsel->attr.sample_regs_user;
+ u.val64 = evsel->attr.sample_regs_user;
- sz = hweight_long(mask) * sizeof(u64);
+ if (sizeof(u64) > sizeof(unsigned long)) {
+ u64 mask = u.val64;
+ u.val32[1] = mask >> 32;
+ u.val32[0] = mask & ULONG_MAX;
+ }
+
+ sz = hweight_long(u.val64) * sizeof(u64);
OVERFLOW_CHECK(array, sz, max_size);
- data->user_regs.mask = mask;
+ data->user_regs.mask = u.val64;
data->user_regs.regs = (u64 *)array;
Issue I see is when printing the mask value in a 32bit perf
on a 64bit kernel (big endian).
442044948492 0xdc0 [0x188]: PERF_RECORD_SAMPLE(IP, 0x1): 7299/7299:
0xc000000000059200 period: 12599 addr: 0
... intr regs: mask 0xffffffff000007ff ABI 32-bit
^^^ shld have been 0x7ffffffffff
I agree it is better to fix this when reading, but
we need to swap again when printing?
perf_event__swap_ops[PERF_RECORD_SAMPLE] function
also there's print_sample_iregs in builtin-script.c that's
most likely affected as well
From: Jiri Olsa <hidden> Date: 2016-06-17 11:24:09
On Fri, Jun 17, 2016 at 02:43:31PM +0530, Madhavan Srinivasan wrote:
SNIP
if (data->user_regs.abi) {
- u64 mask = evsel->attr.sample_regs_user;
+ u.val64 = evsel->attr.sample_regs_user;
- sz = hweight_long(mask) * sizeof(u64);
+ if (sizeof(u64) > sizeof(unsigned long)) {
+ u64 mask = u.val64;
+ u.val32[1] = mask >> 32;
+ u.val32[0] = mask & ULONG_MAX;
+ }
+
+ sz = hweight_long(u.val64) * sizeof(u64);
OVERFLOW_CHECK(array, sz, max_size);
- data->user_regs.mask = mask;
+ data->user_regs.mask = u.val64;
data->user_regs.regs = (u64 *)array;
Issue I see is when printing the mask value in a 32bit perf
on a 64bit kernel (big endian).
442044948492 0xdc0 [0x188]: PERF_RECORD_SAMPLE(IP, 0x1): 7299/7299:
0xc000000000059200 period: 12599 addr: 0
... intr regs: mask 0xffffffff000007ff ABI 32-bit
^^^ shld have been 0x7ffffffffff
I agree it is better to fix this when reading, but
we need to swap again when printing?
hum, sample data should be swap at this point already..
see perf_session__process_event
jirka