Re: [PATCH v2 1/5] perf arm-spe: Define metadata header version 2

[Leo Yan <leo.yan@arm.com>]

On 9/27/24 07:32, Namhyung Kim wrote:

[...]

>> diff --git a/tools/perf/util/arm-spe.h b/tools/perf/util/arm-spe.h
>> index 4f4900c18f3e..5416d4e1d15f 100644
>> --- a/tools/perf/util/arm-spe.h
>> +++ b/tools/perf/util/arm-spe.h
>> @@ -12,10 +12,46 @@
>>   enum {
>>        ARM_SPE_PMU_TYPE,
>>        ARM_SPE_PER_CPU_MMAPS,
>> +     ARM_SPE_AUXTRACE_V1_PRIV_MAX,
>> +};
>> +
>> +#define ARM_SPE_AUXTRACE_V1_PRIV_SIZE        \
>> +     (ARM_SPE_AUXTRACE_V1_PRIV_MAX * sizeof(u64))
>> +
>> +enum {
>> +     /*
>> +      * The old metadata format (defined above) does not include a
>> +      * field for version number. Version 1 is reserved and starts
>> +      * from version 2.
>> +      */
>> +     ARM_SPE_HEADER_VERSION,
>> +     /* Number of sizeof(u64) */
>> +     ARM_SPE_HEADER_SIZE,
>> +     /* PMU type shared by CPUs */
>> +     ARM_SPE_SHARED_PMU_TYPE,
>> +     /* Number of CPUs */
>> +     ARM_SPE_CPUS_NUM,
>>        ARM_SPE_AUXTRACE_PRIV_MAX,
>>   };
> 
> Why don't you define something like struct arm_spe_header_v2 ?

Here the target is to make metadata to be self-described and can be
easily extended for new version (but I would not expect updating header
will happen frequently).

The fields ARM_SPE_HEADER_VERSION and ARM_SPE_HEADER_SIZE give a
header version and a header size. This allows us to easily extend new
enum items for new header. A benefit is we can use general code for
decoding because we don't need to process structure for a specific
version.

Thanks for review!

Leo