Messages in this thread | | | Date | Wed, 27 Oct 2021 21:09:42 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH v2 1/2] tools/perf: Add bitfield_swap to handle branch_stack endian issue |
| |
Em Mon, Oct 25, 2021 at 02:07:23PM +0200, Jiri Olsa escreveu: > On Sat, Oct 16, 2021 at 06:20:58PM +0530, Madhavan Srinivasan wrote: > > branch_stack struct has bit field definition which > > produces different bit ordering for big/little endian. > > Because of this, when branch_stack sample is collected > > in a BE system and viewed/reported in a LE system, bit > > fields of the branch stack are not presented properly. > > To address this issue, a evsel__bitfield_swap_branch_stack() > > is defined and introduced in evsel__parse_sample. > > > > Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com> > > for both patches > > Acked-by: Jiri Olsa <jolsa@redhat.com>
Thanks, applied.
- Arnaldo
> thanks, > jirka > > > --- > > Changelog v1: > > - Renamed function and macro > > - Added comments in code > > > > tools/perf/util/evsel.c | 74 +++++++++++++++++++++++++++++++++++++++-- > > tools/perf/util/evsel.h | 13 ++++++++ > > 2 files changed, 85 insertions(+), 2 deletions(-) > > > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > > index dbfeceb2546c..746e642d4d32 100644 > > --- a/tools/perf/util/evsel.c > > +++ b/tools/perf/util/evsel.c > > @@ -2221,6 +2221,51 @@ void __weak arch_perf_parse_sample_weight(struct perf_sample *data, > > data->weight = *array; > > } > > > > +u64 evsel__bitfield_swap_branch_flags(u64 value) > > +{ > > + u64 new_val = 0; > > + > > + /* > > + * branch_flags > > + * union { > > + * u64 values; > > + * struct { > > + * mispred:1 //target mispredicted > > + * predicted:1 //target predicted > > + * in_tx:1 //in transaction > > + * abort:1 //transaction abort > > + * cycles:16 //cycle count to last branch > > + * type:4 //branch type > > + * reserved:40 > > + * } > > + * } > > + * > > + * Avoid bswap64() the entire branch_flag.value, > > + * as it has variable bit-field sizes. Instead the > > + * macro takes the bit-field position/size, > > + * swaps it based on the host endianness. > > + */ > > + if (bigendian()) { > > + new_val = bitfield_swap(value, 0, 1); > > + new_val |= bitfield_swap(value, 1, 1); > > + new_val |= bitfield_swap(value, 2, 1); > > + new_val |= bitfield_swap(value, 3, 1); > > + new_val |= bitfield_swap(value, 4, 16); > > + new_val |= bitfield_swap(value, 20, 4); > > + new_val |= bitfield_swap(value, 24, 40); > > + } else { > > + new_val = bitfield_swap(value, 63, 1); > > + new_val |= bitfield_swap(value, 62, 1); > > + new_val |= bitfield_swap(value, 61, 1); > > + new_val |= bitfield_swap(value, 60, 1); > > + new_val |= bitfield_swap(value, 44, 16); > > + new_val |= bitfield_swap(value, 40, 4); > > + new_val |= bitfield_swap(value, 0, 40); > > + } > > + > > + return new_val; > > +} > > + > > int evsel__parse_sample(struct evsel *evsel, union perf_event *event, > > struct perf_sample *data) > > { > > @@ -2408,6 +2453,8 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event, > > if (type & PERF_SAMPLE_BRANCH_STACK) { > > const u64 max_branch_nr = UINT64_MAX / > > sizeof(struct branch_entry); > > + struct branch_entry *e; > > + unsigned int i; > > > > OVERFLOW_CHECK_u64(array); > > data->branch_stack = (struct branch_stack *)array++; > > @@ -2416,10 +2463,33 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event, > > return -EFAULT; > > > > sz = data->branch_stack->nr * sizeof(struct branch_entry); > > - if (evsel__has_branch_hw_idx(evsel)) > > + if (evsel__has_branch_hw_idx(evsel)) { > > sz += sizeof(u64); > > - else > > + e = &data->branch_stack->entries[0]; > > + } else { > > data->no_hw_idx = true; > > + /* > > + * if the PERF_SAMPLE_BRANCH_HW_INDEX is not applied, > > + * only nr and entries[] will be output by kernel. > > + */ > > + e = (struct branch_entry *)&data->branch_stack->hw_idx; > > + } > > + > > + if (swapped) { > > + /* > > + * struct branch_flag does not have endian > > + * specific bit field definition. And bswap > > + * will not resolve the issue, since these > > + * are bit fields. > > + * > > + * evsel__bitfield_swap_branch_flags() uses a > > + * bitfield_swap macro to swap the bit position > > + * based on the host endians. > > + */ > > + for (i = 0; i < data->branch_stack->nr; i++, e++) > > + e->flags.value = evsel__bitfield_swap_branch_flags(e->flags.value); > > + } > > + > > OVERFLOW_CHECK(array, sz, max_size); > > array = (void *)array + sz; > > } > > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h > > index 1f7edfa8568a..2e82cdbe2c08 100644 > > --- a/tools/perf/util/evsel.h > > +++ b/tools/perf/util/evsel.h > > @@ -482,4 +482,17 @@ struct evsel *evsel__leader(struct evsel *evsel); > > bool evsel__has_leader(struct evsel *evsel, struct evsel *leader); > > bool evsel__is_leader(struct evsel *evsel); > > void evsel__set_leader(struct evsel *evsel, struct evsel *leader); > > + > > +/* > > + * Macro to swap the bit-field postition and size. > > + * Used when, > > + * - dont need to swap the entire u64 && > > + * - when u64 has variable bit-field sizes && > > + * - when presented in a host endian which is different > > + * than the source endian of the perf.data file > > + */ > > +#define bitfield_swap(src, pos, size) \ > > + ((((src) >> (pos)) & ((1ull << (size)) - 1)) << (63 - ((pos) + (size) - 1))) > > + > > +u64 evsel__bitfield_swap_branch_flags(u64 value); > > #endif /* __PERF_EVSEL_H */ > > -- > > 2.31.1 > >
--
- Arnaldo
| |