Messages in this thread Patch in this message |  | | Subject | Re: [RFC] Support Intel-PT code build in 32-bit arches | From | Adrian Hunter <> | Date | Fri, 22 Oct 2021 15:23:53 +0300 |
| |
On 21/10/2021 23:31, Arnaldo Carvalho de Melo wrote: > Em Wed, Oct 20, 2021 at 02:25:43PM -0300, Arnaldo Carvalho de Melo escreveu: >> Em Wed, Oct 20, 2021 at 03:41:01PM +0100, John Garry escreveu: >>> I suppose the best thing now is to send a patch on top once perf/core >>> contains that commit. Let me know otherwise. > >> You can send a v2, as: > >> 29 8.60 debian:experimental-x-mipsel : FAIL gcc version 11.2.0 (Debian 11.2.0-9) >> util/intel-pt.c: In function 'intel_pt_synth_pebs_sample': >> util/intel-pt.c:2146:33: error: passing argument 1 of 'find_first_bit' from incompatible pointer type [-Werror=incompatible-pointer-types] >> 2146 | for_each_set_bit(hw_id, &items->applicable_counters, INTEL_PT_MAX_PEBS) { >> /git/perf-5.15.0-rc4/tools/include/linux/bitops.h:37:38: note: in definition of macro 'for_each_set_bit' >> 37 | for ((bit) = find_first_bit((addr), (size)); \ >> | ^~~~ >> In file included from /git/perf-5.15.0-rc4/tools/include/asm-generic/bitops.h:21, >> from /git/perf-5.15.0-rc4/tools/include/linux/bitops.h:34, >> from /git/perf-5.15.0-rc4/tools/include/linux/bitmap.h:6, >> from util/header.h:10, >> from util/session.h:7, >> from util/intel-pt.c:16: >> /git/perf-5.15.0-rc4/tools/include/asm-generic/bitops/find.h:109:51: note: expected 'const long unsigned int *' but argument is of type 'const uint64_t *' {aka 'const long long unsigned int *'} >> >> Adrian, this is on: >> >> commit 803a3c9233990e1adac8ea2421e3759c2d380cf8 >> Author: Adrian Hunter <adrian.hunter@intel.com> >> Date: Tue Sep 7 19:39:03 2021 +0300 >> >> perf intel-pt: Add support for PERF_RECORD_AUX_OUTPUT_HW_ID >> >> Originally, software only supported redirecting at most one PEBS event to >> Intel PT (PEBS-via-PT) because it was not able to differentiate one event >> from another. To overcome that, add support for the >> PERF_RECORD_AUX_OUTPUT_HW_ID side-band event. >> >> Reviewed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com> >> Reviewed-by: Andi Kleen <ak@linux.intel.com> >> >> >> That is still just on tmp.perf/core, so we can fix it, probably its just >> making that uint64_t into a unsigned long, will check later if you don't >> beat me to it. > > Adrian, > > Probably we should just disable Intel PT support from being > built on 32-bit arches, right? I don't know if anybody is interested on > that, my tests just try to figure out if it continues to build, and if > fixing any problem isn't costly, which in this case is more than my > threshold, wdyt?
It feels like bad form not to compile on 32-bit.
In this case we could just cast it, and I see that has been done in other places for for_each_set_bit(), but (and maybe I've got it wrong) that wouldn't work for a 32-bit big endian system?
The following might cover all cases, do you think?
diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c index 1073c56a512c..7c979ffefade 100644 --- a/tools/perf/util/intel-pt.c +++ b/tools/perf/util/intel-pt.c @@ -2129,9 +2129,17 @@ static int intel_pt_synth_single_pebs_sample(struct intel_pt_queue *ptq) return intel_pt_do_synth_pebs_sample(ptq, evsel, id); } +/* For 32-bit big endian, put the longs from u64 in correct order */ +#define U64_BITS(var, val) \ + unsigned long var[] = { \ + [0] = (val), \ + [1] = (val) >> 32, \ + } + static int intel_pt_synth_pebs_sample(struct intel_pt_queue *ptq) { const struct intel_pt_blk_items *items = &ptq->state->items; + U64_BITS(ac_bits, items->applicable_counters); struct intel_pt_pebs_event *pe; struct intel_pt *pt = ptq->pt; int err = -EINVAL; @@ -2143,7 +2151,7 @@ static int intel_pt_synth_pebs_sample(struct intel_pt_queue *ptq) return intel_pt_synth_single_pebs_sample(ptq); } - for_each_set_bit(hw_id, &items->applicable_counters, INTEL_PT_MAX_PEBS) { + for_each_set_bit(hw_id, ac_bits, INTEL_PT_MAX_PEBS) { pe = &ptq->pebs[hw_id]; if (!pe->evsel) { if (!pt->single_pebs)
|  |