lkml.org 
[lkml]   [2022]   [Jan]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 2/2] perf arm-spe: Parse more SPE fields and store source
Date
Hi German,

On 28/01/2022 19:20, German Gomez wrote:
>Hi Ali,
>
>On 25/01/2022 19:20, Ali Saidi wrote:
>> Decode more SPE events and op types to allow for processing by perf
>> scripts. For example looking for branches which may indicate candidates
>> for conversion to a CSEL, store exclusives that are candidates for
>> conversion to LSE atomics and record the source information for memory
>> ops.
>> [snip]
>> + if (SPE_OP_PKT_IS_LDST_ATOMIC(payload)) {
>> + if (payload & SPE_OP_PKT_AT)
>> + decoder->record.op |= ARM_SPE_LDST_ATOMIC;
>
>In "utils/arm-spe.c" we check "if (record->op == ARM_SPE_LD)" so this
>ORing could break some of the generated samples.

Yep, you're correct. Interestingly I can only find one use of record->op using
equivalence instead of a logical and so perhaps it's best to fix this one use.

...
>> diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
>> index 69b31084d6be..113e427afe99 100644
>> --- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
>> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
>> @@ -22,11 +22,18 @@ enum arm_spe_sample_type {
>> ARM_SPE_TLB_MISS = 1 << 5,
>> ARM_SPE_BRANCH_MISS = 1 << 6,
>> ARM_SPE_REMOTE_ACCESS = 1 << 7,
>> + ARM_SPE_BR_NOT_TAKEN = 1 << 8,
>
>Can you rename it to ARM_SPE_BRANCH_NOT_TAKEN for consistency?

No problem

>
>> };
>>
>> enum arm_spe_op_type {
>> ARM_SPE_LD = 1 << 0,
>> ARM_SPE_ST = 1 << 1,
>> + ARM_SPE_LDST_EXCL = 1 << 2,
>> + ARM_SPE_LDST_ATOMIC = 1 << 3,
>> + ARM_SPE_LDST_ACQREL = 1 << 4,
>> + ARM_SPE_BR = 1 << 5,
>> + ARM_SPE_BR_COND = 1 << 6,
>> + ARM_SPE_BR_IND = 1 << 7,
>
>I'm not sure if we should keep everything in one enum/bitmask. I'm also
>looking at adding more of the data from the packets to the record, and
>considering refactoring the record structure. I'll share here when I
>have something.

One straight forward way to do this would be to make it a u16 field that was
SPE operation-type header and operation-type payload with some accessors instead
of trying to re-encode the operation type into a new format.

Thanks,
Ali

\
 
 \ /
  Last update: 2022-01-28 22:04    [W:0.048 / U:0.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site