| 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
| ||
|
Date: Thu, 28 Oct 2021 10:00:27 +0530
From: Madhavan Srinivasan <maddy@...ux.ibm.com>
To: Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: jolsa@...hat.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
On 10/28/21 8:07 AM, Arnaldo Carvalho de Melo wrote:
>
> On October 27, 2021 9:16:51 PM GMT-03:00, Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
>> Em Sat, Oct 16, 2021 at 06:20:58PM +0530, Madhavan Srinivasan escreveu:
>>> 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>
>>> ---
>>> Changelog v1:
>>> - Renamed function and macro
>>> - Added comments in code
> Please, run 'perf test' before submitting patches, it's not that difficult, please do so.
I did. I ran 'perf test' and did not see any fail. I did not do 'perf
test python', will check that out. My bad.
Maddy
>
> - Arnaldo
>
>> [acme@...co perf]$ perf test python
>> 19: 'import perf' in python : FAILED!
>> [acme@...co perf]$ perf test -v python
>> Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
>> 19: 'import perf' in python :
>> --- start ---
>> test child forked, pid 991284
>> python usage test: "echo "import sys ; sys.path.append('/tmp/build/perf/python'); import perf" | '/usr/bin/python3' "
>> Traceback (most recent call last):
>> File "<stdin>", line 1, in <module>
>> ImportError: /tmp/build/perf/python/perf.cpython-39-x86_64-linux-gnu.so: undefined symbol: bigendian
>> test child finished with -1
>> ---- end ----
>> 'import perf' in python: FAILED!
>> [acme@...co perf]$
>>
>>> 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
Powered by blists - more mailing lists