| 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
| ||
|
Message-ID: <YXnqRgcNkRSfijGp@kernel.org>
Date: Wed, 27 Oct 2021 21:09:42 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Jiri Olsa <jolsa@...hat.com>
Cc: Madhavan Srinivasan <maddy@...ux.ibm.com>, michael@...erman.id.au,
eranian@...gle.com, mark.rutland@....com, namhyung@...nel.org,
kjain@...ux.ibm.com, atrajeev@...ux.vnet.ibm.com,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
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@...ux.ibm.com>
>
> for both patches
>
> Acked-by: Jiri Olsa <jolsa@...hat.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
Powered by blists - more mailing lists