tools/perf: Add bitfield_swap to handle branch_stack endian issue
PATCH v3 1/2 tools/perf: Add bitfield_swap to handle branch_stack endian issue
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.
---
Changelog v2:
- used tep_is_bigendian() instead bigendian()
to avoid perf test python fail.
Changelog v1:
- Renamed function and macro
- Added comments in code
tools/perf/util/evsel.c | 77 +++++++++++++++++++++++++++++++++++++++-- tools/perf/util/evsel.h | 13 +++++++ 2 files changed, 88 insertions(+), 2 deletions(-)
tools/perf/util/evsel.c +75 -2
@@ -2221,6 +2221,54 @@ 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. +* +* tep_is_bigendian() is used here instead of +* bigendian() to avoid python test fails. +*/ +if (tep_is_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 +2456,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 +2466,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; }
tools/perf/util/evsel.h +13 -0
@@ -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 */
Signature · 1 line
2.31.1
PATCH v3 2/2 tools/perf/test: Add endian test for struct branch_flags
Extend sample-parsing test to include branch_flag bitfield-endian swap test. Patch include "util/trace-event.h" to sample-parsing for importing tep_is_bigendian() and extends samples_same() to include "needs_swap" to detect/enable check for bitfield-endian swap.
---
Changelog v2:
- used tep_is_bigendian() instead bigendian()
to avoid perf test python fail.
- Updated commit message
Changelog v1:
- Updated commit message
tools/perf/tests/sample-parsing.c | 43 +++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 5 deletions(-)
tools/perf/tests/sample-parsing.c +38 -5
@@ -13,6 +13,7 @@#include "evsel.h" #include "debug.h" #include "util/synthetic-events.h" +#include "util/trace-event.h" #include "tests.h"
@@ -30,9 +31,18 @@ } \ }while (0 ) +/* +* Hardcode the expected values for branch_entry flags. +* These are based on the input value (213) specified +* in branch_stack variable. + */ +#define BS_EXPECTED_BE 0xa00d000000000000 +#define BS_EXPECTED_LE 0xd5000000 +#define FLAG (s) s->branch_stack->entries[i].flags +static bool samples_same (const struct perf_sample *s1,const struct perf_sample *s2, -u64 type,u64 read_format) +u64 type,u64 read_format,bool needs_swap) {size_t i;
@@ -100,8 +110,14 @@ static bool samples_same(const struct perf_sample *s1,if (type &PERF_SAMPLE_BRANCH_STACK ) {COMP (branch_stack->nr);COMP (branch_stack->hw_idx); -for (i =0 ; i < s1->branch_stack->nr; i++) -MCOMP (branch_stack->entries[i]); +for (i =0 ; i < s1->branch_stack->nr; i++) { +if (needs_swap) +return ((tep_is_bigendian ()) ? + (FLAG (s2).value ==BS_EXPECTED_BE ) : + (FLAG (s2).value ==BS_EXPECTED_LE )); +else +MCOMP (branch_stack->entries[i]); + } }if (type &PERF_SAMPLE_REGS_USER ) {
@@ -248,7 +264,7 @@ static int do_test(u64 sample_type, u64 sample_regs, u64 read_format) }, };struct sample_read_value values[] = {{1 ,5 }, {9 ,3 }, {2 ,7 }, {6 ,4 },}; -struct perf_sample sample_out; +struct perf_sample sample_out, sample_out_endian;size_t i, sz, bufsz;int err, ret =-1 ;
@@ -313,12 +329,29 @@ static int do_test(u64 sample_type, u64 sample_regs, u64 read_format) goto out_free; } -if (!samples_same (&sample, &sample_out, sample_type, read_format)) { +if (!samples_same (&sample, &sample_out, sample_type, read_format, evsel.needs_swap)) {pr_debug ("parsing failed for sample_type %#" PRIx64"\n" , sample_type); goto out_free; } +if (sample_type ==PERF_SAMPLE_BRANCH_STACK ) { + evsel.needs_swap = true; + evsel.sample_size =__evsel__sample_size (sample_type); + err =evsel__parse_sample (&evsel, event, &sample_out_endian); +if (err) { +pr_debug ("%s failed for sample_type %#" PRIx64", error %d\n" , +"evsel__parse_sample" , sample_type, err); + goto out_free; + } + +if (!samples_same (&sample, &sample_out_endian, sample_type, read_format, evsel.needs_swap)) { +pr_debug ("parsing failed for sample_type %#" PRIx64"\n" , + sample_type); + goto out_free; + } + } + ret =0 ; out_free:free (event);
Signature · 1 line
2.31.1
Arnaldo Carvalho de Melo
maintainerEm Thu, Oct 28, 2021 at 05:07:13PM +0530, Madhavan Srinivasan escreveu:
Quoted 7 lines
branch_stackstruct has bit field definition which produces different bit ordering for big/little endian. Because of this, whenbranch_stacksample 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, aevsel__bitfield_swap_branch_stack()is defined and introduced inevsel__parse_sample.
Jiri,
This is a minor change, he just switched to an inline equivalent
(the code is slightly different, but should be ok), can I apply yout v2 Acked-by tags?
- Arnaldo
Quoted 142 lines
Signed-off-by Madhavan Srinivasan <[email protected]>---
Changelog v2:
- usedtep_is_bigendian()insteadbigendian()
to avoid perf test python fail.Changelog v1:
- Renamed function and macro
- Added comments in codetools/perf/util/evsel.c | 77 +++++++++++++++++++++++++++++++++++++++-- tools/perf/util/evsel.h | 13 +++++++ 2 files changed, 88 insertions(+), 2 deletions(-)tools/perf/util/evsel.c +75
-2@@ -2221,6 +2221,54 @@ 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. +* +* tep_is_bigendian() is used here instead of +* bigendian() to avoid python test fails. +*/ +if (tep_is_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 +2456,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 +2466,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; }tools/perf/util/evsel.h +13
-0@@ -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 */ Signature · 1 line
2.31.1
Signature · 1 line
- Arnaldo