Re: [PATCH] Fix corrupt data after perf inject from

Hi Al,

On Mon, 1 Jun 2020 at 10:00, Al Grant Al.Grant@arm.com wrote:

(Hi - posted for review. There was a recent ABI breaking change (see commit reference below), after which perf inject on ETM creates corrupt files. The simple fix ends up with it using the new ABI. I have an alternative patch set that emulates the old ABI - for the benefit of tools like autofdo that can't yet consume the new ABI - but it's much messier.)

From: Al Grant al.grant@arm.com

Commit 42bbabed09ce6208026648a71a45b4394c74585a ("perf tools: Add hw_idx in struct branch_stack") changed the format of branch stacks in perf samples. When samples use this new format, a flag must be set in the corresponding event. Synthesized branch stacks generated from CoreSight ETM trace were using the new format, but not setting the event attribute, leading to consumers seeing corrupt data. This patch fixes the issue by setting the event attribute to indicate use of the new format.

Fixes: 42bbabed09ce ("perf tools: Add hw_idx in struct branch_stack") Signed-off-by: Al Grant al.grant@arm.com Acked-by: Andrea Brunato andrea.brunato@arm.com

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 62d2f9b9ce1b..71a056e29675 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -1332,7 +1332,13 @@ static int cs_etm__synth_events(struct cs_etm_auxtrace *etm, }

    if (etm->synth_opts.last_branch)

•   {
            attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
  

•           /* We don't use the hardware index, but the sample generation
  

•              code uses the new format branch_stack with this field,
  

•               so the event attributes must indicate that it's present. */
  

•           attr.branch_sample_type |= PERF_SAMPLE_BRANCH_HW_INDEX;
  

I'm fine with this set and would ask you to send it to the public mailing list when 5.8-rc1 has been released. Out of interest I have the following 2 questions:

Would setting sample.no_hw_idx to 'true' would have the same effect?

Have you tried with PT? I see that Kan's patch applied the same fix to both cs-etm.c and intel-pt.c and as such I would expect to find the same problem there as well.

Thanks, Mathieu

•   }
  
    if (etm->synth_opts.instructions) {
            attr.config = PERF_COUNT_HW_INSTRUCTIONS;
  

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight